Web Inspector: [heap snapshot] It could be useful to have access to the selected heap object from the console.

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

[Ilya Tikhonovsky]

The scenario:
0) make a heap snapshot.
1) select an object
2) open console
3) use it via $$0 property of DOMWindow

[Ilya Tikhonovsky]

Created attachment 126768 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 126768 [details]
Patch

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

> Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:91
> +    ScriptObject object = host->inspectedObject(args[0]->ToInt32()->Value());

You should check for a null object and return a error/undefined here.

> Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp:92
> +    return object.v8Object();

The object may be from a context different from that where inspectedObjectCallback is called, in that case we should return undefined here to avoid object leaks between isolated contexts.

> Source/WebCore/inspector/InjectedScriptHost.cpp:152
> +    if (heapObject.hasNoValue())

No need to check this here.

> Source/WebCore/inspector/InjectedScriptSource.js:582
> +        this.__defineGetter__("$$" + i, bind(commandLineAPIImpl, commandLineAPIImpl._inspectedObject, i));

Let's use existing $* mechanism for addressing recently selected objects.

[WebKit Review Bot]

Comment on attachment 126768 [details]
Patch

Attachment 126768 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11506824

New failing tests:
inspector/protocol/console-agent.html

[Ilya Tikhonovsky]

Created attachment 127128 [details]
Patch

[Philippe Normand]

Comment on attachment 127128 [details]
Patch

Attachment 127128 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11525317

[Early Warning System Bot]

Comment on attachment 127128 [details]
Patch

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

[Gyuyoung Kim]

Comment on attachment 127128 [details]
Patch

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

[WebKit Review Bot]

Comment on attachment 127128 [details]
Patch

Attachment 127128 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11522655

New failing tests:
inspector/protocol/console-agent.html
inspector/extensions/extensions-sidebar.html
inspector/console/command-line-api.html

[Yury Semikhatsky]

Comment on attachment 127128 [details]
Patch

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

> Source/WebCore/inspector/InjectedScriptSource.js:690
> +        console.log(" _inspectedObject: " + num);

Remove this.

> Source/WebCore/inspector/Inspector.json:532
> +                "name": "addInspectedObject",

The method name is to generic, it should be clear that the object id is from heap profile.

> Source/WebCore/inspector/InspectorConsoleAgent.cpp:286
> +    InspectableHeapObject(int heapObjectId) : m_heapObjectId(heapObjectId) { }

Should be marked explicit.

> Source/WebCore/inspector/InspectorConsoleAgent.cpp:289
> +        return ScriptProfiler::objectByHeapObjectId(m_heapObjectId);

What if the object is from a different ScriptState?

> Source/WebCore/inspector/PageConsoleAgent.cpp:68
> +    InspectableNode(Node* node) : m_node(node) { }

Missing explicit.

[Ilya Tikhonovsky]

Created attachment 127212 [details]
Patch

[Ilya Tikhonovsky]

patch for bots
comments were addressed except about ScriptState

[Yury Semikhatsky]

Comment on attachment 127212 [details]
Patch

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

> Source/WebCore/ChangeLog:51
> [phone]  Ilya Tikhonovsky  <[e-mail]>

Please remove duplicate entry.

> Source/WebCore/inspector/InspectorConsoleAgent.cpp:289
> +        return ScriptProfiler::objectByHeapObjectId(m_heapObjectId);

I think we need to reject objects from a different ScriptState here and when wrapping DOM Nodes. This can be done in a separate patch.

[Ilya Tikhonovsky]

Committed r107929: <http://trac.webkit.org/changeset/107929>