[inspector] Side-effect free FunctionMirror descriptions.

Drive-by-fix: Remove command line API fn.toString() override, which was still in place from the early days when much of the inspector was implemented in JavaScript. Fixed: chromium:1207867 Bug: chromium:1206620 Change-Id: I8429f109da5f021f729f184fd824160a24e60897 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2887508 Auto-Submit: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Benedikt Meurer <bmeurer@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Reviewed-by: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#74516}
2021-05-12 09:40:56 +02:00 · 2021-05-12 09:40:56 +02:00 · 203f527619
commit 203f527619
parent ba6a1a7c34
8 changed files with 157 additions and 80 deletions
--- a/src/debug/debug-interface.cc
+++ b/src/debug/debug-interface.cc
@ -58,6 +58,20 @@ Local<String> GetFunctionDebugName(Local<StackFrame> frame) {
  return frame->GetFunctionName();
 }

+Local<String> GetFunctionDescription(Local<Function> function) {
+  auto receiver = Utils::OpenHandle(*function);
+  if (receiver->IsJSBoundFunction()) {
+    return Utils::ToLocal(i::JSBoundFunction::ToString(
+        i::Handle<i::JSBoundFunction>::cast(receiver)));
+  }
+  if (receiver->IsJSFunction()) {
+    return Utils::ToLocal(
+        i::JSFunction::ToString(i::Handle<i::JSFunction>::cast(receiver)));
+  }
+  return Utils::ToLocal(
+      receiver->GetIsolate()->factory()->function_native_code_string());
+}
+
 void SetBreakOnNextFunctionCall(Isolate* isolate) {
  reinterpret_cast<i::Isolate*>(isolate)->debug()->SetBreakOnNextFunctionCall();
 }
--- a/src/debug/debug-interface.h
+++ b/src/debug/debug-interface.h
@ -45,6 +45,9 @@ v8_inspector::V8Inspector* GetInspector(Isolate* isolate);
 // preceeded by a dollar ('$').
 Local<String> GetFunctionDebugName(Local<StackFrame> frame);

+// Returns a debug string representation of the function.
+Local<String> GetFunctionDescription(Local<Function> function);
+
 // Schedule a debugger break to happen when function is called inside given
 // isolate.
 V8_EXPORT_PRIVATE void SetBreakOnNextFunctionCall(Isolate* isolate);
--- a/src/inspector/v8-console.cc
+++ b/src/inspector/v8-console.cc
@ -162,14 +162,9 @@ class ConsoleHelper {
  int m_groupId;
 };

-void returnDataCallback(const v8::FunctionCallbackInfo<v8::Value>& info) {
-  info.GetReturnValue().Set(info.Data());
-}
-
 void createBoundFunctionProperty(
    v8::Local<v8::Context> context, v8::Local<v8::Object> console,
    v8::Local<v8::Value> data, const char* name, v8::FunctionCallback callback,
-    const char* description = nullptr,
    v8::SideEffectType side_effect_type = v8::SideEffectType::kHasSideEffect) {
  v8::Local<v8::String> funcName =
      toV8StringInternalized(context->GetIsolate(), name);
@ -179,18 +174,6 @@ void createBoundFunctionProperty(
           .ToLocal(&func))
    return;
  func->SetName(funcName);
-  if (description) {
-    v8::Local<v8::String> returnValue =
-        toV8String(context->GetIsolate(), description);
-    v8::Local<v8::Function> toStringFunction;
-    if (v8::Function::New(context, returnDataCallback, returnValue, 0,
-                          v8::ConstructorBehavior::kThrow,
-                          v8::SideEffectType::kHasNoSideEffect)
-            .ToLocal(&toStringFunction))
-      createDataProperty(context, func, toV8StringInternalized(
-                                            context->GetIsolate(), "toString"),
-                         toStringFunction);
-  }
  createDataProperty(context, console, funcName, func);
 }

