Revert of Simplify DoParseProgram (patchset #2 id:20001 of...

Revert of Simplify DoParseProgram (patchset #2 id:20001 of https://codereview.chromium.org/1058363003/)

Reason for revert:
[Sheriff] Changes some layout tests on all platforms, e.g.:
http://build.chromium.org/p/client.v8/builders/V8-Blink%20Linux%2032/builds/2543

Original issue's description:
> Simplify DoParseProgram
>
> DoParseProgram doesn't appear to need to receive toplevel scopes as
> arguments; it can properly set the end_position of the scopes to the
> scanner's position after parsing is complete.
>
> R=marja@chromium.org
> BUG=
> LOG=N
>
> Committed: https://crrev.com/8da9252f61d3c499a78b0b94299c314b2eb0b0c8
> Cr-Commit-Position: refs/heads/master@{#27847}

TBR=marja@chromium.org,wingo@igalia.com
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=

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

Cr-Commit-Position: refs/heads/master@{#27856}

src/parser.cc

@@ -914,6 +914,8 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
   source = String::Flatten(source);
   FunctionLiteral* result;
 
+  Scope* top_scope = NULL;
+  Scope* eval_scope = NULL;
   if (source->IsExternalTwoByteString()) {
     // Notice that the stream is destroyed at the end of the branch block.
     // The last line of the blocks can't be moved outside, even though they're
@@ -921,11 +923,15 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
     ExternalTwoByteStringUtf16CharacterStream stream(
         Handle<ExternalTwoByteString>::cast(source), 0, source->length());
     scanner_.Initialize(&stream);
-    result = DoParseProgram(info);
+    result = DoParseProgram(info, &top_scope, &eval_scope);
   } else {
     GenericStringUtf16CharacterStream stream(source, 0, source->length());
     scanner_.Initialize(&stream);
-    result = DoParseProgram(info);
+    result = DoParseProgram(info, &top_scope, &eval_scope);
+  }
+  top_scope->set_end_position(source->length());
+  if (eval_scope != NULL) {
+    eval_scope->set_end_position(source->length());
   }
   HandleSourceURLComments(isolate, info->script());
 
@@ -950,7 +956,8 @@ FunctionLiteral* Parser::ParseProgram(Isolate* isolate, ParseInfo* info) {
 }
 
 
-FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
+FunctionLiteral* Parser::DoParseProgram(ParseInfo* info, Scope** scope,
+                                        Scope** eval_scope) {
   // Note that this function can be called from the main thread or from a
   // background thread. We should not access anything Isolate / heap dependent
   // via ParseInfo, and also not pass it forward.
@@ -959,11 +966,11 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
 
   FunctionLiteral* result = NULL;
   {
-    Scope* scope = NewScope(scope_, SCRIPT_SCOPE);
-    info->set_script_scope(scope);
+    *scope = NewScope(scope_, SCRIPT_SCOPE);
+    info->set_script_scope(*scope);
     if (!info->context().is_null() && !info->context()->IsNativeContext()) {
-      scope = Scope::DeserializeScopeChain(info->isolate(), zone(),
-                                           *info->context(), scope);
+      *scope = Scope::DeserializeScopeChain(info->isolate(), zone(),
+                                            *info->context(), *scope);
       // The Scope is backed up by ScopeInfo (which is in the V8 heap); this
       // means the Parser cannot operate independent of the V8 heap. Tell the
       // string table to internalize strings and values right after they're
@@ -971,27 +978,28 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
       DCHECK(parsing_on_main_thread_);
       ast_value_factory()->Internalize(info->isolate());
     }
-    original_scope_ = scope;
+    original_scope_ = *scope;
     if (info->is_eval()) {
-      if (!scope->is_script_scope() || is_strict(info->language_mode())) {
-        scope = NewScope(scope, EVAL_SCOPE);
+      if (!(*scope)->is_script_scope() || is_strict(info->language_mode())) {
+        *scope = NewScope(*scope, EVAL_SCOPE);
       }
     } else if (info->is_module()) {
-      scope = NewScope(scope, MODULE_SCOPE);
+      *scope = NewScope(*scope, MODULE_SCOPE);
     }
-
-    scope->set_start_position(0);
+    (*scope)->set_start_position(0);
+    // End position will be set by the caller.
 
     // Compute the parsing mode.
     Mode mode = (FLAG_lazy && allow_lazy()) ? PARSE_LAZILY : PARSE_EAGERLY;
-    if (allow_natives() || extension_ != NULL || scope->is_eval_scope()) {
+    if (allow_natives() || extension_ != NULL ||
+        (*scope)->is_eval_scope()) {
       mode = PARSE_EAGERLY;
     }
     ParsingModeScope parsing_mode(this, mode);
 
     // Enters 'scope'.
     AstNodeFactory function_factory(ast_value_factory());
-    FunctionState function_state(&function_state_, &scope_, scope,
+    FunctionState function_state(&function_state_, &scope_, *scope,
                                  kNormalFunction, &function_factory);
 
     scope_->SetLanguageMode(info->language_mode());
@@ -1002,14 +1010,9 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
       DCHECK(allow_harmony_modules());
       ParseModuleItemList(body, &ok);
     } else {
-      Scope* eval_scope = nullptr;
-      ParseStatementList(body, Token::EOS, info->is_eval(), &eval_scope, &ok);
-      if (eval_scope != nullptr)
-        eval_scope->set_end_position(scanner()->location().end_pos);
+      ParseStatementList(body, Token::EOS, info->is_eval(), eval_scope, &ok);
     }
 
-    scope->set_end_position(scanner()->location().end_pos);
-
     if (ok && is_strict(language_mode())) {
       CheckStrictOctalLiteral(beg_pos, scanner()->location().end_pos, &ok);
       CheckConflictingVarDeclarations(scope_, &ok);
@@ -5521,7 +5524,14 @@ void Parser::ParseOnBackground(ParseInfo* info) {
   // don't). We work around this by storing all the scopes which need their end
   // position set at the end of the script (the top scope and possible eval
   // scopes) and set their end position after we know the script length.
-  result = DoParseProgram(info);
+  Scope* top_scope = NULL;
+  Scope* eval_scope = NULL;
+  result = DoParseProgram(info, &top_scope, &eval_scope);
+
+  top_scope->set_end_position(scanner()->location().end_pos);
+  if (eval_scope != NULL) {
+    eval_scope->set_end_position(scanner()->location().end_pos);
+  }
 
   info->set_literal(result);
 

src/parser.h

@@ -884,7 +884,8 @@ class Parser : public ParserBase<ParserTraits> {
                              Utf16CharacterStream* source);
 
   // Called by ParseProgram after setting up the scanner.
-  FunctionLiteral* DoParseProgram(ParseInfo* info);
+  FunctionLiteral* DoParseProgram(ParseInfo* info, Scope** scope,
+                                  Scope** ad_hoc_eval_scope);
 
   void SetCachedData(ParseInfo* info);