[inspector] improve this value for arrow function in scopes

Currently we incorrectly show global object as arrow function receiver.
With this CL:
- if this is used inside of function we show correct this value,
- if this is unused and V8 optimizes it out - we show undefined.

Second is known issue which we should address separately.

R=dgozman@chromium.org,yangguo@chromium.org

Bug: chromium:552753
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Iac88a07fe622eb9b2f8af7ecbc4a32a56c8cdfaa
Reviewed-on: https://chromium-review.googlesource.com/723840
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48839}

src/debug/debug-interface.h

@@ -456,7 +456,7 @@ class StackTraceIterator {
   virtual void Advance() = 0;
 
   virtual int GetContextId() const = 0;
-  virtual v8::Local<v8::Value> GetReceiver() const = 0;
+  virtual v8::MaybeLocal<v8::Value> GetReceiver() const = 0;
   virtual v8::Local<v8::Value> GetReturnValue() const = 0;
   virtual v8::Local<v8::String> GetFunctionName() const = 0;
   virtual v8::Local<v8::debug::Script> GetScript() const = 0;

src/debug/debug-stack-trace-iterator.cc

@@ -78,13 +78,42 @@ int DebugStackTraceIterator::GetContextId() const {
   return 0;
 }
 
-v8::Local<v8::Value> DebugStackTraceIterator::GetReceiver() const {
+v8::MaybeLocal<v8::Value> DebugStackTraceIterator::GetReceiver() const {
   DCHECK(!Done());
+  if (frame_inspector_->IsJavaScript() &&
+      frame_inspector_->GetFunction()->shared()->kind() == kArrowFunction) {
+    // FrameInspector is not able to get receiver for arrow function.
+    // So let's try to fetch it using same logic as is used to retrieve 'this'
+    // during DebugEvaluate::Local.
+    Handle<JSFunction> function = frame_inspector_->GetFunction();
+    Handle<Context> context(function->context());
+    // Arrow function defined in top level function without references to
+    // variables may have NativeContext as context.
+    if (!context->IsFunctionContext()) return v8::MaybeLocal<v8::Value>();
+    ScopeIterator scope_iterator(isolate_, frame_inspector_.get(),
+                                 ScopeIterator::COLLECT_NON_LOCALS);
+    // We lookup this variable in function context only when it is used in arrow
+    // function otherwise V8 can optimize it out.
+    if (!scope_iterator.GetNonLocals()->Has(isolate_->factory()->this_string()))
+      return v8::MaybeLocal<v8::Value>();
+
+    Handle<ScopeInfo> scope_info(context->scope_info());
+    VariableMode mode;
+    InitializationFlag flag;
+    MaybeAssignedFlag maybe_assigned_flag;
+    int slot_index = ScopeInfo::ContextSlotIndex(
+        scope_info, isolate_->factory()->this_string(), &mode, &flag,
+        &maybe_assigned_flag);
+    if (slot_index < 0) return v8::MaybeLocal<v8::Value>();
+    Handle<Object> value = handle(context->get(slot_index), isolate_);
+    if (value->IsTheHole(isolate_)) return v8::MaybeLocal<v8::Value>();
+    return Utils::ToLocal(value);
+  }
   Handle<Object> value = frame_inspector_->GetReceiver();
   if (value.is_null() || (value->IsSmi() || !value->IsTheHole(isolate_))) {
     return Utils::ToLocal(value);
   }
-  return v8::Undefined(reinterpret_cast<v8::Isolate*>(isolate_));
+  return v8::MaybeLocal<v8::Value>();
 }
 
 v8::Local<v8::Value> DebugStackTraceIterator::GetReturnValue() const {

src/debug/debug-stack-trace-iterator.h

@@ -21,7 +21,7 @@ class DebugStackTraceIterator final : public debug::StackTraceIterator {
   void Advance() override;
 
   int GetContextId() const override;
-  v8::Local<v8::Value> GetReceiver() const override;
+  v8::MaybeLocal<v8::Value> GetReceiver() const override;
   v8::Local<v8::Value> GetReturnValue() const override;
   v8::Local<v8::String> GetFunctionName() const override;
   v8::Local<v8::debug::Script> GetScript() const override;

src/inspector/v8-debugger-agent-impl.cc

@@ -1177,16 +1177,19 @@ Response V8DebuggerAgentImpl::currentCallFrames(
     Response res = buildScopes(scopeIterator.get(), injectedScript, &scopes);
     if (!res.isSuccess()) return res;
 
-    std::unique_ptr<RemoteObject> receiver;
+    std::unique_ptr<RemoteObject> protocolReceiver;
     if (injectedScript) {
-      res = injectedScript->wrapObject(iterator->GetReceiver(),
-                                       kBacktraceObjectGroup, false, false,
-                                       &receiver);
-      if (!res.isSuccess()) return res;
-    } else {
-      receiver = RemoteObject::create()
-                     .setType(RemoteObject::TypeEnum::Undefined)
-                     .build();
+      v8::Local<v8::Value> receiver;
+      if (iterator->GetReceiver().ToLocal(&receiver)) {
+        res = injectedScript->wrapObject(receiver, kBacktraceObjectGroup, false,
+                                         false, &protocolReceiver);
+        if (!res.isSuccess()) return res;
+      }
+    }
+    if (!protocolReceiver) {
+      protocolReceiver = RemoteObject::create()
+                             .setType(RemoteObject::TypeEnum::Undefined)
+                             .build();
     }
 
     v8::Local<v8::debug::Script> script = iterator->GetScript();
@@ -1213,7 +1216,7 @@ Response V8DebuggerAgentImpl::currentCallFrames(
             .setLocation(std::move(location))
             .setUrl(url)
             .setScopeChain(std::move(scopes))
-            .setThis(std::move(receiver))
+            .setThis(std::move(protocolReceiver))
             .build();
 
     v8::Local<v8::Function> func = iterator->GetFunction();

test/inspector/debugger/this-in-arrow-function-expected.txt

+Checks this in arrow function scope
+    (function() {
+      let f = () => { #debugger; };
+      f();
+
+This on callFrame:
+{
+    type : undefined
+}
+This in evaluateOnCallFrame:
+{
+    type : undefined
+}
+Values equal: true
+
+      let f = () => { debugger; };
+      #f();
+    }).call('a');
+
+This on callFrame:
+{
+    className : String
+    description : String
+    objectId : <objectId>
+    type : object
+}
+This in evaluateOnCallFrame:
+{
+    className : String
+    description : String
+    objectId : <objectId>
+    type : object
+}
+Values equal: true
+
+      f();
+    }).#call('a');
+    return a;
+
+This on callFrame:
+{
+    className : Number
+    description : Number
+    objectId : <objectId>
+    type : object
+}
+This in evaluateOnCallFrame:
+{
+    className : Number
+    description : Number
+    objectId : <objectId>
+    type : object
+}
+Values equal: true
+
+function boo() {
+  foo.call(1)#();
+}
+
+This on callFrame:
+{
+    className : Object
+    description : Object
+    objectId : <objectId>
+    type : object
+}
+This in evaluateOnCallFrame:
+{
+    className : Object
+    description : Object
+    objectId : <objectId>
+    type : object
+}
+Values equal: true
+
+}
+(() => boo.#call({}))();
+
+This on callFrame:
+{
+    type : undefined
+}
+This in evaluateOnCallFrame:
+{
+    type : undefined
+}
+Values equal: true
+
+}
+(() => boo.call({}))#();
+
+This on callFrame:
+{
+    className : global
+    description : global
+    objectId : <objectId>
+    type : object
+}
+This in evaluateOnCallFrame:
+{
+    className : Object
+    description : Object
+    objectId : <objectId>
+    type : object
+}
+Values equal: false
+

test/inspector/debugger/this-in-arrow-function.js

+// Copyright 2017 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.
+
+let {session, contextGroup, Protocol} =
+    InspectorTest.start('Checks this in arrow function scope');
+
+(async function test() {
+  session.setupScriptMap();
+  Protocol.Debugger.enable();
+  Protocol.Runtime.evaluate({expression: `
+function foo() {
+  return () => {
+    let a = this;
+    (function() {
+      let f = () => { debugger; };
+      f();
+    }).call('a');
+    return a;
+  };
+}
+function boo() {
+  foo.call(1)();
+}
+(() => boo.call({}))();`
+  });
+  let {params:{callFrames}} = await Protocol.Debugger.oncePaused();
+  for (let callFrame of callFrames) {
+    await session.logSourceLocation(callFrame.location);
+
+    InspectorTest.log('This on callFrame:');
+    InspectorTest.logMessage(callFrame.this);
+    let {result:{result}} = await Protocol.Debugger.evaluateOnCallFrame({
+      callFrameId: callFrame.callFrameId,
+      expression: 'this'
+    });
+    InspectorTest.log('This in evaluateOnCallFrame:');
+    InspectorTest.logMessage(result);
+
+    if (callFrame.this.type === 'undefined' || result.type === 'undefined') {
+      InspectorTest.log('Values equal: ' + (callFrame.this.type === result.type) + '\n');
+      continue;
+    }
+
+    let {result:{result:{value}}} = await Protocol.Runtime.callFunctionOn({
+      functionDeclaration: 'function equal(a) { return this === a; }',
+      objectId: callFrame.this.objectId,
+      arguments: [ result.value ? {value: result.value} : {objectId: result.objectId}],
+      returnByValue: true
+    });
+    InspectorTest.log('Values equal: ' + value + '\n');
+  }
+  InspectorTest.completeTest();
+})();