Bug 86204 – Web Inspector: heap profiler should allow revealing an element which is logged to the console

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Yury Semikhatsky]

At the moment we can select an element in a heap snapshot and than access the element in the console expressions using $0 variable. It is logical to have backward navigation: from an object in the console to the corresponding heap snapshot entry.

[Yury Semikhatsky]

Created attachment 141391 [details]
Patch

[Early Warning System Bot]

Comment on attachment 141391 [details]
Patch

Attachment 141391 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12674164

[Build Bot]

Comment on attachment 141391 [details]
Patch

Attachment 141391 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12663686

[Gyuyoung Kim]

Comment on attachment 141391 [details]
Patch

Attachment 141391 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/12664666

[Early Warning System Bot]

Comment on attachment 141391 [details]
Patch

Attachment 141391 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12672307

[Yury Semikhatsky]

Created attachment 141400 [details]
Patch

[Pavel Feldman]

Comment on attachment 141400 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141400&action=review

> Source/WebCore/ChangeLog:8
> +        JS objects in the console have context menu item that allows to reveal them in a heap snapshot view.

Please be more verbose here, what is SnapshotObjectId?

> Source/WebCore/inspector/InjectedScript.cpp:193
> +ScriptValue InjectedScript::findObjectById(const String& objectId) const

objectForId?

> Source/WebCore/inspector/Inspector.json:2680
> +                    { "name": "heapSnapshotObjectId", "type": "string", "description": "Heap snapshot object id." }

Please define a type for object id.

> Source/WebCore/inspector/InspectorProfilerAgent.cpp:455
> +    ScriptValue jsValue = injectedScript.findObjectById(objectId);

jsValue name is misleading - I started thinking it was jsc-specific.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1569
> +    serializeItemsRange: function(begin, end)

Please annotate.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1572
> +        if (end >= this._iterationOrder.length)

Is this a part of this change?

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1626
> +    serializeItem: function(edge)

Please annotate.

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1721
> +    getNodePosition: function(snapshotObjectId)

Please annotate.

> Source/WebCore/inspector/front-end/HeapSnapshotDataGrids.js:73
> +    selectObjectByHeapSnapshotId: function(heapSnapshotObjectId)

Please annotate.

> Source/WebCore/inspector/front-end/HeapSnapshotDataGrids.js:392
> +    selectObjectByHeapSnapshotId: function(id)

Please annotate.

> Source/WebCore/inspector/front-end/HeapSnapshotGridNodes.js:158
> +            if (!this._positionRanges.length) {

Is it the part of this change?

> Source/WebCore/inspector/front-end/HeapSnapshotGridNodes.js:724
> +        WebInspector.log("findAndSerializeItems " + snapshotObjectId);

Please remove logging.

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:64
> +            WebInspector.log("revealInDominatorsView " + objectId);

Please remove logging.

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:68
> +            ProfilerAgent.getHeapObjectId(objectId, didReceiveHeapObjectId.bind(this));

ObjectPropertiesSection is in components module, outsite profiler module. Ideally, this should not compile.

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:73
> +            WebInspector.log("didReceiveHeapObjectId error = " + error + " result = " + result);

logging.

[Yury Semikhatsky]

Created attachment 141581 [details]
Patch

[Pavel Feldman]

Comment on attachment 141581 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=141581&action=review

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1289
> +    getNodeClassName: function(snapshotObjectId)

nodeClassName:

> Source/WebCore/inspector/front-end/HeapSnapshot.js:1605
> +    getNodePosition: function(snapshotObjectId)

nodePosition

> Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:80
> +WebInspector.ObjectPropertiesSection.addContextMenuProvier = function(provider)

Provider

> Source/WebCore/inspector/front-end/ProfilesPanel.js:1066
> +WebInspector.RevealInHeapSnapshotContextMenuProvier = function()

Provider

[Yury Semikhatsky]

(In reply to comment #9)
> (From update of attachment 141581 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=141581&action=review
> 
> > Source/WebCore/inspector/front-end/HeapSnapshot.js:1289
> > +    getNodeClassName: function(snapshotObjectId)
> 
> nodeClassName:
> 
Done.

> > Source/WebCore/inspector/front-end/HeapSnapshot.js:1605
> > +    getNodePosition: function(snapshotObjectId)
> 
> nodePosition
> 
Done.

> > Source/WebCore/inspector/front-end/ObjectPropertiesSection.js:80
> > +WebInspector.ObjectPropertiesSection.addContextMenuProvier = function(provider)
> 
> Provider
> 
Done.

> > Source/WebCore/inspector/front-end/ProfilesPanel.js:1066
> > +WebInspector.RevealInHeapSnapshotContextMenuProvier = function()
> 
> Provider
Done.

[Yury Semikhatsky]

Committed r116857: <http://trac.webkit.org/changeset/116857>