AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert of Parser: Make skipping HTML comments optional. (patchset #6 id:140001 of https://codereview.chromium.org/1801203002/ )

Browse Source 
Reason for revert:
Violates ES6 spec (crbug.com/4850), and implementation was over-eager. Will revert for now.

Original issue's description:
> Parser: Make skipping HTML comments optional.
>
> API change: This adds a new flag skip_html_comments to v8::ScriptOriginOptions. This flag controls whether V8 will attempt to honour HTML-style comments in JS sources.
>
> (That is: Gracefully ignore <!-- ... ---> in JS sources, which was a popular technique in the early days of JavaScript, to prevent non-JS-enabled browsers from displaying script sources to uses.)
>
> The flag defaults to 'true' when using v8::ScriptOrigin constructor, which preserves the existing behaviour. Embedders which are happy with the existing behaviour will thus not need any changes.
>
> BUG=chromium:573887
> LOG=Y
>
> Committed: https://crrev.com/91d344288aa51ed03eaaa1cb3e368ac1e82f0173
> Cr-Commit-Position: refs/heads/master@{#34904}

TBR=jochen@chromium.org,rossberg@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=chromium:573887, v8:4850
LOG=Y

Review URL: https://codereview.chromium.org/1817163003

Cr-Commit-Position: refs/heads/master@{#34958}
This commit is contained in:
vogelheim 2016-03-21 10:48:29 -07:00 committed by Commit bot
parent 35a14c75e3
commit 09ac4f295c
9 changed files with 37 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
include/v8.h
View File

@ -1005,31 +1005,28 @@ class ScriptOriginOptions {
public:
V8_INLINE ScriptOriginOptions(bool is_embedder_debug_script = false,
bool is_shared_cross_origin = false,
bool is_opaque = false,
bool allow_html_comments = false)
bool is_opaque = false)
: flags_((is_embedder_debug_script ? kIsEmbedderDebugScript : 0) |
(is_shared_cross_origin ? kIsSharedCrossOrigin : 0) |
(is_opaque ? kIsOpaque : 0) |
(allow_html_comments ? kAllowHtmlComments : 0)) {}
(is_opaque ? kIsOpaque : 0)) {}
V8_INLINE ScriptOriginOptions(int flags)
: flags_(flags & (kIsEmbedderDebugScript | kIsSharedCrossOrigin |
kIsOpaque | kAllowHtmlComments)) {}
bool IsEmbedderDebugScript() const { return HasFlag(kIsEmbedderDebugScript); }
bool IsSharedCrossOrigin() const { return HasFlag(kIsSharedCrossOrigin); }
bool IsOpaque() const { return HasFlag(kIsOpaque); }
bool AllowHtmlComments() const { return HasFlag(kAllowHtmlComments); }
: flags_(flags &
(kIsEmbedderDebugScript | kIsSharedCrossOrigin | kIsOpaque)) {}
bool IsEmbedderDebugScript() const {
return (flags_ & kIsEmbedderDebugScript) != 0;
}
bool IsSharedCrossOrigin() const {
return (flags_ & kIsSharedCrossOrigin) != 0;
}
bool IsOpaque() const { return (flags_ & kIsOpaque) != 0; }
int Flags() const { return flags_; }
private:
enum {
kIsEmbedderDebugScript = 1,
kIsSharedCrossOrigin = 1 << 1,
kIsOpaque = 1 << 2,
kAllowHtmlComments = 1 << 3
kIsOpaque = 1 << 2
};
inline bool HasFlag(int flag) const { return (flags_ & flag) != 0; }
const int flags_;
};
@ -1046,8 +1043,7 @@ class ScriptOrigin {
Local<Integer> script_id = Local<Integer>(),
Local<Boolean> resource_is_embedder_debug_script = Local<Boolean>(),
Local<Value> source_map_url = Local<Value>(),
Local<Boolean> resource_is_opaque = Local<Boolean>(),
Local<Boolean> allow_html_comments = Local<Boolean>());
Local<Boolean> resource_is_opaque = Local<Boolean>());
V8_INLINE Local<Value> ResourceName() const;
V8_INLINE Local<Integer> ResourceLineOffset() const;
V8_INLINE Local<Integer> ResourceColumnOffset() const;
@ -7832,8 +7828,7 @@ ScriptOrigin::ScriptOrigin(Local<Value> resource_name,
Local<Integer> script_id,
Local<Boolean> resource_is_embedder_debug_script,
Local<Value> source_map_url,
Local<Boolean> resource_is_opaque,
Local<Boolean> allow_html_comments)
Local<Boolean> resource_is_opaque)
: resource_name_(resource_name),
resource_line_offset_(resource_line_offset),
resource_column_offset_(resource_column_offset),
@ -7841,8 +7836,7 @@ ScriptOrigin::ScriptOrigin(Local<Value> resource_name,
resource_is_embedder_debug_script->IsTrue(),
!resource_is_shared_cross_origin.IsEmpty() &&
resource_is_shared_cross_origin->IsTrue(),
!resource_is_opaque.IsEmpty() && resource_is_opaque->IsTrue(),
allow_html_comments.IsEmpty() || allow_html_comments->IsTrue()),
!resource_is_opaque.IsEmpty() && resource_is_opaque->IsTrue()),
script_id_(script_id),
source_map_url_(source_map_url) {}

