Unreviewed, rolling out r241784.
authortsavell@apple.com <tsavell@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Feb 2019 00:38:20 +0000 (00:38 +0000)
committertsavell@apple.com <tsavell@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 20 Feb 2019 00:38:20 +0000 (00:38 +0000)
Broke all OpenSource builds.

Reverted changeset:

"Web Inspector: Improve ES6 Class instances in Heap Snapshot
instances view"
https://bugs.webkit.org/show_bug.cgi?id=172848
https://trac.webkit.org/changeset/241784

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

21 files changed:
JSTests/ChangeLog
JSTests/heapProfiler/basic-nodes.js
JSTests/heapProfiler/class-names.js [deleted file]
JSTests/heapProfiler/driver/driver.js
JSTests/heapProfiler/variable-edge-types.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/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
Tools/ChangeLog
Tools/GCHeapInspector/heap-analysis/HeapSnapshot.js

index 8c4089c..65c701c 100644 (file)
@@ -1,3 +1,16 @@
+2019-02-19  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r241784.
+
+        Broke all OpenSource builds.
+
+        Reverted changeset:
+
+        "Web Inspector: Improve ES6 Class instances in Heap Snapshot
+        instances view"
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        https://trac.webkit.org/changeset/241784
+
 2019-02-19  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
index 6acd29f..c633936 100644 (file)
@@ -1,5 +1,3 @@
-var SimpleObject = $vm.SimpleObject;
-
 load("./driver/driver.js");
 
 function hasDifferentSizeNodes(nodes) {
diff --git a/JSTests/heapProfiler/class-names.js b/JSTests/heapProfiler/class-names.js
deleted file mode 100644 (file)
index 6ffd761..0000000
+++ /dev/null
@@ -1,47 +0,0 @@
-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.every((x) => x.isObjectType), "Every 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.every((x) => x.isObjectType), "Every 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.every((x) => x.isObjectType), "Every MyES6Subclass instance should have its ObjectType flag set");
-
-    nodes = snapshot.nodesWithClassName("Function");
-    assert(nodes.length > 0, "Should be at least 1 Function instance");
-    assert(nodes.every((x) => !x.isObjectType), "No Function instance should have its ObjectType flag set");
-
-    nodes = snapshot.nodesWithClassName("Map");
-    assert(nodes.length > 0, "Should be at least 1 Map instance");
-    assert(nodes.every((x) => !x.isObjectType), "No Map instance should have its ObjectType flag set");
-
-    nodes = snapshot.nodesWithClassName("Object");
-    assert(nodes.every((x) => x.isObjectType), "Every Object should also have its ObjectType flag set");
-})();
index 31666cf..58ee5a8 100644 (file)
@@ -9,19 +9,15 @@ function assert(condition, reason) {
 // Contains two large lists of all node data and all edge data.
 // Lazily creates node and edge objects off of indexes into these lists.
 
-// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:flags>, <4:firstEdgeIndex>];
+// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:internal>, <4:firstEdgeIndex>];
 const nodeFieldCount = 5;
 const nodeIdOffset = 0;
 const nodeSizeOffset = 1;
 const nodeClassNameOffset = 2;
-const nodeFlagsOffset = 3;
+const nodeInternalOffset = 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;
@@ -39,10 +35,7 @@ CheapHeapSnapshotNode = class CheapHeapSnapshotNode
         this.id = nodes[nodeIndex + nodeIdOffset];
         this.size = nodes[nodeIndex + nodeSizeOffset];
         this.className = snapshot.classNameFromTableIndex(nodes[nodeIndex + nodeClassNameOffset]);
-
-        let flags = nodes[nodeIndex + nodeFlagsOffset];
-        this.internal = flags & internalFlagMask ? true : false;
-        this.isObjectType = flags & objectTypeMask ? true : false;
+        this.internal = nodes[nodeIndex + nodeInternalOffset] ? true : false;
 
         this.outgoingEdges = [];
         let firstEdgeIndex = nodes[nodeIndex + nodeFirstEdgeOffset];
@@ -98,7 +91,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++]; // flags
+            this._nodes[n++] = nodes[i++]; // internal
             this._nodes[n++] = nodeNoEdgeValue;
         }
 
