1575891 - NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.hostPort]

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P3)

Description

[vtol]

• FX 70 x_64 build dcfcd7909aff0ef81a3b884ead0745645c6d6670
• W10 Pro x_64 1903 b18362.295

---

browser console repeatedly prints

NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.hostPort] ext-browsingData.js:192

pointing to

let host = principal.URI.hostPort;

No idea what triggers it

Comment 1

[Victor Carciu]

Can you please let us know if you have any addons installed and if yes, which ones?

Comment 2

[vtol]

Attachment: wx info

Comment 3

[vtol]

Attachment: recent log

Comment 4

[Victor Carciu]

I installed all extensions described above on FX 70.0b4, Build ID 20190905174512, Win10x64 and I can confirm that the errors described are displayed in console. It can be simply reproduced by opening Spotify login page .

I will attach a screenshot with my browser console.

Comment 5

[Victor Carciu]

Attachment: NS_ERROR.png

Comment 6

[Shane Caraveo (:mixedpuppy)]

This is happening in clearLocalStorage.

Comment 7

[vtol]

The error output shown in comment 5 seems to be different, and might even be unrelated, than what been previously/initially reported

NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIURI.hostPort] ext-browsingData.js:194

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

This probably wants a change like the fix from https://bugzilla.mozilla.org/show_bug.cgi?id=1507171#c13 to only try and inspect the host/hostPort of URI types that actually have hosts. It's likely an "about:newtab" principal or similar is now using storage in the profile.

Of course, in general, the recommendation continues to be to use Sanitizer.jsm and its "offlineApps" category. (But which does constitute a change in API behavior.)

Comment 9

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

Attachment: Bug 1575891 - browsingData API should not clear nextGen localStorage data stored by browser components and extensions. r?mixedpuppy!,asuth,johannh

Comment 10

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

Andrew is right, the issue is definitely triggered when an about page is storing data.

"Ironically" the about pages don't seem to actually support localStorage at all, accessing localStorage from an about page throws an Exception "Component is not available" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE) error.

Nevertheless, we do try to clear localStorage for any principal that has stored any kind of data when Services.lsm.nextGenLocalStorageEnabled is true (and it looks that we enabled it on all channels starting from Firefox 68, Bug 1539835):

https://searchfox.org/mozilla-central/rev/5cb522c7baba24e55874809e0e206b001494c1e9/browser/components/extensions/parent/ext-browsingData.js#175-194

and so the fact that about:newtab (or any other about page) stores some data into IndexedDB is going to trigger this issue.

I do agree with Andrew that we should definitely look into fixing Bug 1531276 and make the browserData API to reuse as much as possible Sanitizer.jsm/nsIClearDataService, to avoid keeping fixing this kind of issues twice (once for Firefox sanitizer and once more for the WebExtensions APIs).

In the meantime I've attached a patch with a small change to fix the regression and an additional test case (given that this has regressed on Firefox 68 and this smaller fix is a low risky change, in case we may want to uplift a fix for this regression on ESR).

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4b4c2fa6e6fa
browsingData API should not clear nextGen localStorage data stored by browser components and extensions. r=mixedpuppy,asuth,johannh

Comment 12

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/4b4c2fa6e6fa

Comment 13

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

Comment on attachment 9099549 [details]
Bug 1575891 - browsingData API should not clear nextGen localStorage data stored by browser components and extensions. r?mixedpuppy!,asuth,johannh

Beta/Release Uplift Approval Request

• User impact if declined: If the nextGenLocalStorage is going to be turned back on in Firefox 71, then for any user that used extensions based on the browsingData WebExtensions API:

• clearing the localStorage data is not going to work anymore, because the API will throw while trying to access hostPort on a URI with the about scheme (if any Firefox integrated about:* page has any data stored in a WebAPI storage, it doesn't need to be stored in localStorage),

• or the API may actually clear also any localStorage data stored by all extensions (because before this fix the API is enumerating all the origins with some data stored and then it tries to clear the nextGenLocalStorage data from the origin without checking if the schema is one of those only related to web pages, e.g. it would also clear moz-extension origins which are actually related to other extensions).

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The change is pretty simple and limited to the part of the browsingData API implementation that clean the data stored into the nextGenLocalStorage. The patch also include two new test cases (which is basically half of the small patch) to explicitly test the fix as part of the automated tests (and prevent it from regressing).
• String changes made/needed:

Comment 14

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9099549 [details]
Bug 1575891 - browsingData API should not clear nextGen localStorage data stored by browser components and extensions. r?mixedpuppy!,asuth,johannh

Uplift approved for 71 beta 10, thanks.

Comment 15

[Narcis Beleuzu [:NarcisB]]

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

Comment 16

[Alexandru Michis [:malexandru]]

Backed out changeset b6846fa525a2 (Bug 1575891) for causing browser-chrome failures in browser_ext_browsingData_localStorage.js

Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/992520c31cde6750358c5d5c3a610f77ebadd404

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&selectedJob=276208517&resultStatus=testfailed%2Cbusted%2Cexception&tochange=992520c31cde6750358c5d5c3a610f77ebadd404&fromchange=b6846fa525a2dd976e16339215c3d63c8fe67245

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=276208517&repo=mozilla-beta&lineNumber=5075

[task 2019-11-14T15:39:57.631Z] 15:39:57 INFO - Console message: Warning: attempting to write 6412 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes.
[task 2019-11-14T15:39:57.632Z] 15:39:57 INFO - Buffered messages finished
[task 2019-11-14T15:39:57.633Z] 15:39:57 INFO - TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_browsingData_localStorage.js | Got the expected localStorage data - Got null, expected value
[task 2019-11-14T15:39:57.633Z] 15:39:57 INFO - Stack trace:
[task 2019-11-14T15:39:57.633Z] 15:39:57 INFO - chrome://mochikit/content/browser-test.js:test_is:1312
[task 2019-11-14T15:39:57.634Z] 15:39:57 INFO - chrome://mochitests/content/browser/browser/components/extensions/test/browser/browser_ext_browsingData_localStorage.js:test_browserData_should_not_remove_extension_data:152
[task 2019-11-14T15:39:57.635Z] 15:39:57 INFO - Leaving test bound test_browserData_should_not_remove_extension_data
[task 2019-11-14T15:39:57.637Z] 15:39:57 INFO - GECKO(1152) | MEMORY STAT | vsize 3066MB | residentFast 445MB | heapAllocated 187MB

Comment 17

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

I discussed about this with Aryx over IRC, and it is fine to revert this patch if we have also disabled LSNG in Firefox 71 Beta.

The failure is triggered by the second test case added in this patch: 'test_browserData_should_not_remove_extension_data',
which pass successfully when LSNG is enabled, but fails when it is disabled.

When LSNG is disabled, the browsingData API does clear the localStorage data by notify a "extension:purge-localStorage" message over the observer service, when the browserData API call doesn't specify a list of specific origins to clear the code that handles "extension:purge-localStorage" will clear all the localStorage data, including the one stored by extensions:

• https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/browser/components/extensions/parent/ext-browsingData.js#172
• https://searchfox.org/mozilla-central/rev/e7c61f4a68b974d5fecd216dc7407b631a24eb8f/dom/storage/StorageObserver.cpp#292-295,323,326

I filed Bug 1596542 to make sure that this particular test case is going to be skipped if LSNG has to be disabled on Firefox 72 (which should be unlikely, but it would still be better to prevent upfront to make it more complicate in case we have to do it during the beta cycle or in a dot release).

Comment 18

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Can you remove the approval-mozilla-beta+ to prevent this bug from appearing in the uplift queries, please?