1684912 - The caller of CreateStorageConnection needs to know if the usage file was removed even if the method failed

Status: RESOLVED FIXED

(Core :: Storage: localStorage & sessionStorage, defect, P2)

Description

[Jan Varga [:janv]]

This is a regression that was introduced by changes done in bug 1671367.

Comment 1

[Jan Varga [:janv]]

The call at https://searchfox.org/mozilla-central/rev/f8a41209af503016e78278774052d48d8c52b91c/dom/localstorage/ActorsParent.cpp#7395 no longer sets rv and LS_TRY_INSPECT immediately returns if there was an error, so the check below doesn't have a chance to update the quota object.

Comment 2

[Simon Giesecke [:sg] [he/him]]

We could either change the return type of CreateStorageConnection to std::pair<Result<nsCOMPtr<mozIStorageConnection>, nsresult>, bool> (with the bool indicating whether the usage file was removed), or add a functor argument that is called by CreateStorageConnection in case it removes the usage file and change the return type to simply Result<nsCOMPtr<mozIStorageConnection>, nsresult>. Without having checked feasibility, I have a slight preference for the latter option.

Comment 3

[Jan Varga [:janv]]

Yes, I would prefer the latter as well.

Comment 4

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1684912 - Call a handler in case a corrupted database file was detected. r=#dom-workers-and-storage

The handler is called before opening the connection again. This does not restore
the behaviour before Bug 1684912 exactly. In case removal of the usage or database
file fails, the handler will not be called. Also, if the handler fails, the database
will not be attempted to be recreated.

Comment 5

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

Set release status flags based on info from the regressing bug 1671367

Comment 6

[Julien Cristau [:jcristau] (back August 18)]

Is this something we can live with for 85, is the fix going to be ready for uplift this week, or are we better off reverting the regressing change?

Comment 7

[Simon Giesecke [:sg] [he/him]]

(In reply to Julien Cristau [:jcristau] from comment #6)

Is this something we can live with for 85, is the fix going to be ready for uplift this week, or are we better off reverting the regressing change?

The affected behaviour is enabled only in Nightly, so we can live with the unpatched version riding to 85.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1cc2386869a3
Call a handler in case a corrupted database file was detected. r=dom-workers-and-storage-reviewers,janv

Comment 9

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/1cc2386869a3