Bug 55824

Summary: Web Inspector: [Chromium] Allow dynamic enabling of detailed heap profiles
Product: WebKit Reporter: Mikhail Naganov <mnaganov>
Component: Web Inspector (Deprecated)Assignee: Mikhail Naganov <mnaganov>
Status: RESOLVED FIXED    
Severity: Normal CC: apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 50510    
Attachments:
Description Flags
patch pfeldman: review+, mnaganov: commit-queue-

Description Mikhail Naganov 2011-03-05 07:17:02 PST
The current approach that requires editing DevTools.js is inconvenient -- it should be allowed to enable detailed heap profiles from Inspector UI.

The proposal is to introduce a keyboard combo (e.g. "leakz") that enables detailed heap profiles.
The setting isn't stored anywhere. Instead, after loading the first profile from renderer, its type
is determined, and the mode got switched appropriately.
Comment 1 Mikhail Naganov 2011-03-05 07:20:08 PST
Created attachment 84864 [details]
patch
Comment 2 Alexander Pavlov (apavlov) 2011-03-05 07:27:32 PST
Comment on attachment 84864 [details]
patch

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

> Source/WebCore/inspector/front-end/ProfilesPanel.js:475
> +            if (!Preferences.detailedHeapProfiles

We do not break lines in this way - everything should be on the same line

> Source/WebCore/inspector/front-end/ProfilesPanel.js:727
> +        setTimeout(HideHint, 2000);

function names are camelCase in WebKit
Comment 3 Pavel Feldman 2011-03-05 09:00:58 PST
Comment on attachment 84864 [details]
patch

Looks good except for Alexander's comment. Also, could you factor out this shortcut handler as a separate method parametrized with the easter egg phrase?
Comment 4 Mikhail Naganov 2011-03-05 09:33:24 PST
(In reply to comment #2)
> (From update of attachment 84864 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84864&action=review
> 
> > Source/WebCore/inspector/front-end/ProfilesPanel.js:475
> > +            if (!Preferences.detailedHeapProfiles
> 
> We do not break lines in this way - everything should be on the same line
> 

Fixed.

> > Source/WebCore/inspector/front-end/ProfilesPanel.js:727
> > +        setTimeout(HideHint, 2000);
> 
> function names are camelCase in WebKit

Fixed.
Comment 5 Mikhail Naganov 2011-03-05 09:34:40 PST
(In reply to comment #3)
> (From update of attachment 84864 [details])
> Looks good except for Alexander's comment. Also, could you factor out this shortcut handler as a separate method parametrized with the easter egg phrase?

I've factored it out, but left it as ProfilesPanel method. But it's now easy to extract the method to any other common class.
Comment 6 Mikhail Naganov 2011-03-05 09:41:22 PST
Manually committed as http://trac.webkit.org/changeset/80425

2011-03-05  Mikhail Naganov  <mnaganov@chromium.org>

        Reviewed by Pavel Feldman.

        Web Inspector: [Chromium] Allow dynamic enabling of detailed heap profiles.
        https://bugs.webkit.org/show_bug.cgi?id=55824

        Detailed heap profiles can be now enabled by typing "leakz" in Profiles tab.

        * inspector/front-end/DetailedHeapshotView.js:
        (WebInspector.DetailedHeapshotView.prototype.isDetailedSnapshot):
        * inspector/front-end/ProfilesPanel.js:
        (WebInspector.ProfilesPanel.prototype._finishHeapSnapshot.doParse):
        (WebInspector.ProfilesPanel.prototype._finishHeapSnapshot):
        (WebInspector.ProfilesPanel.prototype._reportHeapSnapshotProgress):
        (WebInspector.ProfilesPanel.prototype.handleShortcut):
        (WebInspector.ProfilesPanel.prototype._displayDetailedHeapProfilesEnabledHint.hideHint):
        (WebInspector.ProfilesPanel.prototype._displayDetailedHeapProfilesEnabledHint):
        (WebInspector.ProfilesPanel.prototype._enableDetailedHeapProfiles):
        (WebInspector.ProfilesPanel.prototype._recognizeKeyboardCombo):