@ -697,80 +680,62 @@ v8::Local<v8::Object> V8Console::createCommandLineAPI(
  *static_cast<CommandLineAPIData*>(data->GetBackingStore()->Data()) =
      CommandLineAPIData(this, sessionId);
  createBoundFunctionProperty(context, commandLineAPI, data, "dir",
-                              &V8Console::call<&V8Console::Dir>,
-                              "function dir(value) { [Command Line API] }");
+                              &V8Console::call<&V8Console::Dir>);
  createBoundFunctionProperty(context, commandLineAPI, data, "dirxml",
-                              &V8Console::call<&V8Console::DirXml>,
-                              "function dirxml(value) { [Command Line API] }");
+                              &V8Console::call<&V8Console::DirXml>);
  createBoundFunctionProperty(context, commandLineAPI, data, "profile",
-                              &V8Console::call<&V8Console::Profile>,
-                              "function profile(title) { [Command Line API] }");
-  createBoundFunctionProperty(
-      context, commandLineAPI, data, "profileEnd",
-      &V8Console::call<&V8Console::ProfileEnd>,
-      "function profileEnd(title) { [Command Line API] }");
+                              &V8Console::call<&V8Console::Profile>);
+  createBoundFunctionProperty(context, commandLineAPI, data, "profileEnd",
+                              &V8Console::call<&V8Console::ProfileEnd>);
  createBoundFunctionProperty(context, commandLineAPI, data, "clear",
-                              &V8Console::call<&V8Console::Clear>,
-                              "function clear() { [Command Line API] }");
-  createBoundFunctionProperty(
-      context, commandLineAPI, data, "table",
-      &V8Console::call<&V8Console::Table>,
-      "function table(data, [columns]) { [Command Line API] }");
+                              &V8Console::call<&V8Console::Clear>);
+  createBoundFunctionProperty(context, commandLineAPI, data, "table",
+                              &V8Console::call<&V8Console::Table>);

  createBoundFunctionProperty(context, commandLineAPI, data, "keys",
                              &V8Console::call<&V8Console::keysCallback>,
-                              "function keys(object) { [Command Line API] }",
                              v8::SideEffectType::kHasNoSideEffect);
  createBoundFunctionProperty(context, commandLineAPI, data, "values",
                              &V8Console::call<&V8Console::valuesCallback>,
-                              "function values(object) { [Command Line API] }",
                              v8::SideEffectType::kHasNoSideEffect);
  createBoundFunctionProperty(
      context, commandLineAPI, data, "debug",
-      &V8Console::call<&V8Console::debugFunctionCallback>,
-      "function debug(function, condition) { [Command Line API] }");
+      &V8Console::call<&V8Console::debugFunctionCallback>);
  createBoundFunctionProperty(
      context, commandLineAPI, data, "undebug",
-      &V8Console::call<&V8Console::undebugFunctionCallback>,
-      "function undebug(function) { [Command Line API] }");
+      &V8Console::call<&V8Console::undebugFunctionCallback>);
  createBoundFunctionProperty(
      context, commandLineAPI, data, "monitor",
-      &V8Console::call<&V8Console::monitorFunctionCallback>,
-      "function monitor(function) { [Command Line API] }");
+      &V8Console::call<&V8Console::monitorFunctionCallback>);
  createBoundFunctionProperty(
      context, commandLineAPI, data, "unmonitor",
-      &V8Console::call<&V8Console::unmonitorFunctionCallback>,
-      "function unmonitor(function) { [Command Line API] }");
-  createBoundFunctionProperty(
-      context, commandLineAPI, data, "inspect",
-      &V8Console::call<&V8Console::inspectCallback>,
-      "function inspect(object) { [Command Line API] }");
+      &V8Console::call<&V8Console::unmonitorFunctionCallback>);
+  createBoundFunctionProperty(context, commandLineAPI, data, "inspect",
+                              &V8Console::call<&V8Console::inspectCallback>);
  createBoundFunctionProperty(context, commandLineAPI, data, "copy",
-                              &V8Console::call<&V8Console::copyCallback>,
-                              "function copy(value) { [Command Line API] }");
+                              &V8Console::call<&V8Console::copyCallback>);
  createBoundFunctionProperty(
      context, commandLineAPI, data, "queryObjects",
-      &V8Console::call<&V8Console::queryObjectsCallback>,
-      "function queryObjects(constructor) { [Command Line API] }");
+      &V8Console::call<&V8Console::queryObjectsCallback>);
  createBoundFunctionProperty(
      context, commandLineAPI, data, "$_",
-      &V8Console::call<&V8Console::lastEvaluationResultCallback>, nullptr,
+      &V8Console::call<&V8Console::lastEvaluationResultCallback>,
      v8::SideEffectType::kHasNoSideEffect);
  createBoundFunctionProperty(context, commandLineAPI, data, "$0",
                              &V8Console::call<&V8Console::inspectedObject0>,
-                              nullptr, v8::SideEffectType::kHasNoSideEffect);
+                              v8::SideEffectType::kHasNoSideEffect);
  createBoundFunctionProperty(context, commandLineAPI, data, "$1",
                              &V8Console::call<&V8Console::inspectedObject1>,
-                              nullptr, v8::SideEffectType::kHasNoSideEffect);
+                              v8::SideEffectType::kHasNoSideEffect);
  createBoundFunctionProperty(context, commandLineAPI, data, "$2",
                              &V8Console::call<&V8Console::inspectedObject2>,
-                              nullptr, v8::SideEffectType::kHasNoSideEffect);
+                              v8::SideEffectType::kHasNoSideEffect);
  createBoundFunctionProperty(context, commandLineAPI, data, "$3",
                              &V8Console::call<&V8Console::inspectedObject3>,
-                              nullptr, v8::SideEffectType::kHasNoSideEffect);
+                              v8::SideEffectType::kHasNoSideEffect);
  createBoundFunctionProperty(context, commandLineAPI, data, "$4",
                              &V8Console::call<&V8Console::inspectedObject4>,
-                              nullptr, v8::SideEffectType::kHasNoSideEffect);
+                              v8::SideEffectType::kHasNoSideEffect);

  m_inspector->client()->installAdditionalCommandLineAPI(context,
                                                         commandLineAPI);
