Don't expose GetFunction on the scope iterator. Simply take it into account...

Don't expose GetFunction on the scope iterator. Simply take it into account for GetFunctionDebugName

This is a step towards avoiding materializing function_ altogether if we deoptimize. Typically we only need the SharedFunctionInfo.

Cq-Include-Trybots: luci.chromium.try:linux_chromium_headless_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Idee78f02d1afe3d2cb70e93a6d96a5a33907f892
Reviewed-on: https://chromium-review.googlesource.com/1100474
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53789}

src/debug/debug-interface.h

@@ -398,7 +398,6 @@ class ScopeIterator {
   virtual void Advance() = 0;
   virtual ScopeType GetType() = 0;
   virtual v8::Local<v8::Object> GetObject() = 0;
-  virtual v8::Local<v8::Function> GetFunction() = 0;
   virtual v8::Local<v8::Value> GetFunctionDebugName() = 0;
   virtual int GetScriptId() = 0;
   virtual bool HasLocationInfo() = 0;

src/debug/debug-scope-iterator.cc

@@ -90,12 +90,6 @@ v8::Local<v8::Object> DebugScopeIterator::GetObject() {
   return Utils::ToLocal(value);
 }
 
-v8::Local<v8::Function> DebugScopeIterator::GetFunction() {
-  DCHECK(!Done());
-  Handle<JSFunction> closure = iterator_.GetFunction();
-  if (closure.is_null()) return v8::Local<v8::Function>();
-  return Utils::ToLocal(closure);
-}
 int DebugScopeIterator::GetScriptId() {
   DCHECK(!Done());
   return iterator_.GetScript()->id();
@@ -180,11 +174,6 @@ int DebugWasmScopeIterator::GetScriptId() {
   return -1;
 }
 
-v8::Local<v8::Function> DebugWasmScopeIterator::GetFunction() {
-  DCHECK(!Done());
-  return v8::Local<v8::Function>();
-}
-
 v8::Local<v8::Value> DebugWasmScopeIterator::GetFunctionDebugName() {
   DCHECK(!Done());
   return Utils::ToLocal(isolate_->factory()->empty_string());

src/debug/debug-scope-iterator.h

@@ -23,7 +23,6 @@ class DebugScopeIterator final : public debug::ScopeIterator {
   void Advance() override;
   ScopeType GetType() override;
   v8::Local<v8::Object> GetObject() override;
-  v8::Local<v8::Function> GetFunction() override;
   v8::Local<v8::Value> GetFunctionDebugName() override;
   int GetScriptId() override;
   bool HasLocationInfo() override;
@@ -48,7 +47,6 @@ class DebugWasmScopeIterator final : public debug::ScopeIterator {
   void Advance() override;
   ScopeType GetType() override;
   v8::Local<v8::Object> GetObject() override;
-  v8::Local<v8::Function> GetFunction() override;
   v8::Local<v8::Value> GetFunctionDebugName() override;
   int GetScriptId() override;
   bool HasLocationInfo() override;

src/debug/debug-scopes.cc

@@ -41,6 +41,8 @@ ScopeIterator::ScopeIterator(Isolate* isolate, FrameInspector* frame_inspector,
 ScopeIterator::~ScopeIterator() { delete info_; }
 
 Handle<Object> ScopeIterator::GetFunctionDebugName() const {
+  if (!function_.is_null()) return JSFunction::GetDebugName(function_);
+
   if (!context_->IsNativeContext()) {
     DisallowHeapAllocation no_gc;
     ScopeInfo* closure_info = context_->closure_context()->scope_info();

src/debug/debug-scopes.h

@@ -80,8 +80,6 @@ class ScopeIterator {
   // Populate the set with collected non-local variable names.
   Handle<StringSet> GetNonLocals();
 
-  // Return function which represents closure for current scope.
-  Handle<JSFunction> GetFunction() { return function_; }
   // Similar to JSFunction::GetName return the function's name or it's inferred
   // name.
   Handle<Object> GetFunctionDebugName() const;

src/inspector/v8-debugger.cc

@@ -718,14 +718,9 @@ v8::MaybeLocal<v8::Value> V8Debugger::getTargetScopes(
     }
     String16 type = v8_inspector::scopeType(iterator->GetType());
     String16 name;
-    v8::Local<v8::Function> closure = iterator->GetFunction();
-    if (!closure.IsEmpty()) {
-      name = toProtocolStringWithTypeCheck(closure->GetDebugName());
-    } else {
-      v8::Local<v8::Value> maybe_name = iterator->GetFunctionDebugName();
-      if (!maybe_name->IsUndefined()) {
-        name = toProtocolStringWithTypeCheck(maybe_name);
-      }
+    v8::Local<v8::Value> maybe_name = iterator->GetFunctionDebugName();
+    if (!maybe_name->IsUndefined()) {
+      name = toProtocolStringWithTypeCheck(maybe_name);
     }
     v8::Local<v8::Object> object = iterator->GetObject();
     createDataProperty(context, scope,

test/cctest/test-debug.cc

@@ -2782,7 +2782,6 @@ class DebugScopingListener : public v8::debug::DebugDelegate {
 
     scopes->Advance();
     CHECK_EQ(v8::debug::ScopeIterator::ScopeTypeGlobal, scopes->GetType());
-    CHECK(scopes->GetFunction().IsEmpty());
 
     scopes->Advance();
     CHECK(scopes->Done());

test/inspector/debugger/evaluate-on-call-frame-in-module-expected.txt

@@ -4,7 +4,7 @@ Running test: testTotal
 foo1 (module1:7:2)
 foo2 (module2:6:9)
 (anonymous) (module3:4:0)
-local
+local:foo1
 [
     [0] : c1 = 12
     [1] : g1 = 2
@@ -94,7 +94,7 @@ foo1 =
     objectId : <objectId>
     type : function
 }
-local
+local:foo2
 [
     [0] : c2 = 22
 ]
@@ -239,7 +239,7 @@ Running test: testAnother
 (anonymous) (module4:5:13)
 bar (module4:5:24)
 (anonymous) (module4:7:0)
-local:bar
+local
 [
 ]
 closure:bar