1578447 - Storage panel of the WebExtension Addon Debugger toolbox does not auto-update consistently for storage local updates

Status: VERIFIED FIXED

(DevTools :: Storage Inspector, defect, P1)

Description

[Bianca Danforth [:bdanforth]]

Attachment: test-extension-1-1.0.zip

Steps to Reproduce
Note: "Test Extension" refers to the extension attached to this bug.

In Nightly 71 or Beta 70:

1. In about:config, set devtools.storage.extensionStorage.enabled to true.
2. Load Test Extension in about:debugging.
3. Open any http(s) page in a new tab (e.g. https://www.mozilla.org/). If such a page was already open, reloading it is sufficient.
4. Open the addon toolbox for Test Extension in about:debugging.
5. Open the Storage panel and select the extension under "Extension Storage" in the left sidebar.
6. With the Storage panel left open and visible (it may be easiest to move the toolbox to a separate window), click the browserAction toolbar icon in the (first) window.
7. For each option in the browserAction popup:

• Click an option, e.g. "Background script adds a new storage item".
• Select the Console panel and note the storage item(s) that is/are modified.
• Verify the storage data displayed in the panel reflects the modification(s).

Actual Results
The Storage panel only auto-updates when storage items are added to extension storage local. Other modifications, like edit, remove and remove all do not auto-update and the 'Refresh Items' button in the top right corner of the panel must be clicked before those changes are reflected in the Storage panel.

Expected Results
Any kind of extension storage local modification should auto-update.

Comment 1

[Bianca Danforth [:bdanforth]]

Attachment: Bug 1578447 - Fix storage panel ui not refreshing on changes to items with string keys parsable as JSON.

In /devtools/client/storage/ui.js, 'fetchStorageObjects' calls 'populateTable', but only if the 'data' array returned by 'getStoreObjects' is non-empty. Previously, if 'getStoreObjects' was called passing in an empty array for the 'names' argument, it would incorrectly return an empty 'data' array instead of returning all of the store object items.

Comment 2

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hi Miruna,
while running the test plan for Bug 1542035 QA verification, have you ever been able to reproduce this issue?

I've been trying the STR from comment 0 locally, on a recent nightly (without the fix from the attached patch) and on the last beta build, and I couldn't reproduce it (the storage panel was automatically refreshed on every one of the action executed on the storage).

Comment 3

[Miruna Curtean]

Attachment: Extension storage does not auto-update using text extension1.1.0.gif

Yes, I've been able to reproduce this issue. Right now I've tested it again against FF 70.0b7 (20190916074538) on Windows 64-bit but one thing to note is that this issue occurs when using only "test-extension-1-1.0.zip", the one attached, when trying to edit or remove through background or content script the local storage items.
It could be that you are not able to reproduce this if you are testing using the test-extension-1-1.1.zip extension provided in bug 1542035, which are "part of the solution" for this issue. https://bugzilla.mozilla.org/show_bug.cgi?id=1542035#c12
I hope this clears things up. If there's anything else, please don't hesitate to ask me.

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Miruna Curtean from comment #3)

Yes, I've been able to reproduce this issue. Right now I've tested it again against FF 70.0b7 (20190916074538) on Windows 64-bit but one thing to note is that this issue occurs when using only "test-extension-1-1.0.zip", the one attached, when trying to edit or remove through background or content script the local storage items.
It could be that you are not able to reproduce this if you are testing using the test-extension-1-1.1.zip extension provided in bug 1542035, which are "part of the solution" for this issue. https://bugzilla.mozilla.org/show_bug.cgi?id=1542035#c12
I hope this clears things up. If there's anything else, please don't hesitate to ask me.

Ah! Yes, that was it! You have been a real life saver to me ;-), I was starting to think that I was going crazy (because I was 100% sure that I have been able to reproduce it before)

I've been able to reproduce the issue again and dig into the underlying issue a bit further (I've also updated the patch accordingly and added some additional test case to cover the regression with an automated test \o/).

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:jimm, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 7

[Luca Greco [:rpl] [:luca] [:lgreco]]

• Removed the regression keyword (as this is not a regression from Bug 1542035, other existing actors were already affected by the underlying issue but we never noticed until we caught the bug during Bug 1542035 QA verification)

