[debugger] keep scope and context chain for hidden scopes in sync.

R=jgruber@chromium.org
BUG=chromium:609046
LOG=N

Review-Url: https://codereview.chromium.org/1957303002
Cr-Commit-Position: refs/heads/master@{#36160}

src/debug/debug-scopes.cc

@@ -201,11 +201,15 @@ void ScopeIterator::Next() {
   } else if (nested_scope_chain_.is_empty()) {
     context_ = Handle<Context>(context_->previous(), isolate_);
   } else {
-    if (nested_scope_chain_.last().scope_info->HasContext()) {
-      DCHECK(context_->previous() != NULL);
-      context_ = Handle<Context>(context_->previous(), isolate_);
-    }
-    nested_scope_chain_.RemoveLast();
+    do {
+      if (nested_scope_chain_.last().scope_info->HasContext()) {
+        DCHECK(context_->previous() != NULL);
+        context_ = Handle<Context>(context_->previous(), isolate_);
+      }
+      nested_scope_chain_.RemoveLast();
+      if (nested_scope_chain_.is_empty()) break;
+      // Repeat to skip hidden scopes.
+    } while (nested_scope_chain_.last().is_hidden());
   }
   UnwrapEvaluationContext();
 }
@@ -796,10 +800,16 @@ bool ScopeIterator::CopyContextExtensionToScopeObject(
 
 void ScopeIterator::GetNestedScopeChain(Isolate* isolate, Scope* scope,
                                         int position) {
-  if (!scope->is_eval_scope() && !scope->is_hidden()) {
-    nested_scope_chain_.Add(ExtendedScopeInfo(scope->GetScopeInfo(isolate),
-                                              scope->start_position(),
-                                              scope->end_position()));
+  if (!scope->is_eval_scope()) {
+    if (scope->is_hidden()) {
+      // We need to add this chain element in case the scope has a context
+      // associated. We need to keep the scope chain and context chain in sync.
+      nested_scope_chain_.Add(ExtendedScopeInfo(scope->GetScopeInfo(isolate)));
+    } else {
+      nested_scope_chain_.Add(ExtendedScopeInfo(scope->GetScopeInfo(isolate),
+                                                scope->start_position(),
+                                                scope->end_position()));
+    }
   }
   for (int i = 0; i < scope->inner_scopes()->length(); i++) {
     Scope* inner_scope = scope->inner_scopes()->at(i);

src/debug/debug-scopes.h

@@ -85,9 +85,12 @@ class ScopeIterator {
   struct ExtendedScopeInfo {
     ExtendedScopeInfo(Handle<ScopeInfo> info, int start, int end)
         : scope_info(info), start_position(start), end_position(end) {}
+    explicit ExtendedScopeInfo(Handle<ScopeInfo> info)
+        : scope_info(info), start_position(-1), end_position(-1) {}
     Handle<ScopeInfo> scope_info;
     int start_position;
     int end_position;
+    bool is_hidden() { return start_position == -1 && end_position == -1; }
   };
 
   Isolate* isolate_;

test/mjsunit/es6/regress/regress-468661.js

@@ -9,7 +9,7 @@ var exception = null;
 var break_count = 0;
 
 var expected_values =
-  [ReferenceError, ReferenceError, 0, 0, 0, 0, 1,
+  [ReferenceError, undefined, 0, 0, 0, 0, 1,
    ReferenceError, ReferenceError];
 
 function listener(event, exec_state, event_data, data) {

test/mjsunit/regress/regress-crbug-609046.js

+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug
+
+// Test that hidden scopes are correctly walked in the scope chain.
+
+var Debug = debug.Debug;
+var exception = null;
+var delegate = null;
+var done = false;
+
+function listener(event, exec_state, event_data, data) {
+  if (event != Debug.DebugEvent.Break) return;
+  try {
+    assertEquals([ debug.ScopeType.Block,
+                   debug.ScopeType.Script,
+                   debug.ScopeType.Global ],
+                 exec_state.frame(0).allScopes().map(s => s.scopeType()));
+    done = true;
+  } catch (e) {
+    exception = e;
+  }
+}
+
+Debug.setListener(listener);
+
+for(let a = 0; a < 3; a++) {
+  debugger;
+  eval();  // Force context-allocation of everything.
+}
+
+Debug.setListener(null);
+assertNull(exception);
+assertTrue(done);