3
src/api.cc
View File

@ -226,8 +226,7 @@ static ScriptOrigin GetScriptOriginForScript(i::Isolate* isolate,
v8::Integer::New(v8_isolate, script->id()),
v8::Boolean::New(v8_isolate, options.IsEmbedderDebugScript()),
Utils::ToLocal(source_map_url),
v8::Boolean::New(v8_isolate, options.IsOpaque()),
v8::Boolean::New(v8_isolate, options.AllowHtmlComments()));
v8::Boolean::New(v8_isolate, options.IsOpaque()));
return origin;
}

5
src/objects.h
View File

@ -6565,9 +6565,8 @@ class Script: public Struct {
static const int kCompilationTypeBit = 0;
static const int kCompilationStateBit = 1;
static const int kHideSourceBit = 2;
static const int kAllowHtmlCommentsBit = 3;
static const int kOriginOptionsShift = 4;
static const int kOriginOptionsSize = 4;
static const int kOriginOptionsShift = 3;
static const int kOriginOptionsSize = 3;
static const int kOriginOptionsMask = ((1 << kOriginOptionsSize) - 1)
<< kOriginOptionsShift;

3
src/parsing/parser.cc
View File

@ -764,11 +764,12 @@ ClassLiteral* ParserTraits::ParseClassLiteral(
name_is_strict_reserved, pos, ok);
}
Parser::Parser(ParseInfo* info)
: ParserBase<ParserTraits>(info->zone(), &scanner_, info->stack_limit(),
info->extension(), info->ast_value_factory(),
NULL, this),
scanner_(info->unicode_cache(), info->allow_html_comments()),
scanner_(info->unicode_cache()),
reusable_preparser_(NULL),
original_scope_(NULL),
target_stack_(NULL),

4
src/parsing/parser.h
View File

