Summary: | Web Inspector: make profiles panel a lazily loaded module. | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Pavel Feldman <pfeldman> | ||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Pavel Feldman <pfeldman> | ||||||||||||
Status: | RESOLVED INVALID | ||||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, dglazkov, joepeck, keishi, loislo, pfeldman, pmuellr, rik, webkit.review.bot, yurys | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | 94389 | ||||||||||||||
Bug Blocks: | |||||||||||||||
Attachments: |
|
Description
Pavel Feldman
2012-08-17 08:30:18 PDT
Created attachment 159134 [details]
Patch
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. (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. Created attachment 159138 [details]
[Patch] for landing
Created attachment 159141 [details]
[Patch] for landing 2
Created attachment 159146 [details]
[Patch] for landing 3
Comment on attachment 159146 [details] [Patch] for landing 3 Clearing flags on attachment: 159146 Committed r125922: <http://trac.webkit.org/changeset/125922> 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 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
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. |