[inspector] Improve class name inference.

Previously having a `Symbol.toStringTag` property holding a string
somewhere in the prototype chain would always take predence over trying
to determine a reasonable name from the "constructor" (in case of
subclassing). This would lead to confusing behavior when diagnosing
objects in DevTools, for example

```js
class A extends URLSearchParams {};
new A()
```

would show `URLSearchParam` as class name for the instance rather than
`A`.

With this CL, we change the lookup logic to explicitly check for
`Symbol.toStringTag` and "constructor" along each step of the prototype
chain (skipping the "constructor" for the leaf object) and pick the
first one that yields a string (that is the function debug name in case
of "constructor").

Fixed: chromium:980018
Change-Id: Ic920b4bae02f965bc383c711f8de89c0de55fcac
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3231078
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Kim-Anh Tran <kimanh@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77453}

src/objects/js-objects.cc

@@ -500,36 +500,46 @@ std::pair<MaybeHandle<JSFunction>, Handle<String>> GetConstructorHelper(
     }
   }
 
-  LookupIterator it_tag(isolate, receiver,
-                        isolate->factory()->to_string_tag_symbol(), receiver,
-                        LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
-  Handle<Object> maybe_tag = JSReceiver::GetDataProperty(
-      &it_tag, AllocationPolicy::kAllocationDisallowed);
-  if (maybe_tag->IsString()) {
-    return std::make_pair(MaybeHandle<JSFunction>(),
-                          Handle<String>::cast(maybe_tag));
-  }
-
-  PrototypeIterator iter(isolate, receiver);
-  if (iter.IsAtEnd()) {
-    return std::make_pair(MaybeHandle<JSFunction>(),
-                          handle(receiver->class_name(), isolate));
-  }
-
-  Handle<JSReceiver> start = PrototypeIterator::GetCurrent<JSReceiver>(iter);
-  LookupIterator it(isolate, receiver, isolate->factory()->constructor_string(),
-                    start, LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
-  Handle<Object> maybe_constructor =
-      JSReceiver::GetDataProperty(&it, AllocationPolicy::kAllocationDisallowed);
-  if (maybe_constructor->IsJSFunction()) {
-    Handle<JSFunction> constructor =
-        Handle<JSFunction>::cast(maybe_constructor);
-    Handle<String> name =
-        SharedFunctionInfo::DebugName(handle(constructor->shared(), isolate));
-
-    if (name->length() != 0 &&
-        !name->Equals(ReadOnlyRoots(isolate).Object_string())) {
-      return std::make_pair(constructor, name);
+  for (PrototypeIterator it(isolate, receiver, kStartAtReceiver); !it.IsAtEnd();
+       it.AdvanceIgnoringProxies()) {
+    auto current = PrototypeIterator::GetCurrent<JSReceiver>(it);
+
+    LookupIterator it_to_string_tag(
+        isolate, receiver, isolate->factory()->to_string_tag_symbol(), current,
+        LookupIterator::OWN_SKIP_INTERCEPTOR);
+    auto maybe_to_string_tag = JSReceiver::GetDataProperty(
+        &it_to_string_tag, AllocationPolicy::kAllocationDisallowed);
+    if (maybe_to_string_tag->IsString()) {
+      return std::make_pair(MaybeHandle<JSFunction>(),
+                            Handle<String>::cast(maybe_to_string_tag));
+    }
+
+    // Consider the following example:
+    //
+    //   function A() {}
+    //   function B() {}
+    //   B.prototype = new A();
+    //   B.prototype.constructor = B;
+    //
+    // The constructor name for `B.prototype` must yield "A", so we don't take
+    // "constructor" into account for the receiver itself, but only starting
+    // on the prototype chain.
+    if (!receiver.is_identical_to(current)) {
+      LookupIterator it_constructor(
+          isolate, receiver, isolate->factory()->constructor_string(), current,
+          LookupIterator::OWN_SKIP_INTERCEPTOR);
+      auto maybe_constructor = JSReceiver::GetDataProperty(
+          &it_constructor, AllocationPolicy::kAllocationDisallowed);
+      if (maybe_constructor->IsJSFunction()) {
+        auto constructor = Handle<JSFunction>::cast(maybe_constructor);
+        auto name = SharedFunctionInfo::DebugName(
+            handle(constructor->shared(), isolate));
+
+        if (name->length() != 0 &&
+            !name->Equals(ReadOnlyRoots(isolate).Object_string())) {
+          return std::make_pair(constructor, name);
+        }
+      }
     }
   }
 

test/inspector/regress/regress-crbug-980018-expected.txt

+Regression test for crbug.com/980018
+
+Running test: testClassNames
+{
+    result : {
+        className : A
+        description : A(0)
+        objectId : <objectId>
+        subtype : map
+        type : object
+    }
+}
+{
+    result : {
+        className : ThisIsB
+        description : ThisIsB(0)
+        objectId : <objectId>
+        subtype : map
+        type : object
+    }
+}
+{
+    result : {
+        className : ThisIsC
+        description : ThisIsC(0)
+        objectId : <objectId>
+        subtype : map
+        type : object
+    }
+}

test/inspector/regress/regress-crbug-980018.js

+// 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.com/980018');
+
+contextGroup.addInlineScript(`
+class A extends Map {
+}
+
+class B extends Map {
+  constructor() {
+    super();
+    Object.defineProperty(this, Symbol.toStringTag, {value: 'ThisIsB'});
+  }
+}
+
+class C extends Map {
+}
+Object.defineProperty(C.prototype, Symbol.toStringTag, {value: 'ThisIsC'});`);
+
+InspectorTest.runAsyncTestSuite([
+  async function testClassNames() {
+    await Protocol.Runtime.enable();
+    await Promise.all(['A', 'B', 'C'].map(klass => Protocol.Runtime.evaluate({expression: `new ${klass}();`}).then(({result}) => InspectorTest.logMessage(result))));
+    await Protocol.Runtime.disable();
+  }
+]);