@ -121,10 +121,6 @@ class ParseInfo {
uint32_t hash_seed() { return hash_seed_; }
void set_hash_seed(uint32_t hash_seed) { hash_seed_ = hash_seed; }
bool allow_html_comments() const {
return !script_.is_null() && script_->origin_options().AllowHtmlComments();
}
//--------------------------------------------------------------------------
// TODO(titzer): these should not be part of ParseInfo.
//--------------------------------------------------------------------------

5
src/parsing/scanner.cc
View File

@ -36,11 +36,10 @@ void Utf16CharacterStream::ResetToBookmark() { UNREACHABLE(); }
// ----------------------------------------------------------------------------
// Scanner
Scanner::Scanner(UnicodeCache* unicode_cache, bool allow_html_comments)
Scanner::Scanner(UnicodeCache* unicode_cache)
: unicode_cache_(unicode_cache),
bookmark_c0_(kNoBookmark),
octal_pos_(Location::invalid()),
allow_html_comments_(allow_html_comments),
found_html_comment_(false),
allow_harmony_exponentiation_operator_(false) {
bookmark_current_.literal_chars = &bookmark_current_literal_;
@ -485,7 +484,7 @@ void Scanner::Scan() {
token = Select(Token::LTE);
} else if (c0_ == '<') {
token = Select('=', Token::ASSIGN_SHL, Token::SHL);
} else if (c0_ == '!' && allow_html_comments_) {
} else if (c0_ == '!') {
token = ScanHtmlComment();
} else {
token = Token::LT;

5
src/parsing/scanner.h
View File

@ -340,7 +340,7 @@ class Scanner {
// -1 is outside of the range of any real source code.
static const int kNoOctalLocation = -1;
Scanner(UnicodeCache* scanner_contants, bool allow_html_comments);
explicit Scanner(UnicodeCache* scanner_contants);
void Initialize(Utf16CharacterStream* source);
@ -761,9 +761,6 @@ class Scanner {
// Whether there is a multi-line comment that contains a
// line-terminator after the current token, and before the next.
bool has_multiline_comment_before_next_;
// Whether to allow HTML comments (that is, skip over them, rather than
// reporting the comment marker as a sequence of tokens.)
bool allow_html_comments_;
// Whether this scanner encountered an HTML comment.
bool found_html_comment_;

37
test/cctest/test-api.cc
View File

@ -24858,40 +24858,3 @@ TEST(Proxy) {
CHECK(proxy->GetTarget()->SameValue(target));
CHECK(proxy->GetHandler()->IsNull());
}
TEST(HTMLCommentInJavascript) {
// Regression test for crbug.com/573887.
LocalContext context;
v8::HandleScope scope(CcTest::isolate());
// Inline scripts - i.e. the contents of <script> tags - need to obey HTML
// comments, but JS sources - i.e., everything else - shouldn't.
const char* source = "<!--\n var result = 2 + 2;\n-->\n result";
v8::ScriptOrigin inline_script(v8_str(""));
v8::ScriptOrigin source_script(
v8_str("http://some-resource.net/"), Local<v8::Integer>(),
Local<v8::Integer>(), Local<Boolean>(), Local<v8::Integer>(),
Local<Boolean>(), Local<Value>(), Local<Boolean>(),
v8::False(CcTest::isolate()));
// Case 1: An inline script.
{
v8::MaybeLocal<v8::Script> script =
v8::Script::Compile(context.local(), v8_str(source), &inline_script);
CHECK(!script.IsEmpty());
CHECK_EQ(4, script.ToLocalChecked()
->Run(context.local())
.ToLocalChecked()
->Int32Value(context.local())
.FromJust());
}
// Case 2: Js source file.
{
v8::MaybeLocal<v8::Script> script =
v8::Script::Compile(context.local(), v8_str(source), &source_script);
CHECK(script.IsEmpty());
}
}

54
test/cctest/test-parsing.cc
View File

@ -70,7 +70,7 @@ TEST(ScanKeywords) {
CHECK(static_cast<int>(sizeof(buffer)) >= length);
{
i::Utf8ToUtf16CharacterStream stream(keyword, length);
i::Scanner scanner(&unicode_cache, false);
i::Scanner scanner(&unicode_cache);
scanner.Initialize(&stream);
CHECK_EQ(key_token.token, scanner.Next());
CHECK_EQ(i::Token::EOS, scanner.Next());
@ -78,7 +78,7 @@ TEST(ScanKeywords) {
// Removing characters will make keyword matching fail.
{
i::Utf8ToUtf16CharacterStream stream(keyword, length - 1);
i::Scanner scanner(&unicode_cache, false);
i::Scanner scanner(&unicode_cache);
scanner.Initialize(&stream);
CHECK_EQ(i::Token::IDENTIFIER, scanner.Next());
CHECK_EQ(i::Token::EOS, scanner.Next());
@ -89,7 +89,7 @@ TEST(ScanKeywords) {
i::MemMove(buffer, keyword, length);
buffer[length] = chars_to_append[j];
i::Utf8ToUtf16CharacterStream stream(buffer, length + 1);
i::Scanner scanner(&unicode_cache, false);
i::Scanner scanner(&unicode_cache);
scanner.Initialize(&stream);
CHECK_EQ(i::Token::IDENTIFIER, scanner.Next());
CHECK_EQ(i::Token::EOS, scanner.Next());
@ -99,7 +99,7 @@ TEST(ScanKeywords) {
i::MemMove(buffer, keyword, length);
buffer[length - 1] = '_';
i::Utf8ToUtf16CharacterStream stream(buffer, length);
i::Scanner scanner(&unicode_cache, false);
i::Scanner scanner(&unicode_cache);
scanner.Initialize(&stream);
CHECK_EQ(i::Token::IDENTIFIER, scanner.Next());
CHECK_EQ(i::Token::EOS, scanner.Next());
@ -151,7 +151,7 @@ TEST(ScanHTMLEndComments) {
reinterpret_cast<const i::byte*>(tests[i]);
i::Utf8ToUtf16CharacterStream stream(source, i::StrLength(tests[i]));
i::CompleteParserRecorder log;
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
i::Zone zone;
i::AstValueFactory ast_value_factory(
@ -169,7 +169,7 @@ TEST(ScanHTMLEndComments) {
reinterpret_cast<const i::byte*>(fail_tests[i]);
i::Utf8ToUtf16CharacterStream stream(source, i::StrLength(fail_tests[i]));
i::CompleteParserRecorder log;
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
i::Zone zone;
i::AstValueFactory ast_value_factory(
@ -184,32 +184,6 @@ TEST(ScanHTMLEndComments) {
}
}
TEST(ScanHtmlComments) {
const char* src = "a <!-- b --> c";
i::UnicodeCache unicode_cache;
// Skip HTML comments:
{
i::Utf8ToUtf16CharacterStream stream(reinterpret_cast<const i::byte*>(src),
i::StrLength(src));
i::Scanner scanner(&unicode_cache, true);
scanner.Initialize(&stream);
CHECK_EQ(i::Token::IDENTIFIER, scanner.Next());
CHECK_EQ(i::Token::EOS, scanner.Next());
}
// Parse (don't skip) HTML comments:
{
i::Utf8ToUtf16CharacterStream stream(reinterpret_cast<const i::byte*>(src),
i::StrLength(src));
i::Scanner scanner(&unicode_cache, false);
scanner.Initialize(&stream);
CHECK_EQ(i::Token::IDENTIFIER, scanner.Next());
CHECK_EQ(i::Token::LT, scanner.Next());
CHECK_EQ(i::Token::NOT, scanner.Next());
CHECK_EQ(i::Token::DEC, scanner.Next());
}
}
class ScriptResource : public v8::String::ExternalOneByteStringResource {
public:
@ -349,7 +323,7 @@ TEST(StandAlonePreParser) {
reinterpret_cast<const i::byte*>(program),
static_cast<unsigned>(strlen(program)));
i::CompleteParserRecorder log;
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
i::Zone zone;
@ -385,7 +359,7 @@ TEST(StandAlonePreParserNoNatives) {
reinterpret_cast<const i::byte*>(program),
static_cast<unsigned>(strlen(program)));
i::CompleteParserRecorder log;
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
// Preparser defaults to disallowing natives syntax.
@ -456,7 +430,7 @@ TEST(RegressChromium62639) {
reinterpret_cast<const i::byte*>(program),
static_cast<unsigned>(strlen(program)));
i::CompleteParserRecorder log;
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
i::Zone zone;
i::AstValueFactory ast_value_factory(&zone,
@ -491,7 +465,7 @@ TEST(Regress928) {
i::Handle<i::String> source = factory->NewStringFromAsciiChecked(program);
i::GenericStringUtf16CharacterStream stream(source, 0, source->length());
i::CompleteParserRecorder log;
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
i::Zone zone;
i::AstValueFactory ast_value_factory(&zone,
@ -543,7 +517,7 @@ TEST(PreParseOverflow) {
reinterpret_cast<const i::byte*>(program.get()),
static_cast<unsigned>(kProgramSize));
i::CompleteParserRecorder log;
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
i::Zone zone;
@ -773,7 +747,7 @@ void TestStreamScanner(i::Utf16CharacterStream* stream,
i::Token::Value* expected_tokens,
int skip_pos = 0, // Zero means not skipping.
int skip_to = 0) {
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(stream);
int i = 0;
@ -856,7 +830,7 @@ void TestScanRegExp(const char* re_source, const char* expected) {
reinterpret_cast<const i::byte*>(re_source),
static_cast<unsigned>(strlen(re_source)));
i::HandleScope scope(CcTest::i_isolate());
i::Scanner scanner(CcTest::i_isolate()->unicode_cache(), false);
i::Scanner scanner(CcTest::i_isolate()->unicode_cache());
scanner.Initialize(&stream);
i::Token::Value start = scanner.peek();
@ -1576,7 +1550,7 @@ void TestParserSyncWithFlags(i::Handle<i::String> source,
// Preparse the data.
i::CompleteParserRecorder log;
if (test_preparser) {
i::Scanner scanner(isolate->unicode_cache(), false);
i::Scanner scanner(isolate->unicode_cache());
i::GenericStringUtf16CharacterStream stream(source, 0, source->length());
i::Zone zone;
i::AstValueFactory ast_value_factory(