Bug 55824 - Web Inspector: [Chromium] Allow dynamic enabling of detailed heap profiles
Summary: Web Inspector: [Chromium] Allow dynamic enabling of detailed heap profiles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Mikhail Naganov
URL:
Keywords:
Depends on:
Blocks: 50510
  Show dependency treegraph
 
Reported: 2011-03-05 07:17 PST by Mikhail Naganov
Modified: 2011-03-05 09:41 PST (History)
10 users (show)

See Also:


Attachments
patch (5.04 KB, patch)
2011-03-05 07:20 PST, Mikhail Naganov
pfeldman: review+
mnaganov: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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):