[webkit-reviews] review granted: [Bug 39224] Web Inspector: There should be a way to clean up profiles : [Attachment 57411] Remove the profiles from the backend as well.

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has granted Jessie Berlin
<jberlin at webkit.org>'s request for review:
Bug 39224: Web Inspector: There should be a way to clean up profiles
https://bugs.webkit.org/show_bug.cgi?id=39224

Attachment 57411: Remove the profiles from the backend as well.
https://bugs.webkit.org/attachment.cgi?id=57411&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
Looks great! (see some nits below).

Please make sure to restore buttons order in the timeline panel prior to
landing this. And one more thing: could you please add an
installInspectorControllerDelegate_ calls for new InspectorBackend functions in
the WebKit/chromium/src/js/InspectorControllerImpl.js? Otherwise chromium tests
might fail downstream. This is a temporary measure while we are working on
remote debugging and are aligning chromium with new WebKit code.

WebCore/inspector/front-end/treeoutline.js:392
 +	} else if (event.keyIdentifier === "U+0008" || event.keyIdentifier ===
"U+007F") {
event.keyCode === WebInspector.KeyboardShortcut.Keys.Backspace.code ||
event.keyCode === WebInspector.KeyboardShortcut.Keys.Delete.code


WebCore/inspector/front-end/treeoutline.js:395
 +		this.selectedTreeElement.ondelete();
Nit: Now that you introduced this, you might want to migrate elements panel's
delete handler to this new schema (see
WebInspector.ElementsTreeOutline.prototype._keyDown). You don't have to do
this, but if you'd like that'd be cool.

WebCore/inspector/front-end/ProfilesPanel.js:567
 +	    WebInspector.panels.profiles.removeProfileHeader(this.profile);
Nit: It would be better to pass the reference to panel into the sidebar instead
of using the global accessor. I do realize that the method above was using it,
but it was just wrong. Same as above, you don't need to fix, but would be a
good drive-by improvement.

WebCore/inspector/front-end/TimelinePanel.js:151
 +	    return [this.toggleFilterButton.element, this.clearButton.element,
this.toggleTimelineButton.element, this._overviewPane.statusBarFilters];
Why this change? I think clear should be the rightmost button.

WebCore/ChangeLog:8
 +	    Adds a button to clear the profiles from the profiles panel like
that used for the console, the audits panel, and the timeline panel.
Consolidates the css rules, since they all use the same image. Also allows for
individual profiles to be deleted via the keyboard (U+0008 or U+007F).
Please format this message so that it was ~80 chars per line. Also, you can
remove trivial changes with the dittos in the log below.