• moved to the "DevTools::Storage Inspector" bugzilla component (because the underlying issue isn't specific to inspecting the extension storage data, as described in https://phabricator.services.mozilla.com/D45197#1412281 it does effect other existing storage actors and not just the RDP ExtensionStorage storage actor introduced by Bug 1542035)

• priority set to P1 (because there is already a patch attached to fix the issue, and it has already been approved for landing on central).

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/36bfbbce7ee2
Fix storage panel ui not refreshing on changes to items with string keys parsable as JSON. r=miker

Comment 9

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/36bfbbce7ee2

Comment 10

[Miruna Curtean]

Verified fixed on Windows 64-bit and macOS High Sierra 10.13.6 on Firefox 71.0a1 (Build ID: 20190926213542).
Using the test extension attached to this bug, I've added, edited and removed local storage items and all the changes were auto-updated in both the Storage tab and the Console tab.

Comment 11

[Liz Henry (:lizzard) (relman/hg->git project)]

Luca is this something that might be low risk enough to uplift to beta? What do you think?

Comment 12

[Bianca Danforth [:bdanforth]]

Comment on attachment 9091452 [details]
Bug 1578447 - Fix storage panel ui not refreshing on changes to items with string keys parsable as JSON.

Beta/Release Uplift Approval Request

• User impact if declined: Without this fix, the DevTools Storage Panel doesn't refresh automatically for changes (e.g. editing or removing) to items stored in localStorage by webpages and/or extensions (as well as in the browser.storage.local by extensions) if the key of the stored item is a string parsable as JSON (e.g. "1", "2" etc. or "null").
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce: STR can be found at https://bugzilla.mozilla.org/show_bug.cgi?id=1578447#c0 .
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This change is not risky, because it is a small patch that extends the behavior of the Storage panel client to include a third, previously unhandled case in handleDeletedItems and handleChangedItems. Also, tests were added to test handling of this new case, so test coverage for the Storage panel has actually increased. Finally, QA has already verified the fix in Nightly 71.
• String changes made/needed: None

Comment 13

[Luca Greco [:rpl] [:luca] [:lgreco]]

Clearing needinfo assigned to me (Bianca created the uplift request as we agreed yesterday, thanks Bianca!)

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9091452 [details]
Bug 1578447 - Fix storage panel ui not refreshing on changes to items with string keys parsable as JSON.

Fix verified in nightly, adds tests, let's uplift for beta 12.

Comment 15

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/a1ddf2dcc0e4

Comment 16

[Miruna Curtean]

Unless I'm interpreting this badly, it looks as the Windows 2012 opt build got busted and it would have been useful to have it in order to test the fix in beta as well. Can something be done about that?
Thank you

Comment 17

[Oana Pop-Rus]

(In reply to Miruna Curtean from comment #16)

Unless I'm interpreting this badly, it looks as the Windows 2012 opt build got busted and it would have been useful to have it in order to test the fix in beta as well. Can something be done about that?
Thank you

The bustages appeared due to a code conflict, however they are green on the push with the conflict fix: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&resultStatus=success%2Csuperseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=windows%2C2012%2Copt%2Cbuild&group_state=expanded&fromchange=14706dc0f3aa5ebf6754ea7c97b6e9ab395f787d&tochange=b07a932f6af3d95c5429c3051d625ce9b9ff1556

Comment 18

[Miruna Curtean]

(In reply to Oana Pop-Rus from comment #17)

The bustages appeared due to a code conflict, however they are green on the push with the conflict fix: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&resultStatus=success%2Csuperseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&searchStr=windows%2C2012%2Copt%2Cbuild&group_state=expanded&fromchange=14706dc0f3aa5ebf6754ea7c97b6e9ab395f787d&tochange=b07a932f6af3d95c5429c3051d625ce9b9ff1556

Thank you for providing this link. I've gotten the target/zip that I wanted but unfortunately I am having problems today with loading the profile after unzipping and cannot figure out why. However, the latest beta loads just fine and I can create new testing profiles on it so I will wait for beta 12 which is bound to be released sometime tomorrow and proceed with testing there.

Comment 19

[Miruna Curtean]

Verified fixed on Windows 64-bit and macOS High Sierra 10.13.6 on Firefox Beta 70.0b13 (Build ID: 20191007220302).
All modifications done to the local data through either test extensions ( test-extension-1-1.0.zip and test-extension-1-1.1.zip) have been visible immediately, without having to refresh the dev toolbox.