Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Jun 2017 23:52:25 +0000 (23:52 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 5 Jun 2017 23:52:25 +0000 (23:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172848
<rdar://problem/25709212>

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2017-06-05
Reviewed by Saam Barati.

JSTests:

* typeProfiler/inheritance.js:
Rewrite the test slightly for clarity. The hoisting was confusing.

* heapProfiler/class-names.js: Added.
(MyES5Class):
(MyES6Class):
(MyES6Subclass):
Test object types and improved class names.

* heapProfiler/driver/driver.js:
(CheapHeapSnapshotNode):
(CheapHeapSnapshot):
(createCheapHeapSnapshot):
(HeapSnapshot):
(createHeapSnapshot):
Update snapshot parsing from version 1 to version 2.

Source/JavaScriptCore:

* heap/HeapSnapshotBuilder.h:
* heap/HeapSnapshotBuilder.cpp:
Update the snapshot version. Change the node's 0 | 1 internal value
to be a 32bit bit flag. This is nice in that it is both compatible
with the previous snapshot version and the same size. We can use more
flags in the future.

(JSC::HeapSnapshotBuilder::json):
In cases where the classInfo gives us "Object" check for a better
class name by checking (o).__proto__.constructor.name. We avoid this
check in cases where (o).hasOwnProperty("constructor") which is the
case for most Foo.prototype objects. Otherwise this would get the
name of the Foo superclass for the Foo.prototype object.

* runtime/JSObject.cpp:
(JSC::JSObject::calculatedClassName):
Handle some possible edge cases that were not handled before. Such
as a JSObject without a GlobalObject, and an object which doesn't
have a default getPrototype. Try to make the code a little clearer.

Source/WebInspectorUI:

* UserInterface/Workers/HeapSnapshot/HeapSnapshot.js:
(HeapSnapshot):
Support the new snapshot version. The only thing that changes are the
node flags, and its actually completely compatible with version 1.

(HeapSnapshot.updateCategoriesAndMetadata):
List the count of object type instances in each class category.

(HeapSnapshot.prototype.serializeNode):
Include whether or not the node is an object type.

* UserInterface/Proxies/HeapSnapshotNodeProxy.js:
(WebInspector.HeapSnapshotNodeProxy):
(WebInspector.HeapSnapshotNodeProxy.deserialize):
Add a new Node isObjectType property based on the new data.

* UserInterface/Views/HeapSnapshotClassDataGridNode.js:
(WebInspector.HeapSnapshotClassDataGridNode.prototype.createCellContent):
* UserInterface/Views/HeapSnapshotClusterContentView.js:
(WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName):
If a class contains 50% or more object type instances then it as such
instead of defaulting to native.

* UserInterface/Views/HeapSnapshotDataGridTree.js:
(WebInspector.HeapSnapshotInstancesDataGridTree.prototype.populateTopLevel):
* UserInterface/Views/HeapSnapshotInstanceDataGridNode.js:
(WebInspector.HeapSnapshotInstanceDataGridNode.prototype.createCellContent):
We can be more specific than the default if the individual instance is
known to be an object type.

LayoutTests:

* inspector/unit-tests/heap-snapshot-expected.txt:
* inspector/unit-tests/heap-snapshot.html:
Update for the new node flag.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@217807 268f45cc-cd09-0410-ab3c-d52691b4dbfc

18 files changed:
JSTests/ChangeLog
JSTests/heapProfiler/class-names.js [new file with mode: 0644]
JSTests/heapProfiler/driver/driver.js
JSTests/typeProfiler/inheritance.js
LayoutTests/ChangeLog
LayoutTests/inspector/unit-tests/heap-snapshot-expected.txt
LayoutTests/inspector/unit-tests/heap-snapshot.html
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/HeapSnapshotBuilder.cpp
Source/JavaScriptCore/heap/HeapSnapshotBuilder.h
Source/JavaScriptCore/runtime/JSObject.cpp
Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Proxies/HeapSnapshotNodeProxy.js
Source/WebInspectorUI/UserInterface/Views/HeapSnapshotClassDataGridNode.js
Source/WebInspectorUI/UserInterface/Views/HeapSnapshotClusterContentView.js
Source/WebInspectorUI/UserInterface/Views/HeapSnapshotDataGridTree.js
Source/WebInspectorUI/UserInterface/Views/HeapSnapshotInstanceDataGridNode.js
Source/WebInspectorUI/UserInterface/Workers/HeapSnapshot/HeapSnapshot.js

index 22994ef..ba07ec9 100644 (file)
@@ -1,3 +1,28 @@
+2017-06-05  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        <rdar://problem/25709212>
+
+        Reviewed by Saam Barati.
+
+        * typeProfiler/inheritance.js:
+        Rewrite the test slightly for clarity. The hoisting was confusing.
+
+        * heapProfiler/class-names.js: Added.
+        (MyES5Class):
+        (MyES6Class):
+        (MyES6Subclass):
+        Test object types and improved class names.
+
+        * heapProfiler/driver/driver.js:
+        (CheapHeapSnapshotNode):
+        (CheapHeapSnapshot):
+        (createCheapHeapSnapshot):
+        (HeapSnapshot):
+        (createHeapSnapshot):
+        Update snapshot parsing from version 1 to version 2.
+
 2017-06-02  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         ASSERTION FAILED: "We should only declare a function as a lexically scoped variable in scopes where var declarations aren't allowed. ..." for function redeclaration with async function module export
diff --git a/JSTests/heapProfiler/class-names.js b/JSTests/heapProfiler/class-names.js
new file mode 100644 (file)
index 0000000..4818da5
--- /dev/null
@@ -0,0 +1,44 @@
+load("./driver/driver.js");
+
+function MyES5ClassUgly() {};
+MyES5ClassUgly.displayName = "MyES5ClassDisplayName";
+MyES5ClassUgly.prototype = { constructor: MyES5ClassUgly };
+
+class MyES6Class {};
+class MyES6Subclass extends MyES6Class {};
+
+let classInstances = [];
+for (let i = 0; i < 5; ++i)
+    classInstances.push(new MyES5ClassUgly);
+for (let i = 0; i < 10; ++i)
+    classInstances.push(new MyES6Class);
+for (let i = 0; i < 20; ++i)
+    classInstances.push(new MyES6Subclass);
+
+let myFunction = function() {};
+let myMap = new Map;
+
+(function() {
+    let nodes;
+    let snapshot = createCheapHeapSnapshot();
+
+    nodes = snapshot.nodesWithClassName("MyES5ClassDisplayName");
+    assert(nodes.length === 5, "Snapshot should contain 5 'MyES5ClassDisplayName' (MyES5ClassUgly) instances");
+    assert(nodes[0].isObjectType, "MyES5Class instance should have had its ObjectType flag set");
+
+    nodes = snapshot.nodesWithClassName("MyES6Class");
+    assert(nodes.length === 10, "Snapshot should contain 10 'MyES6Class' instances");
+    assert(nodes[0].isObjectType, "MyES6Class instance should have had its ObjectType flag set");
+
+    nodes = snapshot.nodesWithClassName("MyES6Subclass");
+    assert(nodes.length === 20, "Snapshot should contain 20 'MyES6Subclass' instances");
+    assert(nodes[0].isObjectType, "MyES6Subclass instance should have had its ObjectType flag set");
+
+    nodes = snapshot.nodesWithClassName("Function");
+    assert(nodes.length > 0, "Should be at least 1 Function instance");
+    assert(!nodes[0].isObjectType, "Function instance should not have its ObjectType flag set");
+
+    nodes = snapshot.nodesWithClassName("Map");
+    assert(nodes.length > 0, "Should be at least 1 Map instance");
+    assert(!nodes[0].isObjectType, "Map instance should not have its ObjectType flag set");
+})();
index 58ee5a8..008ae5e 100644 (file)
@@ -14,10 +14,14 @@ const nodeFieldCount = 5;
 const nodeIdOffset = 0;
 const nodeSizeOffset = 1;
 const nodeClassNameOffset = 2;
-const nodeInternalOffset = 3;
+const nodeFlagsOffset = 3;
 const nodeFirstEdgeOffset = 4;
 const nodeNoEdgeValue = 0xffffffff; // UINT_MAX
 
+// Node Flags.
+const internalFlagMask = (1 << 0);
+const objectTypeMask = (1 << 1);
+
 // [<0:fromId>, <1:toId>, <2:typeTableIndex>, <3:edgeDataIndexOrEdgeNameIndex>]
 const edgeFieldCount = 4;
 const edgeFromIdOffset = 0;
@@ -35,7 +39,10 @@ CheapHeapSnapshotNode = class CheapHeapSnapshotNode
         this.id = nodes[nodeIndex + nodeIdOffset];
         this.size = nodes[nodeIndex + nodeSizeOffset];
         this.className = snapshot.classNameFromTableIndex(nodes[nodeIndex + nodeClassNameOffset]);
-        this.internal = nodes[nodeIndex + nodeInternalOffset] ? true : false;
+
+        let flags = nodes[nodeIndex + nodeFlagsOffset];
+        this.internal = flags & internalFlagMask ? true : false;
+        this.isObjectType = flags & objectTypeMask ? true : false;
 
         this.outgoingEdges = [];
         let firstEdgeIndex = nodes[nodeIndex + nodeFirstEdgeOffset];
@@ -91,7 +98,7 @@ CheapHeapSnapshot = class CheapHeapSnapshot
             this._nodes[n++] = nodes[i++]; // id
             this._nodes[n++] = nodes[i++]; // size
             this._nodes[n++] = nodes[i++]; // classNameTableIndex
-            this._nodes[n++] = nodes[i++]; // internal
+            this._nodes[n++] = nodes[i++]; // flags
             this._nodes[n++] = nodeNoEdgeValue;
         }
 
@@ -154,7 +161,7 @@ function createCheapHeapSnapshot() {
     let json = generateHeapSnapshot();
 
     let {version, nodes, nodeClassNames, edges, edgeTypes} = json;
-    assert(version === 1, "Heap Snapshot payload should be version 1");
+    assert(version === 2, "Heap Snapshot payload should be version 2");
     assert(nodes.length, "Heap Snapshot should have nodes");
     assert(nodeClassNames.length, "Heap Snapshot should have nodeClassNames");
     assert(edges.length, "Heap Snapshot should have edges");
@@ -210,7 +217,8 @@ HeapSnapshot = class HeapSnapshot
             let id = nodes[i++];
             let size = nodes[i++];
             let classNameIndex = nodes[i++];
-            let internal = nodes[i++];
+            let flags = nodes[i++];
+            let internal = flags & internalFlagMask ? true : false;
 
             let node = new HeapSnapshotNode(id, nodeClassNames[classNameIndex], size, internal);
             this.nodeMap.set(id, node);
@@ -256,7 +264,7 @@ function createHeapSnapshot() {
     let json = generateHeapSnapshot();
 
     let {version, nodes, nodeClassNames, edges, edgeTypes} = json;
-    assert(version === 1, "Heap Snapshot payload should be version 1");
+    assert(version === 2, "Heap Snapshot payload should be version 2");
     assert(nodes.length, "Heap Snapshot should have nodes");
     assert(nodeClassNames.length, "Heap Snapshot should have nodeClassNames");
     assert(edges.length, "Heap Snapshot should have edges");
index 176b2d7..21e8d47 100644 (file)
@@ -3,6 +3,10 @@ load("./driver/driver.js");
 function wrapper()
 {
 
+function A() { };
+function B() { };
+function C() { };
+
 var theA = new A;
 var theB = new B;
 var theC = new C;
@@ -14,9 +18,8 @@ secondHierarchyTest = Object.create(theC);
 
 var secondB = Object.create(theB);
 
-function A() { };
-function B() { }; B.prototype.__proto__ = A.prototype;
-function C() { }; C.prototype.__proto__ = A.prototype;
+B.prototype.__proto__ = A.prototype;
+C.prototype.__proto__ = A.prototype;
 
 }
 wrapper();
index de7465f..3266b5f 100644 (file)
@@ -1,3 +1,15 @@
+2017-06-05  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        <rdar://problem/25709212>
+
+        Reviewed by Saam Barati.
+
+        * inspector/unit-tests/heap-snapshot-expected.txt:
+        * inspector/unit-tests/heap-snapshot.html:
+        Update for the new node flag.
+
 2017-06-05  Ryan Haddad  <ryanhaddad@apple.com>
 
         Rebaseline tests for Windows after r217772.
index 5ef12a8..af5f709 100644 (file)
@@ -23,6 +23,7 @@ PASS: Node className should be 'Window'.
 PASS: Node identifier should match.
 PASS: Node size should match.
 PASS: Node internal state should match.
+PASS: Node isObjectType state should match.
 PASS: Node gcRoot state should match.
 PASS: Node retainedSize should at least be the size.
 
index 470c579..924c431 100644 (file)
@@ -9,12 +9,13 @@ function test()
 
     WebInspector.TestHeapSnapshotNode = class TestHeapSnapshotNode
     {
-        constructor(identifier, className, size, internal)
+        constructor(identifier, className, size, flags)
         {
             this.id = identifier;
             this.className = className;
             this.size = size; 
-            this.internal = internal;
+            this.internal = flags & (1 << 0) ? true : false;
+            this.isObjectType = flags & (1 << 1) ? true : false;
             this.gcRoot = false;
             this.outgoingEdges = [];
             this.incomingEdges = [];
@@ -59,9 +60,9 @@ function test()
                 let id = nodes[i++];
                 let size = nodes[i++];
                 let classNameIndex = nodes[i++];
-                let internal = nodes[i++];
+                let flags = nodes[i++];
 
-                let node = new WebInspector.TestHeapSnapshotNode(id, nodeClassNames[classNameIndex], size, !!internal);
+                let node = new WebInspector.TestHeapSnapshotNode(id, nodeClassNames[classNameIndex], size, flags);
                 nodeMap.set(id, node);
                 processedNodes.push(node);
             }
@@ -127,6 +128,7 @@ function test()
             && node1.size === node2.size
             && node1.className === node2.className
             && node1.internal === node2.internal
+            && node1.isObjectType === node2.isObjectType
             && node1.gcRoot === node2.gcRoot;
     }
 
@@ -189,6 +191,7 @@ function test()
                 InspectorTest.expectThat(heapSnapshotNode.id === testSnapshotNodeForWindowObject.id, "Node identifier should match.")
                 InspectorTest.expectThat(heapSnapshotNode.size === testSnapshotNodeForWindowObject.size, "Node size should match.");
                 InspectorTest.expectThat(heapSnapshotNode.internal === testSnapshotNodeForWindowObject.internal, "Node internal state should match.");
+                InspectorTest.expectThat(heapSnapshotNode.isObjectType === testSnapshotNodeForWindowObject.isObjectType, "Node isObjectType state should match.");
                 InspectorTest.expectThat(heapSnapshotNode.gcRoot === testSnapshotNodeForWindowObject.gcRoot, "Node gcRoot state should match.");
                 InspectorTest.expectThat(heapSnapshotNode.retainedSize >= heapSnapshotNode.size, "Node retainedSize should at least be the size.");
                 resolve();
index 3d430d3..d4951e3 100644 (file)
@@ -1,3 +1,31 @@
+2017-06-05  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        <rdar://problem/25709212>
+
+        Reviewed by Saam Barati.
+
+        * heap/HeapSnapshotBuilder.h:
+        * heap/HeapSnapshotBuilder.cpp:
+        Update the snapshot version. Change the node's 0 | 1 internal value
+        to be a 32bit bit flag. This is nice in that it is both compatible
+        with the previous snapshot version and the same size. We can use more
+        flags in the future.
+
+        (JSC::HeapSnapshotBuilder::json):
+        In cases where the classInfo gives us "Object" check for a better
+        class name by checking (o).__proto__.constructor.name. We avoid this
+        check in cases where (o).hasOwnProperty("constructor") which is the
+        case for most Foo.prototype objects. Otherwise this would get the
+        name of the Foo superclass for the Foo.prototype object.
+
+        * runtime/JSObject.cpp:
+        (JSC::JSObject::calculatedClassName):
+        Handle some possible edge cases that were not handled before. Such
+        as a JSObject without a GlobalObject, and an object which doesn't
+        have a default getPrototype. Try to make the code a little clearer.
+
 2017-06-05  Saam Barati  <sbarati@apple.com>
 
         Update JSC's features.json
index 632183d..006f1fe 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2017 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -20,7 +20,7 @@
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "config.h"
@@ -37,7 +37,7 @@
 #include <wtf/text/StringBuilder.h>
 
 namespace JSC {
-    
+
 unsigned HeapSnapshotBuilder::nextAvailableObjectIdentifier = 1;
 unsigned HeapSnapshotBuilder::getNextObjectIdentifier() { return nextAvailableObjectIdentifier++; }
 void HeapSnapshotBuilder::resetNextAvailableObjectIdentifier() { HeapSnapshotBuilder::nextAvailableObjectIdentifier = 1; }
@@ -54,7 +54,7 @@ HeapSnapshotBuilder::~HeapSnapshotBuilder()
 void HeapSnapshotBuilder::buildSnapshot()
 {
     PreventCollectionScope preventCollectionScope(m_profiler.vm().heap);
-    
+
     m_snapshot = std::make_unique<HeapSnapshot>(m_profiler.mostRecentSnapshot());
     {
         m_profiler.setActiveSnapshotBuilder(this);
@@ -135,10 +135,10 @@ bool HeapSnapshotBuilder::hasExistingNodeForCell(JSCell* cell)
 // Heap Snapshot JSON Format:
 //
 //   {
-//      "version": 1.0,
+//      "version": 2,
 //      "nodes": [
-//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <internal>,
-//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <internal>,
+//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <flags>,
+//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <flags>,
 //          ...
 //      ],
 //      "nodeClassNames": [
@@ -162,8 +162,10 @@ bool HeapSnapshotBuilder::hasExistingNodeForCell(JSCell* cell)
 //     <nodeClassNameIndex>
 //       - index into the "nodeClassNames" list.
 //
-//     <internal>
-//       - 0 = false, 1 = true.
+//     <flags>
+//       - 0b0000 - no flags
+//       - 0b0001 - internal instance
+//       - 0b0010 - Object subclassification
 //
 //     <edgeTypeIndex>
 //       - index into the "edgeTypes" list.
@@ -173,6 +175,11 @@ bool HeapSnapshotBuilder::hasExistingNodeForCell(JSCell* cell)
 //       - for Index edges this is the index value.
 //       - for Property or Variable edges this is an index into the "edgeNames" list.
 
+typedef enum {
+    Internal      = 1 << 0,
+    ObjectSubtype = 1 << 1,
+} NodeFlags;
+
 static uint8_t edgeTypeToNumber(EdgeType type)
 {
     return static_cast<uint8_t>(type);
@@ -223,20 +230,38 @@ String HeapSnapshotBuilder::json(std::function<bool (const HeapSnapshotNode&)> a
         if (!allowNodeCallback(node))
             return;
 
+        unsigned flags = 0;
+
         allowedNodeIdentifiers.set(node.cell, node.identifier);
 
-        auto result = classNameIndexes.add(node.cell->classInfo(vm)->className, nextClassNameIndex);
+        const char* className = node.cell->classInfo(vm)->className;
+        if (node.cell->isObject() && !strcmp(className, JSObject::info()->className)) {
+            flags |= ObjectSubtype;
+
+            // Skip calculating a class name if this object has a `constructor` own property.
+            // These cases are typically F.prototype objects and we want to treat these as
+            // "Object" in snapshots and not get the name of what the prototype's parent.
+            JSObject* object = jsCast<JSObject*>(node.cell);
+            if (JSGlobalObject* globalObject = object->structure(vm)->globalObject()) {
+                ExecState* exec = globalObject->globalExec();
+                PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry);
+                if (!object->getOwnPropertySlot(object, exec, vm.propertyNames->constructor, slot))
+                    className = JSObject::calculatedClassName(object).utf8().data();
+            }
+        }
+
+        auto result = classNameIndexes.add(className, nextClassNameIndex);
         if (result.isNewEntry)
             nextClassNameIndex++;
         unsigned classNameIndex = result.iterator->value;
 
-        bool isInternal = false;
         if (!node.cell->isString()) {
             Structure* structure = node.cell->structure(vm);
-            isInternal = !structure || !structure->globalObject();
+            if (!structure || !structure->globalObject())
+                flags |= Internal;
         }
 
-        // <nodeId>, <sizeInBytes>, <className>, <optionalInternalBoolean>
+        // <nodeId>, <sizeInBytes>, <className>, <flags>
         json.append(',');
         json.appendNumber(node.identifier);
         json.append(',');
@@ -244,7 +269,7 @@ String HeapSnapshotBuilder::json(std::function<bool (const HeapSnapshotNode&)> a
         json.append(',');
         json.appendNumber(classNameIndex);
         json.append(',');
-        json.append(isInternal ? '1' : '0');
+        json.appendNumber(flags);
     };
 
     bool firstEdge = true;
@@ -283,7 +308,7 @@ String HeapSnapshotBuilder::json(std::function<bool (const HeapSnapshotNode&)> a
     json.append('{');
 
     // version
-    json.appendLiteral("\"version\":1");
+    json.appendLiteral("\"version\":2");
 
     // nodes
     json.append(',');
index 0ed85d1..639563e 100644 (file)
@@ -36,6 +36,8 @@ namespace JSC {
 class HeapProfiler;
 class HeapSnapshot;
 class JSCell;
+class JSObject;
+class VM;
 
 struct HeapSnapshotNode {
     HeapSnapshotNode(JSCell* cell, unsigned identifier)
index 77f4736..649f115 100644 (file)
@@ -526,19 +526,23 @@ String JSObject::toStringName(const JSObject* object, ExecState*)
 String JSObject::calculatedClassName(JSObject* object)
 {
     String prototypeFunctionName;
-    auto globalObject = object->globalObject();
+    auto structure = object->structure();
+    auto globalObject = structure->globalObject();
     VM& vm = globalObject->vm();
     auto scope = DECLARE_CATCH_SCOPE(vm);
 
-    ExecState* exec = globalObject->globalExec();
-    PropertySlot slot(object->getPrototypeDirect(), PropertySlot::InternalMethodType::VMInquiry);
-    PropertyName constructor(exec->propertyNames().constructor);
-    if (object->getPropertySlot(exec, constructor, slot)) {
-        if (slot.isValue()) {
-            JSValue constructorValue = slot.getValue(exec, constructor);
-            if (constructorValue.isCell()) {
-                if (JSCell* constructorCell = constructorValue.asCell()) {
-                    if (JSObject* ctorObject = constructorCell->getObject()) {
+    // Get the display name of obj.__proto__.constructor.
+    MethodTable::GetPrototypeFunctionPtr defaultGetPrototype = JSObject::getPrototype;
+    if (structure->classInfo()->methodTable.getPrototype == defaultGetPrototype) {
+        JSValue protoValue = object->getPrototypeDirect();
+        if (protoValue.isObject()) {
+            JSObject* protoObject = jsCast<JSObject*>(protoValue);
+            ExecState* exec = globalObject->globalExec();
+            PropertyName constructor(exec->propertyNames().constructor);
+            PropertySlot slot(protoValue, PropertySlot::InternalMethodType::VMInquiry);
+            if (protoObject->getPropertySlot(exec, constructor, slot)) {
+                if (slot.isValue()) {
+                    if (JSObject* ctorObject = jsDynamicCast<JSObject*>(vm, slot.getValue(exec, constructor))) {
                         if (JSFunction* constructorFunction = jsDynamicCast<JSFunction*>(vm, ctorObject))
                             prototypeFunctionName = constructorFunction->calculatedDisplayName(vm);
                         else if (InternalFunction* constructorFunction = jsDynamicCast<InternalFunction*>(vm, ctorObject))
@@ -548,6 +552,7 @@ String JSObject::calculatedClassName(JSObject* object)
             }
         }
     }
+
     ASSERT(!scope.exception() || prototypeFunctionName.isNull());
     if (UNLIKELY(scope.exception()))
         scope.clearException();
index 5962c39..f7a0f87 100644 (file)
@@ -1,3 +1,41 @@
+2017-06-05  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        <rdar://problem/25709212>
+
+        Reviewed by Saam Barati.
+
+        * UserInterface/Workers/HeapSnapshot/HeapSnapshot.js:
+        (HeapSnapshot):
+        Support the new snapshot version. The only thing that changes are the
+        node flags, and its actually completely compatible with version 1.
+
+        (HeapSnapshot.updateCategoriesAndMetadata):
+        List the count of object type instances in each class category.
+
+        (HeapSnapshot.prototype.serializeNode):
+        Include whether or not the node is an object type.
+
+        * UserInterface/Proxies/HeapSnapshotNodeProxy.js:
+        (WebInspector.HeapSnapshotNodeProxy):
+        (WebInspector.HeapSnapshotNodeProxy.deserialize):
+        Add a new Node isObjectType property based on the new data.
+
+        * UserInterface/Views/HeapSnapshotClassDataGridNode.js:
+        (WebInspector.HeapSnapshotClassDataGridNode.prototype.createCellContent):
+        * UserInterface/Views/HeapSnapshotClusterContentView.js:
+        (WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName):
+        If a class contains 50% or more object type instances then it as such
+        instead of defaulting to native.
+
+        * UserInterface/Views/HeapSnapshotDataGridTree.js:
+        (WebInspector.HeapSnapshotInstancesDataGridTree.prototype.populateTopLevel):
+        * UserInterface/Views/HeapSnapshotInstanceDataGridNode.js:
+        (WebInspector.HeapSnapshotInstanceDataGridNode.prototype.createCellContent):
+        We can be more specific than the default if the individual instance is
+        known to be an object type.
+
 2017-06-02  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: Don't create NavigationSidebarPanel classes until they are needed by a Tab
index 908400d..58c7ae1 100644 (file)
@@ -25,7 +25,7 @@
 
 WebInspector.HeapSnapshotNodeProxy = class HeapSnapshotNodeProxy
 {
-    constructor(snapshotObjectId, identifier, className, size, retainedSize, internal, gcRoot, dead, dominatorNodeIdentifier, hasChildren)
+    constructor(snapshotObjectId, identifier, className, size, retainedSize, internal, isObjectType, gcRoot, dead, dominatorNodeIdentifier, hasChildren)
     {
         this._proxyObjectId = snapshotObjectId;
 
@@ -34,6 +34,7 @@ WebInspector.HeapSnapshotNodeProxy = class HeapSnapshotNodeProxy
         this.size = size;
         this.retainedSize = retainedSize;
         this.internal = internal;
+        this.isObjectType = isObjectType;
         this.gcRoot = gcRoot;
         this.dead = dead;
         this.dominatorNodeIdentifier = dominatorNodeIdentifier;
@@ -44,8 +45,8 @@ WebInspector.HeapSnapshotNodeProxy = class HeapSnapshotNodeProxy
 
     static deserialize(objectId, serializedNode)
     {
-        let {id, className, size, retainedSize, internal, gcRoot, dead, dominatorNodeIdentifier, hasChildren} = serializedNode;
-        return new WebInspector.HeapSnapshotNodeProxy(objectId, id, className, size, retainedSize, internal, gcRoot, dead, dominatorNodeIdentifier, hasChildren);
+        let {id, className, size, retainedSize, internal, isObjectType, gcRoot, dead, dominatorNodeIdentifier, hasChildren} = serializedNode;
+        return new WebInspector.HeapSnapshotNodeProxy(objectId, id, className, size, retainedSize, internal, isObjectType, gcRoot, dead, dominatorNodeIdentifier, hasChildren);
     }
 
     // Proxied
index 2a691c1..b8776c7 100644 (file)
@@ -60,10 +60,11 @@ WebInspector.HeapSnapshotClassDataGridNode = class HeapSnapshotClassDataGridNode
             return Number.bytesToString(this._data.size);
 
         if (columnIdentifier === "className") {
-            let {className} = this._data;
+            const internal = false;
+            let {className, isObjectSubcategory} = this._data;
             let fragment = document.createDocumentFragment();
             let iconElement = fragment.appendChild(document.createElement("img"));
-            iconElement.classList.add("icon", WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className));
+            iconElement.classList.add("icon", WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className, internal, isObjectSubcategory));
             let nameElement = fragment.appendChild(document.createElement("span"));
             nameElement.classList.add("class-name");
             nameElement.textContent = className;
index 30f96f8..d3a18bb 100644 (file)
@@ -58,10 +58,12 @@ WebInspector.HeapSnapshotClusterContentView = class HeapSnapshotClusterContentVi
 
     // Static
 
-    static iconStyleClassNameForClassName(className, internal)
+    static iconStyleClassNameForClassName(className, internal, isObjectType)
     {
         if (internal)
             return "native";
+        if (isObjectType)
+            return "object";
 
         switch (className) {
         case "Object":
index 7057f4c..5d725a6 100644 (file)
@@ -216,7 +216,8 @@ WebInspector.HeapSnapshotInstancesDataGridTree = class HeapSnapshotInstancesData
     populateTopLevel()
     {
         // Populate the first level with the different non-internal classes.
-        for (let [className, {size, retainedSize, count, internalCount, deadCount}] of this.heapSnapshot.categories) {
+        for (let [className, {size, retainedSize, count, internalCount, deadCount, objectCount}] of this.heapSnapshot.categories) {
+            console.assert(count > 0);
             if (count === internalCount)
                 continue;
 
@@ -225,7 +226,11 @@ WebInspector.HeapSnapshotInstancesDataGridTree = class HeapSnapshotInstancesData
             if (!liveCount)
                 continue;
 
-            this.appendChild(new WebInspector.HeapSnapshotClassDataGridNode({className, size, retainedSize, count: liveCount}, this));
+            // If over half of the objects with this class name are Object sub-types, treat this as an Object category.
+            // This can happen if the page has a JavaScript Class with the same name as a native class.
+            let isObjectSubcategory = (objectCount / count) > 0.5;
+
+            this.appendChild(new WebInspector.HeapSnapshotClassDataGridNode({className, size, retainedSize, isObjectSubcategory, count: liveCount}, this));
         }
 
         this.didPopulate();
index fc4229b..070d5be 100644 (file)
@@ -146,12 +146,12 @@ WebInspector.HeapSnapshotInstanceDataGridNode = class HeapSnapshotInstanceDataGr
             return Number.bytesToString(this._node.size);
 
         if (columnIdentifier === "className") {
-            let {className, id, internal} = this._node;
+            let {className, id, internal, isObjectType} = this._node;
             let containerElement = document.createElement("span");
             containerElement.addEventListener("contextmenu", this._contextMenuHandler.bind(this));
 
             let iconElement = containerElement.appendChild(document.createElement("img"));
-            iconElement.classList.add("icon", WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className, internal));
+            iconElement.classList.add("icon", WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className, internal, isObjectType));
 
             if (this._edge) {
                 let nameElement = containerElement.appendChild(document.createElement("span"));
@@ -410,7 +410,7 @@ WebInspector.HeapSnapshotInstanceDataGridNode = class HeapSnapshotInstanceDataGr
             containerElement.classList.add("node");
 
             let iconElement = containerElement.appendChild(document.createElement("img"));
-            iconElement.classList.add("icon", WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(node.className, node.internal));
+            iconElement.classList.add("icon", WebInspector.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(node.className, node.internal, node.isObjectType));
 
             let classNameElement = containerElement.appendChild(document.createElement("span"));
             classNameElement.textContent = sanitizeClassName(node.className) + " ";
index b9e14dd..891af4e 100644 (file)
  */
 
 // nodes
-// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:internal>]
+// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:flags>]
 const nodeFieldCount = 4;
 const nodeIdOffset = 0;
 const nodeSizeOffset = 1;
 const nodeClassNameOffset = 2;
-const nodeInternalOffset = 3;
+const nodeFlagsOffset = 3;
+
+// node flags
+const internalFlagsMask = (1 << 0);
+const objectTypeMask = (1 << 1);
 
 // edges
 // [<0:fromId>, <1:toId>, <2:typeTableIndex>, <3:edgeDataIndexOrEdgeNameIndex>]
@@ -50,6 +54,10 @@ const rootNodeIndex = 0;
 const rootNodeOrdinal = 0;
 const rootNodeIdentifier = 0;
 
+// Version Differences:
+//   - In Version 1, node[3] now named <flags> was the value 0 or 1 indicating not-internal or internal.
+//   - In Version 2, this became a bitmask so multiple flags could be included without modifying the size.
+//
 // Terminology:
 //   - `nodeIndex` is an index into the `nodes` list.
 //   - `nodeOrdinal` is the order of the node in the `nodes` list. (nodeIndex / nodeFieldCount).
@@ -83,7 +91,7 @@ HeapSnapshot = class HeapSnapshot
         snapshotDataString = null;
 
         let {version, nodes, nodeClassNames, edges, edgeTypes, edgeNames} = json;
-        console.assert(version === 1, "Expect JavaScriptCore Heap Snapshot version 1");
+        console.assert(version === 1 || version === 2, "Expect JavaScriptCore Heap Snapshot version 1 or 2");
 
         this._nodes = nodes;
         this._nodeCount = nodes.length / nodeFieldCount;
@@ -160,18 +168,20 @@ HeapSnapshot = class HeapSnapshot
             let className = nodeClassNamesTable[classNameTableIndex];
             let size = nodes[nodeIndex + nodeSizeOffset];
             let retainedSize = nodeOrdinalToRetainedSizes[nodeOrdinal];
-            let internal = nodes[nodeIndex + nodeInternalOffset] ? true : false;
+            let flags = nodes[nodeIndex + nodeFlagsOffset];
             let dead = nodeOrdinalIsDead[nodeOrdinal] ? true : false;
 
             let category = categories[className];
             if (!category)
-                category = categories[className] = {className, size: 0, retainedSize: 0, count: 0, internalCount: 0, deadCount: 0};
+                category = categories[className] = {className, size: 0, retainedSize: 0, count: 0, internalCount: 0, deadCount: 0, objectCount: 0};
 
             category.size += size;
             category.retainedSize += retainedSize;
             category.count += 1;
-            if (internal)
+            if (flags & internalFlagsMask)
                 category.internalCount += 1;
+            if (flags & objectTypeMask)
+                category.objectCount += 1;
             if (dead)
                 category.deadCount += 1;
             else
@@ -413,6 +423,7 @@ HeapSnapshot = class HeapSnapshot
         let nodeOrdinal = nodeIndex / nodeFieldCount;
         let edgeIndex = this._nodeOrdinalToFirstOutgoingEdge[nodeOrdinal];
         let hasChildren = this._edges[edgeIndex + edgeFromIdOffset] === nodeIdentifier;
+        let nodeFlags = this._nodes[nodeIndex + nodeFlagsOffset];
 
         let dominatorNodeOrdinal = this._nodeOrdinalToDominatorNodeOrdinal[nodeOrdinal];
         let dominatorNodeIndex = dominatorNodeOrdinal * nodeFieldCount;
@@ -423,7 +434,8 @@ HeapSnapshot = class HeapSnapshot
             className: this._nodeClassNamesTable[this._nodes[nodeIndex + nodeClassNameOffset]],
             size: this._nodes[nodeIndex + nodeSizeOffset],
             retainedSize: this._nodeOrdinalToRetainedSizes[nodeOrdinal],
-            internal: this._nodes[nodeIndex + nodeInternalOffset] ? true : false,
+            internal: nodeFlags & internalFlagsMask ? true : false,
+            isObjectType: nodeFlags & objectTypeMask ? true : false,
             gcRoot: this._nodeOrdinalIsGCRoot[nodeOrdinal] ? true : false,
             dead: this._nodeOrdinalIsDead[nodeOrdinal] ? true : false,
             dominatorNodeIdentifier,
@@ -750,7 +762,7 @@ HeapSnapshot = class HeapSnapshot
             for (let incomingEdgeIndex = incomingEdgeIndexEnd - 1; incomingEdgeIndex >= incomingEdgeIndexStart; --incomingEdgeIndex) {
                 let fromNodeOrdinal = this._incomingNodes[incomingEdgeIndex];
                 let fromNodeIndex = fromNodeOrdinal * nodeFieldCount;
-                let fromNodeIsInternal = this._nodes[fromNodeIndex + nodeInternalOffset];
+                let fromNodeIsInternal = this._nodes[fromNodeIndex + nodeFlagsOffset] & internalFlagsMask;
                 if (fromNodeIsInternal)
                     continue;