Bug 94351 – Web Inspector: make profiles panel a lazily loaded module.

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

[Pavel Feldman]

Patch to follow.

[Pavel Feldman]

Created attachment 159134 [details]
Patch

[Yury Semikhatsky]

Comment on attachment 159134 [details]
Patch

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

> Source/WebCore/inspector/front-end/HeapSnapshotView.js:411
> +        return this.parent.getProfiles(WebInspector.HeapSnapshotProfileType.TypeId);

I'd wrap this.parent into a _profilesPanel() getter.

> Source/WebCore/inspector/front-end/InspectorFrontendAPI.js:60
> +        return WebInspector.panels.profiles && WebInspector.CPUProfileType.instance && WebInspector.CPUProfileType.instance.isRecordingProfile();

Is this change really needed?

> Source/WebCore/inspector/front-end/ProfilesPanel.js:1102
> +        if (WebInspector.inspectorView.currentPanel() !== WebInspector.ProfilesPanel._instance)

Please pass reference to ProfilesPanel directly into RevealInHeapSnapshotContextMenuProvider constructor. You wouldn't need WebInspector.ProfilesPanel._instance then.

[Pavel Feldman]

(In reply to comment #2)
> (From update of attachment 159134 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=159134&action=review
> 
> > Source/WebCore/inspector/front-end/HeapSnapshotView.js:411
> > +        return this.parent.getProfiles(WebInspector.HeapSnapshotProfileType.TypeId);
> 
> I'd wrap this.parent into a _profilesPanel() getter.
> 

I think it should be renamed, but I am hesitant touching this .parent guy - it all may collapse :). On a broader note, I don't think view needs a pointer to the panel - we need to fix the design.

> > Source/WebCore/inspector/front-end/InspectorFrontendAPI.js:60
> > +        return WebInspector.panels.profiles && WebInspector.CPUProfileType.instance && WebInspector.CPUProfileType.instance.isRecordingProfile();
> 
> Is this change really needed?
> 

Yes, on default WebKit container, if module is not yet loaded, then profiling is not enabled.

> > Source/WebCore/inspector/front-end/ProfilesPanel.js:1102
> > +        if (WebInspector.inspectorView.currentPanel() !== WebInspector.ProfilesPanel._instance)
> 
> Please pass reference to ProfilesPanel directly into RevealInHeapSnapshotContextMenuProvider constructor. You wouldn't need WebInspector.ProfilesPanel._instance then.

As we agreed offline, it is a part of a bigger change.

Uploading new version of the patch with tests fixed.

[Pavel Feldman]

Created attachment 159138 [details]
[Patch] for landing

[Pavel Feldman]

Created attachment 159141 [details]
[Patch] for landing 2

[Pavel Feldman]

Created attachment 159146 [details]
[Patch] for landing 3

[WebKit Review Bot]

Comment on attachment 159146 [details]
[Patch] for landing 3

Clearing flags on attachment: 159146

Committed r125922: <http://trac.webkit.org/changeset/125922>

[WebKit Review Bot]

Comment on attachment 159134 [details]
Patch

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

New failing tests:
inspector/profiler/heap-snapshot-loader.html

[WebKit Review Bot]

Created attachment 159173 [details]
Archive of layout-test-results from gce-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Brian Burg]

Closing as invalid, as this bug pertains to the old inspector UI and/or its tests.
Please file a new bug (https://www.webkit.org/new-inspector-bug) if the bug/feature/issue is still relevant to WebKit trunk.