Web Inspector: Persist Audit selections across sessions

[Alice Boxhall]

In the Audits UI, remember which audits the user has selected in future sessions.

[Andrey Kosyakov]

Created attachment 179712 [details]
Patch

[Alexander Pavlov (apavlov)]

Comment on attachment 179712 [details]
Patch

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

> Source/WebCore/inspector/front-end/AuditLauncherView.js:69
> +WebInspector.AuditLauncherView.AllCategoriesKey = "__AllCategories";

This can be a computed setting (will slightly break when more categories from new extensions are added, though)

> Source/WebCore/inspector/front-end/AuditLauncherView.js:268
> +    _selectedCategoriesUpdated: function()

Could you clean up the persisted list of categories to remove junk (e.g. leftover categories from disabled/removed extensions) here?

[Andrey Kosyakov]

Created attachment 179723 [details]
Patch

[Andrey Kosyakov]

(In reply to comment #2)
> (From update of attachment 179712 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=179712&action=review
> 
> > Source/WebCore/inspector/front-end/AuditLauncherView.js:69
> > +WebInspector.AuditLauncherView.AllCategoriesKey = "__AllCategories";
> 
> This can be a computed setting (will slightly break when more categories from new extensions are added, though)

Yep -- keeping all categories explicitly in the state we display to the user simplifies things a lot, given that categories come and go dynamically.

> > Source/WebCore/inspector/front-end/AuditLauncherView.js:268
> > +    _selectedCategoriesUpdated: function()
> 
> Could you clean up the persisted list of categories to remove junk (e.g. leftover categories from disabled/removed extensions) here?

Fixed -- now remove categories that are not present each time we update state due to a user action.

[Alexander Pavlov (apavlov)]

Comment on attachment 179723 [details]
Patch

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

Looks good audits-wise. I'd like to hear an authoritative opinion on whether this kind of setting initialization is valid.

> Source/WebCore/inspector/front-end/AuditLauncherView.js:66
> +    this._selectedCategoriesSetting = WebInspector.settings.createSetting("selectedAuditCategories", defaultSelectedAuditCategory);

I'm not sure if this is the right way to initialize a setting (outside Settings.js) - need advice from someone who knows the settings handling well enough.

[Alexander Pavlov (apavlov)]

Comment on attachment 179723 [details]
Patch

r=me per the offline discussion

[Andrey Kosyakov]

Committed r137903: <http://trac.webkit.org/changeset/137903>