@@ -161,7 +154,7 @@ function createCheapHeapSnapshot() {
     let json = generateHeapSnapshot();
 
     let {version, nodes, nodeClassNames, edges, edgeTypes} = json;
-    assert(version === 2, "Heap Snapshot payload should be version 2");
+    assert(version === 1, "Heap Snapshot payload should be version 1");
     assert(nodes.length, "Heap Snapshot should have nodes");
     assert(nodeClassNames.length, "Heap Snapshot should have nodeClassNames");
     assert(edges.length, "Heap Snapshot should have edges");
@@ -217,8 +210,7 @@ HeapSnapshot = class HeapSnapshot
             let id = nodes[i++];
             let size = nodes[i++];
             let classNameIndex = nodes[i++];
-            let flags = nodes[i++];
-            let internal = flags & internalFlagMask ? true : false;
+            let internal = nodes[i++];
 
             let node = new HeapSnapshotNode(id, nodeClassNames[classNameIndex], size, internal);
             this.nodeMap.set(id, node);
@@ -264,7 +256,7 @@ function createHeapSnapshot() {
     let json = generateHeapSnapshot();
 
     let {version, nodes, nodeClassNames, edges, edgeTypes} = json;
-    assert(version === 2, "Heap Snapshot payload should be version 2");
+    assert(version === 1, "Heap Snapshot payload should be version 1");
     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 730f17c..92a675e 100644 (file)
@@ -1,5 +1,3 @@
-var SimpleObject = $vm.SimpleObject;
-
 load("./driver/driver.js");
 
 let globalScopeVariable = "globalScopeVariableValue";
index 03855b5..9bc6451 100644 (file)
@@ -5,10 +5,6 @@ load("./driver/driver.js");
 function wrapper()
 {
 
-function A() { };
-function B() { };
-function C() { };
-
 var theA = new A;
 var theB = new B;
 var theC = new C;
@@ -20,8 +16,9 @@ secondHierarchyTest = Object.create(theC);
 
 var secondB = Object.create(theB);
 
-B.prototype.__proto__ = A.prototype;
-C.prototype.__proto__ = A.prototype;
+function A() { };
+function B() { }; B.prototype.__proto__ = A.prototype;
+function C() { }; C.prototype.__proto__ = A.prototype;
 
 }
 wrapper();
index 46fc354..6cd435c 100644 (file)
@@ -1,3 +1,16 @@
+2019-02-19  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r241784.
+
+        Broke all OpenSource builds.
+
+        Reverted changeset:
+
+        "Web Inspector: Improve ES6 Class instances in Heap Snapshot
+        instances view"
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        https://trac.webkit.org/changeset/241784
+
 2019-02-19  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
index af5f709..5ef12a8 100644 (file)
@@ -23,7 +23,6 @@ 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 197c0b5..60bf289 100644 (file)
@@ -9,13 +9,12 @@ function test()
 
     WI.TestHeapSnapshotNode = class TestHeapSnapshotNode
     {
-        constructor(identifier, className, size, flags)
+        constructor(identifier, className, size, internal)
         {
             this.id = identifier;
             this.className = className;
             this.size = size; 
-            this.internal = flags & (1 << 0) ? true : false;
-            this.isObjectType = flags & (1 << 1) ? true : false;
+            this.internal = internal;
             this.gcRoot = false;
             this.outgoingEdges = [];
             this.incomingEdges = [];
@@ -60,9 +59,9 @@ function test()
                 let id = nodes[i++];
                 let size = nodes[i++];
                 let classNameIndex = nodes[i++];
-                let flags = nodes[i++];
+                let internal = nodes[i++];
 
-                let node = new WI.TestHeapSnapshotNode(id, nodeClassNames[classNameIndex], size, flags);
+                let node = new WI.TestHeapSnapshotNode(id, nodeClassNames[classNameIndex], size, !!internal);
                 nodeMap.set(id, node);
                 processedNodes.push(node);
             }
@@ -128,7 +127,6 @@ function test()
             && node1.size === node2.size
             && node1.className === node2.className
             && node1.internal === node2.internal
-            && node1.isObjectType === node2.isObjectType
             && node1.gcRoot === node2.gcRoot;
     }
 
@@ -191,7 +189,6 @@ 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 3cd7b4f..892ef3a 100644 (file)
@@ -1,3 +1,16 @@
+2019-02-19  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r241784.
+
+        Broke all OpenSource builds.
+
+        Reverted changeset:
+
+        "Web Inspector: Improve ES6 Class instances in Heap Snapshot
+        instances view"
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        https://trac.webkit.org/changeset/241784
+
 2019-02-19  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
index 4a56c6f..233f683 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2016-2019 Apple Inc. All rights reserved.
+ * Copyright (C) 2016 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"
@@ -79,7 +79,7 @@ void HeapSnapshotBuilder::buildSnapshot()
 void HeapSnapshotBuilder::appendNode(JSCell* cell)
 {
     ASSERT(m_profiler.activeSnapshotBuilder() == this);
-
+    
     ASSERT(Heap::isMarked(cell));
 
     NodeIdentifier identifier;
@@ -177,12 +177,12 @@ bool HeapSnapshotBuilder::previousSnapshotHasNodeForCell(JSCell* cell, NodeIdent
 //  Inspector snapshots:
 //
 //   {
-//      "version": 2,
+//      "version": 1.0,
 //      "type": "Inspector",
-//      // [<address>, <labelIndex>, <wrappedAddress>] only present in GCDebuggingSnapshot-type snapshots
+//      // [<address>, <labelIndex>, <wrappedEddress>] only present in GCDebuggingSnapshot-type snapshots
 //      "nodes": [
-//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <flags>
-//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <flags>
+//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <internal>
+//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <internal>
 //          ...
 //      ],
 //      "nodeClassNames": [
@@ -204,11 +204,11 @@ bool HeapSnapshotBuilder::previousSnapshotHasNodeForCell(JSCell* cell, NodeIdent
 //  GC heap debugger snapshots:
 //
 //   {
-//      "version": 2,
+//      "version": 1.0,
 //      "type": "GCDebugging",
 //      "nodes": [
-//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <flags>, <labelIndex>, <cellEddress>, <wrappedAddress>,
-//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <flags>, <labelIndex>, <cellEddress>, <wrappedAddress>,
+//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <internal>, <labelIndex>, <cellEddress>, <wrappedEddress>,
+//          <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <internal>, <labelIndex>, <cellEddress>, <wrappedEddress>,
 //          ...
 //      ],
 //      "nodeClassNames": [
@@ -240,10 +240,8 @@ bool HeapSnapshotBuilder::previousSnapshotHasNodeForCell(JSCell* cell, NodeIdent
 //     <nodeClassNameIndex>
 //       - index into the "nodeClassNames" list.
 //
-//     <flags>
-//       - 0b0000 - no flags
-//       - 0b0001 - internal instance
-//       - 0b0010 - Object subclassification
+//     <internal>
+//       - 0 = false, 1 = true.
 //
 //     <edgeTypeIndex>
 //       - index into the "edgeTypes" list.
@@ -256,11 +254,6 @@ bool HeapSnapshotBuilder::previousSnapshotHasNodeForCell(JSCell* cell, NodeIdent
 //      <rootReasonIndex>
 //       - index into the "labels" list.
 
-enum class NodeFlags {
-    Internal      = 1 << 0,
-    ObjectSubtype = 1 << 1,
-};
-
 static uint8_t edgeTypeToNumber(EdgeType type)
 {
     return static_cast<uint8_t>(type);
@@ -365,8 +358,8 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
     HashMap<JSCell*, NodeIdentifier> allowedNodeIdentifiers;
 
     // Build a list of used class names.
-    HashMap<String, unsigned> classNameIndexes;
-    classNameIndexes.set("<root>"_s, 0);
+    HashMap<const char*, unsigned> classNameIndexes;
+    classNameIndexes.set("<root>", 0);
     unsigned nextClassNameIndex = 1;
 
     // Build a list of labels (this is just a string table).
@@ -385,37 +378,19 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
         if (!allowNodeCallback(node))
             return;
 
-        unsigned flags = 0;
-
         allowedNodeIdentifiers.set(node.cell, node.identifier);
 
-        String className = node.cell->classInfo(vm)->className;
-        if (node.cell->isObject() && className == JSObject::info()->className) {
-            flags |= NodeFlags::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 the prototype's parent.
-            JSObject* object = asObject(node.cell);
-            if (JSGlobalObject* globalObject = object->globalObject(vm)) {
-                ExecState* exec = globalObject->globalExec();
-                PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry);
-                if (!object->getOwnPropertySlot(object, exec, vm.propertyNames->constructor, slot))
-                    className = JSObject::calculatedClassName(object);
-            }
-        }
-
-        auto result = classNameIndexes.add(className, nextClassNameIndex);
+        auto result = classNameIndexes.add(node.cell->classInfo(vm)->className, nextClassNameIndex);
         if (result.isNewEntry)
             nextClassNameIndex++;
         unsigned classNameIndex = result.iterator->value;
 
+        bool isInternal = false;
         void* wrappedAddress = 0;
         unsigned labelIndex = 0;
         if (!node.cell->isString()) {
             Structure* structure = node.cell->structure(vm);
-            if (!structure || !structure->globalObject())
-                flags |= NodeFlags::Internal;
+            isInternal = !structure || !structure->globalObject();
 
             if (m_snapshotType == SnapshotType::GCDebuggingSnapshot) {
                 String nodeLabel;
@@ -429,7 +404,7 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
                             nodeLabel = function->calculatedDisplayName(vm);
                     }
                 }
-
+                
                 String description = descriptionForCell(node.cell);
                 if (description.length()) {
                     if (nodeLabel.length())
@@ -443,12 +418,12 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
                         nextLabelIndex++;
                     labelIndex = result.iterator->value;
                 }
-
+                
                 wrappedAddress = m_wrappedObjectPointers.get(node.cell);
             }
         }
 
-        // <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <flags>, [<labelIndex>, <cellEddress>, <wrappedAddress>]
+        // <nodeId>, <sizeInBytes>, <nodeClassNameIndex>, <internal>, [<labelIndex>, <cellEddress>, <wrappedEddress>]
         json.append(',');
         json.appendNumber(node.identifier);
         json.append(',');
@@ -456,7 +431,7 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
         json.append(',');
         json.appendNumber(classNameIndex);
         json.append(',');
-        json.appendNumber(flags);
+        json.append(isInternal ? '1' : '0');
         if (m_snapshotType == SnapshotType::GCDebuggingSnapshot) {
             json.append(',');
             json.appendNumber(labelIndex);
@@ -504,7 +479,7 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
     json.append('{');
 
     // version
-    json.appendLiteral("\"version\":2");
+    json.appendLiteral("\"version\":1");
 
     // type
     json.append(',');
@@ -531,7 +506,7 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
     json.append(',');
     json.appendLiteral("\"nodeClassNames\":");
     json.append('[');
-    Vector<String> orderedClassNames(classNameIndexes.size());
+    Vector<const char *> orderedClassNames(classNameIndexes.size());
     for (auto& entry : classNameIndexes)
         orderedClassNames[entry.value] = entry.key;
     classNameIndexes.clear();
@@ -628,7 +603,7 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
         json.append(',');
         json.appendLiteral("\"roots\":");
         json.append('[');
-
+        
         HeapSnapshot* snapshot = m_profiler.mostRecentSnapshot();
 
         bool firstNode = true;
@@ -644,7 +619,7 @@ String HeapSnapshotBuilder::json(Function<bool (const HeapSnapshotNode&)> allowN
 
             firstNode = false;
             json.appendNumber(snapshotNode.value().identifier);
-
+            
             // Maybe we should just always encode the root names.
             const char* rootName = rootTypeToString(it.value.markReason);
             auto result = labelIndexes.add(rootName, nextLabelIndex);
index 3da1b86..1b5c917 100644 (file)
@@ -524,61 +524,35 @@ String JSObject::toStringName(const JSObject* object, ExecState* exec)
 
 String JSObject::calculatedClassName(JSObject* object)
 {
-    String constructorFunctionName;
-    auto* structure = object->structure();
-    auto* globalObject = structure->globalObject();
+    String prototypeFunctionName;
+    auto globalObject = object->globalObject();
     VM& vm = globalObject->vm();
     auto scope = DECLARE_CATCH_SCOPE(vm);
-    auto* exec = globalObject->globalExec();
 
-    // Check for a display name of obj.constructor.
-    // This is useful to get `Foo` for the `(class Foo).prototype` object.
-    PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry);
-    if (object->getOwnPropertySlot(object, exec, vm.propertyNames->constructor, slot)) {
+    ExecState* exec = globalObject->globalExec();
+    PropertySlot slot(object->getPrototypeDirect(vm), PropertySlot::InternalMethodType::VMInquiry);
+    PropertyName constructor(vm.propertyNames->constructor);
+    if (object->getPropertySlot(exec, constructor, slot)) {
         EXCEPTION_ASSERT(!scope.exception());
         if (slot.isValue()) {
-            if (JSObject* ctorObject = jsDynamicCast<JSObject*>(vm, slot.getValue(exec, vm.propertyNames->constructor))) {
-                if (JSFunction* constructorFunction = jsDynamicCast<JSFunction*>(vm, ctorObject))
-                    constructorFunctionName = constructorFunction->calculatedDisplayName(vm);
-                else if (InternalFunction* constructorFunction = jsDynamicCast<InternalFunction*>(vm, ctorObject))
-                    constructorFunctionName = constructorFunction->calculatedDisplayName(vm);
-            }
-        }
-    }
-
-    EXCEPTION_ASSERT(!scope.exception() || constructorFunctionName.isNull());
-    if (UNLIKELY(scope.exception()))
-        scope.clearException();
-
-    // Get the display name of obj.__proto__.constructor.
-    // This is useful to get `Foo` for a `new Foo` object.
-    if (constructorFunctionName.isNull()) {
-        MethodTable::GetPrototypeFunctionPtr defaultGetPrototype = JSObject::getPrototype;
-        if (LIKELY(structure->classInfo()->methodTable.getPrototype == defaultGetPrototype)) {
-            JSValue protoValue = object->getPrototypeDirect(vm);
-            if (protoValue.isObject()) {
-                JSObject* protoObject = asObject(protoValue);
-                PropertySlot slot(protoValue, PropertySlot::InternalMethodType::VMInquiry);
-                if (protoObject->getPropertySlot(exec, vm.propertyNames->constructor, slot)) {
-                    EXCEPTION_ASSERT(!scope.exception());
-                    if (slot.isValue()) {
-                        if (JSObject* ctorObject = jsDynamicCast<JSObject*>(vm, slot.getValue(exec, vm.propertyNames->constructor))) {
-                            if (JSFunction* constructorFunction = jsDynamicCast<JSFunction*>(vm, ctorObject))
-                                constructorFunctionName = constructorFunction->calculatedDisplayName(vm);
-                            else if (InternalFunction* constructorFunction = jsDynamicCast<InternalFunction*>(vm, ctorObject))
-                                constructorFunctionName = constructorFunction->calculatedDisplayName(vm);
-                        }
+            JSValue constructorValue = slot.getValue(exec, constructor);
+            if (constructorValue.isCell()) {
+                if (JSCell* constructorCell = constructorValue.asCell()) {
+                    if (JSObject* ctorObject = constructorCell->getObject()) {
+                        if (JSFunction* constructorFunction = jsDynamicCast<JSFunction*>(vm, ctorObject))
+                            prototypeFunctionName = constructorFunction->calculatedDisplayName(vm);
+                        else if (InternalFunction* constructorFunction = jsDynamicCast<InternalFunction*>(vm, ctorObject))
+                            prototypeFunctionName = constructorFunction->calculatedDisplayName(vm);
                     }
                 }
             }
         }
     }
-
-    EXCEPTION_ASSERT(!scope.exception() || constructorFunctionName.isNull());
+    EXCEPTION_ASSERT(!scope.exception() || prototypeFunctionName.isNull());
     if (UNLIKELY(scope.exception()))
         scope.clearException();
 
-    if (constructorFunctionName.isNull() || constructorFunctionName == "Object") {
+    if (prototypeFunctionName.isNull() || prototypeFunctionName == "Object") {
         String tableClassName = object->methodTable(vm)->className(object, vm);
         if (!tableClassName.isNull() && tableClassName != "Object")
             return tableClassName;
@@ -587,11 +561,11 @@ String JSObject::calculatedClassName(JSObject* object)
         if (!classInfoName.isNull())
             return classInfoName;
 
-        if (constructorFunctionName.isNull())
+        if (prototypeFunctionName.isNull())
             return "Object"_s;
     }
 
-    return constructorFunctionName;
+    return prototypeFunctionName;
 }
 
 bool JSObject::getOwnPropertySlotByIndex(JSObject* thisObject, ExecState* exec, unsigned i, PropertySlot& slot)
index 60fd651..8c96d20 100644 (file)
@@ -1,3 +1,16 @@
+2019-02-19  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r241784.
+
+        Broke all OpenSource builds.
+
+        Reverted changeset:
+
+        "Web Inspector: Improve ES6 Class instances in Heap Snapshot
+        instances view"
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        https://trac.webkit.org/changeset/241784
+
 2019-02-19  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Improve ES6 Class instances in Heap Snapshot instances view
index d44408a..6e50452 100644 (file)
 
 WI.HeapSnapshotNodeProxy = class HeapSnapshotNodeProxy
 {
-    constructor(snapshotObjectId, {id, className, size, retainedSize, internal, isObjectType, gcRoot, dead, dominatorNodeIdentifier, hasChildren})
+    constructor(snapshotObjectId, identifier, className, size, retainedSize, internal, gcRoot, dead, dominatorNodeIdentifier, hasChildren)
     {
         this._proxyObjectId = snapshotObjectId;
 
-        this.id = id;
+        this.id = identifier;
         this.className = className;
         this.size = size;
         this.retainedSize = retainedSize;
         this.internal = internal;
-        this.isObjectType = isObjectType;
         this.gcRoot = gcRoot;
         this.dead = dead;
         this.dominatorNodeIdentifier = dominatorNodeIdentifier;
@@ -45,7 +44,8 @@ WI.HeapSnapshotNodeProxy = class HeapSnapshotNodeProxy
 
     static deserialize(objectId, serializedNode)
     {
-        return new WI.HeapSnapshotNodeProxy(objectId, serializedNode);
+        let {id, className, size, retainedSize, internal, gcRoot, dead, dominatorNodeIdentifier, hasChildren} = serializedNode;
+        return new WI.HeapSnapshotNodeProxy(objectId, id, className, size, retainedSize, internal, gcRoot, dead, dominatorNodeIdentifier, hasChildren);
     }
 
     // Proxied
index 5791ac1..369fb00 100644 (file)
@@ -60,11 +60,10 @@ WI.HeapSnapshotClassDataGridNode = class HeapSnapshotClassDataGridNode extends W
             return Number.bytesToString(this._data.size);
 
         if (columnIdentifier === "className") {
-            const internal = false;
-            let {className, isObjectSubcategory} = this._data;
+            let {className} = this._data;
             let fragment = document.createDocumentFragment();
             let iconElement = fragment.appendChild(document.createElement("img"));
-            iconElement.classList.add("icon", WI.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className, internal, isObjectSubcategory));
+            iconElement.classList.add("icon", WI.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className));
             let nameElement = fragment.appendChild(document.createElement("span"));
             nameElement.classList.add("class-name");
             nameElement.textContent = className;
index 1e0d728..d2091ad 100644 (file)
@@ -58,12 +58,10 @@ WI.HeapSnapshotClusterContentView = class HeapSnapshotClusterContentView extends
 
     // Static
 
-    static iconStyleClassNameForClassName(className, internal, isObjectType)
+    static iconStyleClassNameForClassName(className, internal)
     {
         if (internal)
             return "native";
-        if (isObjectType)
-            return "object";
 
         switch (className) {
         case "Object":
index dbac6a7..d142514 100644 (file)
@@ -218,9 +218,7 @@ WI.HeapSnapshotInstancesDataGridTree = class HeapSnapshotInstancesDataGridTree e
         // Populate the first level with the different classes.
         let skipInternalOnlyObjects = !WI.settings.debugShowInternalObjectsInHeapSnapshot.value;
 
-        for (let [className, {size, retainedSize, count, internalCount, deadCount, objectCount}] of this.heapSnapshot.categories) {
-            console.assert(count > 0);
-
+        for (let [className, {size, retainedSize, count, internalCount, deadCount}] of this.heapSnapshot.categories) {
             // Possibly skip internal only classes.
             if (skipInternalOnlyObjects && count === internalCount)
                 continue;
@@ -230,11 +228,7 @@ WI.HeapSnapshotInstancesDataGridTree = class HeapSnapshotInstancesDataGridTree e
             if (!liveCount)
                 continue;
 
-            // 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 WI.HeapSnapshotClassDataGridNode({className, size, retainedSize, isObjectSubcategory, count: liveCount}, this));
+            this.appendChild(new WI.HeapSnapshotClassDataGridNode({className, size, retainedSize, count: liveCount}, this));
         }
 
         this.didPopulate();
index 3248826..566f350 100644 (file)
@@ -146,12 +146,12 @@ WI.HeapSnapshotInstanceDataGridNode = class HeapSnapshotInstanceDataGridNode ext
             return Number.bytesToString(this._node.size);
 
         if (columnIdentifier === "className") {
-            let {className, id, internal, isObjectType} = this._node;
+            let {className, id, internal} = 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", WI.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className, internal, isObjectType));
+            iconElement.classList.add("icon", WI.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(className, internal));
 
             if (this._edge) {
                 let nameElement = containerElement.appendChild(document.createElement("span"));
@@ -410,7 +410,7 @@ WI.HeapSnapshotInstanceDataGridNode = class HeapSnapshotInstanceDataGridNode ext
             containerElement.classList.add("node");
 
             let iconElement = containerElement.appendChild(document.createElement("img"));
-            iconElement.classList.add("icon", WI.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(node.className, node.internal, node.isObjectType));
+            iconElement.classList.add("icon", WI.HeapSnapshotClusterContentView.iconStyleClassNameForClassName(node.className, node.internal));
 
             let classNameElement = containerElement.appendChild(document.createElement("span"));
             classNameElement.textContent = sanitizeClassName(node.className) + " ";
index 16b4504..af6fdff 100644 (file)
  */
 
 // nodes
-// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:flags>]
+// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:internal>]
 const nodeFieldCount = 4;
 const nodeIdOffset = 0;
 const nodeSizeOffset = 1;
 const nodeClassNameOffset = 2;
-const nodeFlagsOffset = 3;
+const nodeInternalOffset = 3;
 const gcDebuggingNodeFieldCount = 7;
 
-// node flags
-const internalFlagsMask = (1 << 0);
-const objectTypeMask = (1 << 1);
-
 // edges
 // [<0:fromId>, <1:toId>, <2:typeTableIndex>, <3:edgeDataIndexOrEdgeNameIndex>]
 const edgeFieldCount = 4;
@@ -55,10 +51,6 @@ 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).
@@ -93,7 +85,7 @@ HeapSnapshot = class HeapSnapshot
         snapshotDataString = null;
 
         let {version, type, nodes, nodeClassNames, edges, edgeTypes, edgeNames} = json;
-        console.assert(version === 1 || version === 2, "Expect JavaScriptCore Heap Snapshot version 1 or 2");
+        console.assert(version === 1, "Expect JavaScriptCore Heap Snapshot version 1");
         console.assert(!type || (type === "Inspector" || type === "GCDebugging"), "Expect an Inspector / GCDebugging Heap Snapshot");
 
         this._nodeFieldCount = type === "GCDebugging" ? gcDebuggingNodeFieldCount : nodeFieldCount;
@@ -173,20 +165,18 @@ HeapSnapshot = class HeapSnapshot
             let className = nodeClassNamesTable[classNameTableIndex];
             let size = nodes[nodeIndex + nodeSizeOffset];
             let retainedSize = nodeOrdinalToRetainedSizes[nodeOrdinal];
-            let flags = nodes[nodeIndex + nodeFlagsOffset];
+            let internal = nodes[nodeIndex + nodeInternalOffset] ? true : false;
             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, objectCount: 0};
+                category = categories[className] = {className, size: 0, retainedSize: 0, count: 0, internalCount: 0, deadCount: 0};
 
             category.size += size;
             category.retainedSize += retainedSize;
             category.count += 1;
-            if (flags & internalFlagsMask)
+            if (internal)
                 category.internalCount += 1;
-            if (flags & objectTypeMask)
-                category.objectCount += 1;
             if (dead)
                 category.deadCount += 1;
             else
@@ -432,7 +422,6 @@ HeapSnapshot = class HeapSnapshot
         let nodeOrdinal = nodeIndex / this._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 * this._nodeFieldCount;
@@ -443,8 +432,7 @@ HeapSnapshot = class HeapSnapshot
             className: this._nodeClassNamesTable[this._nodes[nodeIndex + nodeClassNameOffset]],
             size: this._nodes[nodeIndex + nodeSizeOffset],
             retainedSize: this._nodeOrdinalToRetainedSizes[nodeOrdinal],
-            internal: nodeFlags & internalFlagsMask ? true : false,
-            isObjectType: nodeFlags & objectTypeMask ? true : false,
+            internal: this._nodes[nodeIndex + nodeInternalOffset] ? true : false,
             gcRoot: this._nodeOrdinalIsGCRoot[nodeOrdinal] ? true : false,
             dead: this._nodeOrdinalIsDead[nodeOrdinal] ? true : false,
             dominatorNodeIdentifier,
@@ -771,7 +759,7 @@ HeapSnapshot = class HeapSnapshot
             for (let incomingEdgeIndex = incomingEdgeIndexEnd - 1; incomingEdgeIndex >= incomingEdgeIndexStart; --incomingEdgeIndex) {
                 let fromNodeOrdinal = this._incomingNodes[incomingEdgeIndex];
                 let fromNodeIndex = fromNodeOrdinal * this._nodeFieldCount;
-                let fromNodeIsInternal = this._nodes[fromNodeIndex + nodeFlagsOffset] & internalFlagsMask;
+                let fromNodeIsInternal = this._nodes[fromNodeIndex + nodeInternalOffset];
                 if (fromNodeIsInternal)
                     continue;
 
index 288e9fa..d4b0f29 100644 (file)
@@ -1,3 +1,16 @@
+2019-02-19  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r241784.
+
+        Broke all OpenSource builds.
+
+        Reverted changeset:
+
+        "Web Inspector: Improve ES6 Class instances in Heap Snapshot
+        instances view"
+        https://bugs.webkit.org/show_bug.cgi?id=172848
+        https://trac.webkit.org/changeset/241784
+
 2019-02-19  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][Floats] Remove redundant intersecting logic
index db2b2b7..7275988 100644 (file)
  */
 
 // nodes
-// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:flags>, <4:labelIndex>, <5:address>, <6:wrapped address>]
-const nodeFieldCount = 7;
+// [<0:id>, <1:size>, <2:classNameTableIndex>, <3:internal>, <4:labelIndex>, <5:address>, <6:wrapped address>]
+let nodeFieldCount = 7;
 const nodeIdOffset = 0;
 const nodeSizeOffset = 1;
 const nodeClassNameOffset = 2;
-const nodeFlagsOffset = 3;
+const nodeInternalOffset = 3;
 const nodeLabelOffset = 4;
 const nodeAddressOffset = 5;
 const nodeWrappedAddressOffset = 6;
 
-// node flags
-const internalFlagsMask = (1 << 0);
-const objectTypeMask = (1 << 1);
-
 // edges
 // [<0:fromId>, <1:toId>, <2:typeTableIndex>, <3:edgeDataIndexOrEdgeNameIndex>]
 const edgeFieldCount = 4;
@@ -97,7 +93,7 @@ HeapSnapshot = class HeapSnapshot
         snapshotDataString = null;
 
         let {version, type, nodes, nodeClassNames, edges, edgeTypes, edgeNames, roots, labels} = json;
-        console.assert(version === 1 || version === 2, "Expect JavaScriptCore Heap Snapshot version 1 or 2");
+        console.assert(version === 1, "Expect JavaScriptCore Heap Snapshot version 1");
         console.assert(type === "GCDebugging", "Expect a GCDebugging-type snapshot");
 
         this._nodes = nodes;
@@ -203,20 +199,18 @@ HeapSnapshot = class HeapSnapshot
             let className = nodeClassNamesTable[classNameTableIndex];
             let size = nodes[nodeIndex + nodeSizeOffset];
             let retainedSize = nodeOrdinalToRetainedSizes[nodeOrdinal];
-            let flags = nodes[nodeIndex + nodeFlagsOffset];
+            let internal = nodes[nodeIndex + nodeInternalOffset] ? true : false;
             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, objectCount: 0};
+                category = categories[className] = {className, size: 0, retainedSize: 0, count: 0, internalCount: 0, deadCount: 0};
 
             category.size += size;
             category.retainedSize += retainedSize;
             category.count += 1;
-            if (flags & internalFlagsMask)
+            if (internal)
                 category.internalCount += 1;
-            if (flags & objectTypeMask)
-                category.objectCount += 1;
             if (dead)
                 category.deadCount += 1;
             else
@@ -484,7 +478,6 @@ 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;
@@ -495,8 +488,7 @@ HeapSnapshot = class HeapSnapshot
             className: this._nodeClassNamesTable[this._nodes[nodeIndex + nodeClassNameOffset]],
             size: this._nodes[nodeIndex + nodeSizeOffset],
             retainedSize: this._nodeOrdinalToRetainedSizes[nodeOrdinal],
-            internal: nodeFlags & internalFlagsMask ? true : false,
-            isObjectType: nodeFlags & objectTypeMask ? true : false,
+            internal: this._nodes[nodeIndex + nodeInternalOffset] ? true : false,
             gcRoot: this._nodeOrdinalIsGCRoot[nodeOrdinal] ? true : false,
             markedRoot : this._rootIdentifierToReasons.has(nodeIdentifier),
             dead: this._nodeOrdinalIsDead[nodeOrdinal] ? true : false,
@@ -837,6 +829,9 @@ 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];
+                // if (fromNodeIsInternal)
+                //     continue;
 
                 let edgeIndex = this._incomingEdges[incomingEdgeIndex];
                 currentPath.push({edge: edgeIndex});