Fix name shown by devtools for subclasses.

This replaces internal GetConstructorName with toStringTag, .constructor's name
and class_name. This entirely changes how the name is computed for use in
devtools.

BUG=chromium:529177
LOG=n

Review URL: https://codereview.chromium.org/1435273002

Cr-Commit-Position: refs/heads/master@{#32374}

src/api.cc

@@ -3870,7 +3870,7 @@ Local<String> v8::Object::ObjectProtoToString() {
 
 Local<String> v8::Object::GetConstructorName() {
   auto self = Utils::OpenHandle(this);
-  i::Handle<i::String> name(self->constructor_name());
+  i::Handle<i::String> name = i::JSReceiver::GetConstructorName(self);
   return Utils::ToLocal(name);
 }
 

src/factory.cc

@@ -96,7 +96,6 @@ Handle<PrototypeInfo> Factory::NewPrototypeInfo() {
   result->set_prototype_users(WeakFixedArray::Empty());
   result->set_registry_slot(PrototypeInfo::UNREGISTERED);
   result->set_validity_cell(Smi::FromInt(0));
-  result->set_constructor_name(Smi::FromInt(0));
   return result;
 }
 

src/messages.cc

@@ -417,7 +417,7 @@ MaybeHandle<String> ErrorToStringHelper::Stringify(Isolate* isolate,
   if (internal_error_lookup.IsFound() &&
       !ShadowsInternalError(isolate, &name_lookup, &internal_error_lookup)) {
     Handle<JSObject> holder = internal_error_lookup.GetHolder<JSObject>();
-    name = Handle<String>(holder->constructor_name());
+    name = JSReceiver::GetConstructorName(holder);
   } else {
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, name,

src/objects-debug.cc

@@ -877,7 +877,6 @@ void PrototypeInfo::PrototypeInfoVerify() {
     CHECK(prototype_users()->IsSmi());
   }
   CHECK(validity_cell()->IsCell() || validity_cell()->IsSmi());
-  VerifyPointer(constructor_name());
 }
 
 

src/objects-inl.h

@@ -5565,7 +5565,6 @@ ACCESSORS(Box, value, Object, kValueOffset)
 ACCESSORS(PrototypeInfo, prototype_users, Object, kPrototypeUsersOffset)
 SMI_ACCESSORS(PrototypeInfo, registry_slot, kRegistrySlotOffset)
 ACCESSORS(PrototypeInfo, validity_cell, Object, kValidityCellOffset)
-ACCESSORS(PrototypeInfo, constructor_name, Object, kConstructorNameOffset)
 
 ACCESSORS(SloppyBlockWithEvalContextExtension, scope_info, ScopeInfo,
           kScopeInfoOffset)

src/objects-printer.cc

@@ -995,7 +995,6 @@ void PrototypeInfo::PrototypeInfoPrint(std::ostream& os) {  // NOLINT
   os << "\n - prototype users: " << Brief(prototype_users());
   os << "\n - registry slot: " << registry_slot();
   os << "\n - validity cell: " << Brief(validity_cell());
-  os << "\n - constructor name: " << Brief(constructor_name());
   os << "\n";
 }
 

src/objects.cc

@@ -731,7 +731,9 @@ Handle<Object> JSReceiver::GetDataProperty(LookupIterator* it) {
       case LookupIterator::TRANSITION:
         UNREACHABLE();
       case LookupIterator::ACCESS_CHECK:
-        if (it->HasAccess()) continue;
+        // Support calling this method without an active context, but refuse
+        // access to access-checked objects in that case.
+        if (it->isolate()->context() != nullptr && it->HasAccess()) continue;
       // Fall through.
       case LookupIterator::JSPROXY:
         it->NotFound();
@@ -1815,9 +1817,7 @@ MaybeHandle<JSFunction> Map::GetConstructorFunction(
 void Map::PrintReconfiguration(FILE* file, int modify_index, PropertyKind kind,
                                PropertyAttributes attributes) {
   OFStream os(file);
-  os << "[reconfiguring ";
-  constructor_name()->PrintOn(file);
-  os << "] ";
+  os << "[reconfiguring]";
   Name* name = instance_descriptors()->GetKey(modify_index);
   if (name->IsString()) {
     String::cast(name)->PrintOn(file);
@@ -1842,9 +1842,7 @@ void Map::PrintGeneralization(FILE* file,
                               HeapType* old_field_type,
                               HeapType* new_field_type) {
   OFStream os(file);
-  os << "[generalizing ";
-  constructor_name()->PrintOn(file);
-  os << "] ";
+  os << "[generalizing]";
   Name* name = instance_descriptors()->GetKey(modify_index);
   if (name->IsString()) {
     String::cast(name)->PrintOn(file);
@@ -1876,9 +1874,7 @@ void Map::PrintGeneralization(FILE* file,
 void JSObject::PrintInstanceMigration(FILE* file,
                                       Map* original_map,
                                       Map* new_map) {
-  PrintF(file, "[migrating ");
-  map()->constructor_name()->PrintOn(file);
-  PrintF(file, "] ");
+  PrintF(file, "[migrating]");
   DescriptorArray* o = original_map->instance_descriptors();
   DescriptorArray* n = new_map->instance_descriptors();
   for (int i = 0; i < original_map->NumberOfOwnDescriptors(); i++) {
@@ -2230,31 +2226,38 @@ String* JSReceiver::class_name() {
 }
 
 
-String* Map::constructor_name() {
-  if (is_prototype_map() && prototype_info()->IsPrototypeInfo()) {
-    PrototypeInfo* proto_info = PrototypeInfo::cast(prototype_info());
-    if (proto_info->constructor_name()->IsString()) {
-      return String::cast(proto_info->constructor_name());
+// static
+Handle<String> JSReceiver::GetConstructorName(Handle<JSReceiver> receiver) {
+  Isolate* isolate = receiver->GetIsolate();
+  if (FLAG_harmony_tostring) {
+    Handle<Object> maybe_tag = JSReceiver::GetDataProperty(
+        receiver, isolate->factory()->to_string_tag_symbol());
+    if (maybe_tag->IsString()) return Handle<String>::cast(maybe_tag);
+  }
+
+  PrototypeIterator iter(isolate, receiver);
+  if (iter.IsAtEnd()) return handle(receiver->class_name());
+  Handle<JSReceiver> start = PrototypeIterator::GetCurrent<JSReceiver>(iter);
+  LookupIterator it(receiver, isolate->factory()->constructor_string(), start,
+                    LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR);
+  Handle<Object> maybe_constructor = JSReceiver::GetDataProperty(&it);
+  Handle<String> result = isolate->factory()->Object_string();
+  if (maybe_constructor->IsJSFunction()) {
+    JSFunction* constructor = JSFunction::cast(*maybe_constructor);
+    String* name = String::cast(constructor->shared()->name());
+    if (name->length() > 0) {
+      result = handle(name, isolate);
+    } else {
+      String* inferred_name = constructor->shared()->inferred_name();
+      if (inferred_name->length() > 0) {
+        result = handle(inferred_name, isolate);
+      }
     }
   }
-  Object* maybe_constructor = GetConstructor();
-  if (maybe_constructor->IsJSFunction()) {
-    JSFunction* constructor = JSFunction::cast(maybe_constructor);
-    String* name = String::cast(constructor->shared()->name());
-    if (name->length() > 0) return name;
-    String* inferred_name = constructor->shared()->inferred_name();
-    if (inferred_name->length() > 0) return inferred_name;
-    Object* proto = prototype();
-    if (proto->IsJSObject()) return JSObject::cast(proto)->constructor_name();
-  }
-  // TODO(rossberg): what about proxies?
-  // If the constructor is not present, return "Object".
-  return GetHeap()->Object_string();
-}
 
-
-String* JSReceiver::constructor_name() {
-  return map()->constructor_name();
+  return result.is_identical_to(isolate->factory()->Object_string())
+             ? handle(receiver->class_name())
+             : result;
 }
 
 
@@ -11754,13 +11757,9 @@ void JSObject::OptimizeAsPrototype(Handle<JSObject> object,
       Isolate* isolate = object->GetIsolate();
       if (!constructor->shared()->IsApiFunction() &&
           object->class_name() == isolate->heap()->Object_string()) {
-        Handle<String> constructor_name(object->constructor_name(), isolate);
         Context* context = constructor->context()->native_context();
         JSFunction* object_function = context->object_function();
         object->map()->SetConstructor(object_function);
-        Handle<PrototypeInfo> proto_info =
-            Map::GetOrCreatePrototypeInfo(object, isolate);
-        proto_info->set_constructor_name(*constructor_name);
       }
     }
   }

src/objects.h

@@ -1887,7 +1887,7 @@ class JSReceiver: public HeapObject {
 
   // Returns the constructor name (the name (possibly, inferred name) of the
   // function that was used to instantiate the object).
-  String* constructor_name();
+  static Handle<String> GetConstructorName(Handle<JSReceiver> receiver);
 
   Context* GetCreationContext();
 
@@ -5651,10 +5651,6 @@ class Map: public HeapObject {
   static Handle<Map> Normalize(Handle<Map> map, PropertyNormalizationMode mode,
                                const char* reason);
 
-  // Returns the constructor name (the name (possibly, inferred name) of the
-  // function that was used to instantiate the object).
-  String* constructor_name();
-
   // Tells whether the map is used for JSObjects in dictionary mode (ie
   // normalized objects, ie objects for which HasFastProperties returns false).
   // A map can never be used for both dictionary mode and fast mode JSObjects.
@@ -6159,8 +6155,6 @@ class PrototypeInfo : public Struct {
   // given receiver embed the currently valid cell for that receiver's prototype
   // during their compilation and check it on execution.
   DECL_ACCESSORS(validity_cell, Object)
-  // [constructor_name]: User-friendly name of the original constructor.
-  DECL_ACCESSORS(constructor_name, Object)
 
   DECLARE_CAST(PrototypeInfo)
 

src/profiler/heap-snapshot-generator.cc

@@ -1724,14 +1724,11 @@ void V8HeapExplorer::ExtractInternalReferences(JSObject* js_obj, int entry) {
 
 
 String* V8HeapExplorer::GetConstructorName(JSObject* object) {
-  Heap* heap = object->GetHeap();
-  if (object->IsJSFunction()) return heap->closure_string();
-  String* constructor_name = object->constructor_name();
-  if (constructor_name == heap->Object_string()) {
-    // TODO(verwaest): Try to get object.constructor.name in this case.
-    // This requires handlification of the V8HeapExplorer.
-  }
-  return object->constructor_name();
+  Isolate* isolate = object->GetIsolate();
+  if (object->IsJSFunction()) return isolate->heap()->closure_string();
+  DisallowHeapAllocation no_gc;
+  HandleScope scope(isolate);
+  return *JSReceiver::GetConstructorName(handle(object, isolate));
 }
 
 

test/cctest/test-api.cc

@@ -5550,10 +5550,10 @@ TEST(TryCatchInTryFinally) {
 static void check_reference_error_message(v8::Local<v8::Message> message,
                                           v8::Local<v8::Value> data) {
   const char* reference_error = "Uncaught ReferenceError: asdf is not defined";
-  CHECK(message->Get()
-            ->Equals(CcTest::isolate()->GetCurrentContext(),
-                     v8_str(reference_error))
-            .FromJust());
+  Local<v8::Context> context = CcTest::isolate()->GetCurrentContext();
+  const char* reference_error2 = "Uncaught Error: asdf is not defined";
+  CHECK(message->Get()->Equals(context, v8_str(reference_error)).FromJust() ||
+        message->Get()->Equals(context, v8_str(reference_error2)).FromJust());
 }
 
 
@@ -12891,11 +12891,13 @@ THREADED_TEST(ObjectGetConstructorName) {
       "function Parent() {};"
       "function Child() {};"
       "Child.prototype = new Parent();"
+      "Child.prototype.constructor = Child;"
       "var outer = { inner: function() { } };"
       "var p = new Parent();"
       "var c = new Child();"
       "var x = new outer.inner();"
-      "var proto = Child.prototype;")->Run();
+      "var proto = Child.prototype;")
+      ->Run();
 
   Local<v8::Value> p = context->Global()->Get(v8_str("p"));
   CHECK(p->IsObject() &&
@@ -12917,6 +12919,28 @@ THREADED_TEST(ObjectGetConstructorName) {
 }
 
 
+THREADED_TEST(SubclassGetConstructorName) {
+  v8::Isolate* isolate = CcTest::isolate();
+  LocalContext context;
+  v8::HandleScope scope(isolate);
+  v8_compile(
+      "\"use strict\";"
+      "class Parent {}"
+      "class Child extends Parent {}"
+      "var p = new Parent();"
+      "var c = new Child();")
+      ->Run();
+
+  Local<v8::Value> p = context->Global()->Get(v8_str("p"));
+  CHECK(p->IsObject() &&
+        p->ToObject(isolate)->GetConstructorName()->Equals(v8_str("Parent")));
+
+  Local<v8::Value> c = context->Global()->Get(v8_str("c"));
+  CHECK(c->IsObject() &&
+        c->ToObject(isolate)->GetConstructorName()->Equals(v8_str("Child")));
+}
+
+
 bool ApiTestFuzzer::fuzzing_ = false;
 v8::base::Semaphore ApiTestFuzzer::all_tests_done_(0);
 int ApiTestFuzzer::active_tests_;

test/cctest/test-heap-profiler.cc

@@ -1818,11 +1818,11 @@ TEST(GetConstructorName) {
       "var Constructor2 = function() {};\n"
       "var obj2 = new Constructor2();\n"
       "var obj3 = {};\n"
-      "obj3.constructor = function Constructor3() {};\n"
+      "obj3.__proto__ = { constructor: function Constructor3() {} };\n"
       "var obj4 = {};\n"
       "// Slow properties\n"
       "for (var i=0; i<2000; ++i) obj4[\"p\" + i] = i;\n"
-      "obj4.constructor = function Constructor4() {};\n"
+      "obj4.__proto__ = { constructor: function Constructor4() {} };\n"
       "var obj5 = {};\n"
       "var obj6 = {};\n"
       "obj6.constructor = 6;");
@@ -1841,17 +1841,13 @@ TEST(GetConstructorName) {
   v8::Local<v8::Object> obj3 = js_global->Get(v8_str("obj3")).As<v8::Object>();
   i::Handle<i::JSObject> js_obj3 =
       i::Handle<i::JSObject>::cast(v8::Utils::OpenHandle(*obj3));
-  // TODO(verwaest): Restore to Constructor3 once supported by the
-  // heap-snapshot-generator.
-  CHECK_EQ(
-      0, StringCmp("Object", i::V8HeapExplorer::GetConstructorName(*js_obj3)));
+  CHECK_EQ(0, StringCmp("Constructor3",
+                        i::V8HeapExplorer::GetConstructorName(*js_obj3)));
   v8::Local<v8::Object> obj4 = js_global->Get(v8_str("obj4")).As<v8::Object>();
   i::Handle<i::JSObject> js_obj4 =
       i::Handle<i::JSObject>::cast(v8::Utils::OpenHandle(*obj4));
-  // TODO(verwaest): Restore to Constructor4 once supported by the
-  // heap-snapshot-generator.
-  CHECK_EQ(
-      0, StringCmp("Object", i::V8HeapExplorer::GetConstructorName(*js_obj4)));
+  CHECK_EQ(0, StringCmp("Constructor4",
+                        i::V8HeapExplorer::GetConstructorName(*js_obj4)));
   v8::Local<v8::Object> obj5 = js_global->Get(v8_str("obj5")).As<v8::Object>();
   i::Handle<i::JSObject> js_obj5 =
       i::Handle<i::JSObject>::cast(v8::Utils::OpenHandle(*obj5));

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

@@ -6,5 +6,5 @@ var error;
 try { reference_error(); } catch (e) { error = e; }
 toString = error.toString;
 error.__proto__ = [];
-assertEquals("ReferenceError: reference_error is not defined",
+assertEquals("Array: reference_error is not defined",
              toString.call(error));