Bug 112066 – Inspector: IndexedDB clear button and menu item

• Component: New Bugs
• Status: RESOLVED FIXED

[Alec Flett]

Inspector: IndexedDB clear button and menu item

[Alec Flett]

Created attachment 192557 [details]
Patch

[Alec Flett]

Created attachment 192560 [details]
Patch

[Alec Flett]

Comment on attachment 192560 [details]
Patch

I still need to write a test for this, but I wanted to run this by vsevik. A few small open questions:
1) I just picked an icon, is there any kind of design review I need to go through for UI changes like this?
2) There is some view state management that doesn't really exist in the current inspector, so much of the code
   between the context menu and the IDBDataView can't be shared.
3) Would there be any interest in adding a progress icon to the IDBDataView statusbar? While debugging I found 
   it confusing to know when the inspector itself was busy with IDB or not.

[Joshua Bell]

Comment on attachment 192560 [details]
Patch

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

> Source/WebCore/inspector/InspectorIndexedDBAgent.cpp:762
> +        if (ec) {

This should only happen if the arguments or state are invalid, e.g. "readonly" transaction, objectstore has been previously deleted within this "versionchange", etc. None of those should apply here - ASSERT instead?

[Alec Flett]

yeah, there's a lot of that in the inspector, I was reluctantly following convention, hoping to do some cleanup later... but I'll fix that instance.

[Vsevolod Vlasov]

(In reply to comment #3)
> (From update of attachment 192560 [details])
> I still need to write a test for this, but I wanted to run this by vsevik. A few small open questions:
> 1) I just picked an icon, is there any kind of design review I need to go through for UI changes like this?
Not really, you could just attach a screenshot to the bug and inspector reviewers could approve it.
> 2) There is some view state management that doesn't really exist in the current inspector, so much of the code
>    between the context menu and the IDBDataView can't be shared.
Looks like there is no a lot of code there, probably not a big deal.
> 3) Would there be any interest in adding a progress icon to the IDBDataView statusbar? While debugging I found 
>    it confusing to know when the inspector itself was busy with IDB or not.
You could have a look on Source/WebCore/inspector/front-end/ProgressIndicator.js and its usages.

[Alec Flett]

Created attachment 193682 [details]
Patch

[Alec Flett]

vsevik@ - this is ready for review now.. I'll do the progress meter another time.

[Vsevolod Vlasov]

Comment on attachment 193682 [details]
Patch

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

Please rebaseline and fix nits before landing.

> Source/WebCore/ChangeLog:27
> [phone]  Alec Flett  <[e-mail]>

Please rebaseline

> Source/WebCore/inspector/Inspector.json:1416
> +                    { "name":  "securityOrigin", "type": "string", "description": "Security origin." },

extra space after "name":

> Source/WebCore/inspector/front-end/IndexedDBViews.js:130
> +    this._clearButton = new WebInspector.StatusBarButton(WebInspector.UIString("Clear All Data"), "clear-storage-status-bar-item");

"Clear object store" and you will need to add it to Source/WebCore/English.lproj/localizedStrings.js.
Or just make it "Clear".

[Alec Flett]

Created attachment 193846 [details]
Patch for landing

[Alec Flett]

Comment on attachment 193846 [details]
Patch for landing

ugh stupid changelog

[Alec Flett]

Created attachment 193847 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 193847 [details]
Patch for landing

Clearing flags on attachment: 193847

Committed r146218: <http://trac.webkit.org/changeset/146218>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.