--- a/src/inspector/value-mirror.cc
+++ b/src/inspector/value-mirror.cc
@ -349,14 +349,9 @@ String16 descriptionForEntry(v8::Local<v8::Context> context,
  return key.length() ? ("{" + key + " => " + value + "}") : value;
 }

-String16 descriptionForFunction(v8::Local<v8::Context> context,
-                                v8::Local<v8::Function> value) {
-  v8::Isolate* isolate = context->GetIsolate();
-  v8::TryCatch tryCatch(isolate);
-  v8::Local<v8::String> description;
-  if (!value->ToString(context).ToLocal(&description)) {
-    return descriptionForObject(isolate, value);
-  }
+String16 descriptionForFunction(v8::Local<v8::Function> value) {
+  v8::Isolate* isolate = value->GetIsolate();
+  v8::Local<v8::String> description = v8::debug::GetFunctionDescription(value);
  return toProtocolString(isolate, description);
 }

@ -646,7 +641,7 @@ class FunctionMirror final : public ValueMirror {
                    .setType(RemoteObject::TypeEnum::Function)
                    .setClassName(toProtocolStringWithTypeCheck(
                        context->GetIsolate(), m_value->GetConstructorName()))
-                    .setDescription(descriptionForFunction(context, m_value))
+                    .setDescription(descriptionForFunction(m_value))
                    .build();
    }
    return Response::Success();
@ -667,7 +662,7 @@ class FunctionMirror final : public ValueMirror {
    *preview =
        ObjectPreview::create()
            .setType(RemoteObject::TypeEnum::Function)
-            .setDescription(descriptionForFunction(context, m_value))
+            .setDescription(descriptionForFunction(m_value))
            .setOverflow(false)
            .setProperties(std::make_unique<protocol::Array<PropertyPreview>>())
            .build();
--- a/test/inspector/regress/regress-crbug-1207867-expected.txt
+++ b/test/inspector/regress/regress-crbug-1207867-expected.txt
@ -0,0 +1,41 @@
+Regression test for crbug/1207867
+
+Running test: testFunctionDescription
+{
+    className : Function
+    description : function fun(x) { return x; }
+    objectId : <objectId>
+    type : function
+}
+
+Running test: testArrowFunctionDescription
+{
+    className : Function
+    description : x => x
+    objectId : <objectId>
+    type : function
+}
+
+Running test: testBoundFunctionDescription
+{
+    className : Function
+    description : function () { [native code] }
+    objectId : <objectId>
+    type : function
+}
+
+Running test: testAsyncFunctionDescription
+{
+    className : AsyncFunction
+    description : async function afun(x) { await x; }
+    objectId : <objectId>
+    type : function
+}
+
+Running test: testNativeFunctionDescription
+{
+    className : Function
+    description : function map() { [native code] }
+    objectId : <objectId>
+    type : function
+}
--- a/test/inspector/regress/regress-crbug-1207867.js
+++ b/test/inspector/regress/regress-crbug-1207867.js
@ -0,0 +1,59 @@
+// Copyright 2021 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('Regression test for crbug/1207867');
+
+contextGroup.addScript(`
+function fun(x) { return x; }
+fun.toString = fn => 'bar';
+
+const arrow = x => x;
+arrow.toString = fn => 'baz';
+
+const bound = fun.bind(this);
+bound.toString = fn => 'foo';
+
+async function afun(x) { await x; }
+afun.toString = fn => 'blah';
+
+const native = Array.prototype.map;
+native.toString = fn => 'meh';
+`);
+
+InspectorTest.runAsyncTestSuite([
+  async function testFunctionDescription() {
+    await Protocol.Runtime.enable();
+    const {result: {result}} = await Protocol.Runtime.evaluate({expression: 'fun'});
+    InspectorTest.logMessage(result);
+    await Protocol.Runtime.disable();
+  },
+
+  async function testArrowFunctionDescription() {
+    await Protocol.Runtime.enable();
+    const {result: {result}} = await Protocol.Runtime.evaluate({expression: 'arrow'});
+    InspectorTest.logMessage(result);
+    await Protocol.Runtime.disable();
+  },
+
+  async function testBoundFunctionDescription() {
+    await Protocol.Runtime.enable();
+    const {result: {result}} = await Protocol.Runtime.evaluate({expression: 'bound'});
+    InspectorTest.logMessage(result);
+    await Protocol.Runtime.disable();
+  },
+
+  async function testAsyncFunctionDescription() {
+    await Protocol.Runtime.enable();
+    const {result: {result}} = await Protocol.Runtime.evaluate({expression: 'afun'});
+    InspectorTest.logMessage(result);
+    await Protocol.Runtime.disable();
+  },
+
+  async function testNativeFunctionDescription() {
+    await Protocol.Runtime.enable();
+    const {result: {result}} = await Protocol.Runtime.evaluate({expression: 'native'});
+    InspectorTest.logMessage(result);
+    await Protocol.Runtime.disable();
+  }
+]);
--- a/test/inspector/runtime/clear-of-command-line-api.js
+++ b/test/inspector/runtime/clear-of-command-line-api.js
@ -13,7 +13,7 @@ function presentedAPIMethods()
    var methodCount = 0;
    for (var method of methods) {
        try {
-            if (eval("window." + method + "&&" + method + ".toString ? " + method + ".toString().indexOf(\\"[Command Line API]\\") !== -1 : false"))
+            if (eval("window." + method + "&&" + method + ".toString ? " + method + ".toString().indexOf(\\"[native code]\\") !== -1 : false"))
                ++methodCount;
        } catch (e) {
        }
--- a/test/inspector/runtime/command-line-api-expected.txt
+++ b/test/inspector/runtime/command-line-api-expected.txt
@ -6,7 +6,7 @@ Running test: testKeys
    result : {
        result : {
            className : Function
-            description : function keys(object) { [Command Line API] }
+            description : function keys() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -114,7 +114,7 @@ Running test: testQueryObjects
    result : {
        result : {
            className : Function
-            description : function queryObjects(constructor) { [Command Line API] }
+            description : function queryObjects() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -235,7 +235,7 @@ Running test: testDebug
    result : {
        result : {
            className : Function
-            description : function debug(function, condition) { [Command Line API] }
+            description : function debug() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -246,7 +246,7 @@ Running test: testDebug
    result : {
        result : {
            className : Function
-            description : function undebug(function) { [Command Line API] }
+            description : function undebug() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -295,7 +295,7 @@ Running test: testMonitor
    result : {
        result : {
            className : Function
-            description : function monitor(function) { [Command Line API] }
+            description : function monitor() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -306,7 +306,7 @@ Running test: testMonitor
    result : {
        result : {
            className : Function
-            description : function unmonitor(function) { [Command Line API] }
+            description : function unmonitor() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -325,7 +325,7 @@ Running test: testProfile
    result : {
        result : {
            className : Function
-            description : function profile(title) { [Command Line API] }
+            description : function profile() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -336,7 +336,7 @@ Running test: testProfile
    result : {
        result : {
            className : Function
-            description : function profileEnd(title) { [Command Line API] }
+            description : function profileEnd() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -399,7 +399,7 @@ Running test: testDir
    result : {
        result : {
            className : Function
-            description : function dir(value) { [Command Line API] }
+            description : function dir() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -506,7 +506,7 @@ Running test: testDirXML
    result : {
        result : {
            className : Function
-            description : function dirxml(value) { [Command Line API] }
+            description : function dirxml() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -579,7 +579,7 @@ Running test: testTable
    result : {
        result : {
            className : Function
-            description : function table(data, [columns]) { [Command Line API] }
+            description : function table() { [native code] }
            objectId : <objectId>
            type : function
        }
@ -652,7 +652,7 @@ Running test: testClear
    result : {
        result : {
            className : Function
-            description : function clear() { [Command Line API] }
+            description : function clear() { [native code] }
            objectId : <objectId>
            type : function
        }