Bug 39224 – Web Inspector: There should be a way to clean up profiles

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

[Mikhail Naganov]

Currently there is no way to clean up profiles except for re-opening the inspected page. I think it would be more convenient for long profiling session to be able to clean up existing profiles.

[Jessie Berlin]

Created attachment 57402 [details]
Adds the ability to clear the profiles in the Web Inspector.

Makes it possible to clear all at once as well as delete each profile individually.

[Pavel Feldman]

Comment on attachment 57402 [details]
Adds the ability to clear the profiles in the Web Inspector.

Reopening the inspector will re-create all the profiles unless they are removed from the backend. ProfilesPanel.prototype._populateProfiles will fetch them from the backend using InspectorController::getProfileHeaders.

[Jessie Berlin]

Created attachment 57411 [details]
Remove the profiles from the backend as well.

Sorry for missing that ...

[Pavel Feldman]

Comment on attachment 57411 [details]
Remove the profiles from the backend as well.

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.

[Jessie Berlin]

Created attachment 57416 [details]
Finalized (hopefully) patch to add the ability to clean up the profiles.

(In reply to comment #4)
> (From update of attachment 57411 [details])
> Looks great! (see some nits below).

Thanks for the review :) Just posting the finalized patch here before I commit it in a little while to give the try-bots another chance at it (this time remembered to use --binary on account of localizedStrings.js).

> 
> Please make sure to restore buttons order in the timeline panel prior to landing this.

Done.

> 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.

Done.

> 
> 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
> 

Done.

> 
> 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.

Done. I created a similar "onenter" to handle the enter key for the ElementsTreeOutline.

> 
> 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.

Done. Since there is no separate SidebarTreeOutline class, I added a reference to the enclosing Panel in the createSidebar() method of the Panel class where the TreeOutline is created.

> 
> 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.

I thought it might make more sense for the toggle button to be the button that the user has the most space to click without by accident clicking on another button instead of the clear element, since there isn't a way to undo a clear. However, I have changed it back, and changed the order in the Profiles panel to be the same (toggle button to the left of the clear 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.

Done.

[Jessie Berlin]

Committed in r60414:
http://trac.webkit.org/changeset/60414