Warn about deprecation of storage: persistent

RESOLVED FIXED in Firefox 60

Status

()

enhancement
P2
normal
RESOLVED FIXED
Last year
Last year

People

(Reporter: johannh, Assigned: johannh)

Tracking

(Blocks 1 bug, {dev-doc-complete, site-compat})

unspecified
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

(Whiteboard: [storage-v2][triage])

Attachments

(1 attachment)

Usage numbers for storage: persistent have returned to what seems like the more likely reflection of reality: https://mzl.la/2GVyQ7g

In preparation of writing a blog post declaring the deprecation, we want to add a console warning for developers.
Priority: -- → P2
Intent to unship will follow tomorrow, I wanted to get the deprecation note out for review since I had the patch already. Thanks!

Comment 3

Last year
mozreview-review
Comment on attachment 8956212 [details]
Bug 1442560 - Add a note for deprecating passing a storage option to indexedDB.open().

https://reviewboard.mozilla.org/r/225122/#review231126

Restating: For window globals, we will warn once per non-SystemPrincipal'ed document if IDBFactory.open()'s second-arg-is-dictionary variant is invoked and "storage" is passed, regardless of the value.  With the change below, we will also warn on workers.  We will not, however, warn for JSM/Sandbox IndexedDB instances.  (If we lost the main thread guard in the else case I propose below, we'd get coverage there too.  I'm fine with that if you think that makes sense.)

::: dom/indexedDB/IDBFactory.cpp:459
(Diff revision 1)
>    if (!IsChrome() &&
>        aOptions.mStorage.WasPassed()) {
>  
> +    if (mWindow && mWindow->GetExtantDoc()) {
> +      mWindow->GetExtantDoc()->WarnOnceAbout(nsIDocument::eIDBOpenDBOptions_StorageType);
> +    }

It seems likely that any code fancy enough to use persistent storage may also be using Workers.  We don't have suppression logic in-place, but I think it can be argued that this is a situation where spamming the console is okay.  The logic would go something like this:

```
} else if (!NS_IsMainThread()) {
  // (The method below reports on the main thread too, so we need to make sure we're on a worker.)
  // Workers don't have a WarnOnceAbout mechanism, so this will be reported every time.
  WorkerPrivate::ReportErrorToConsole("IDBOpenDBOptions_StorageType");
}
```
Attachment #8956212 - Flags: review?(bugmail) → review+
Comment hidden (mozreview-request)
Thank you for the suggestion! I think we should indeed warn on workers but I think internal usage is limited enough that we can deal with it without spamming the browser console. I made your suggested changes and I'll land this now given that the intent to unship didn't receive any negative feedback so far.

It's probably good to have this in ESR 60.

Comment 6

Last year
Pushed by jhofmann@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/78dd2074db57
Add a note for deprecating passing a storage option to indexedDB.open(). r=asuth
https://hg.mozilla.org/mozilla-central/rev/78dd2074db57
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
(In reply to Johann Hofmann [:johannh] from comment #9)
> It would be good to remove this from prominently appearing on MDN everywhere
> (and add a deprecation note):
> 
> https://developer.mozilla.org/en-US/docs/Web/API/IDBFactory/open#Syntax
> 
> https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/
> Browser_storage_limits_and_eviction_criteria#Different_types_of_data_storage

Thanks for the help with the docs!

I've adjusted the wording and layout a bit, and also added a note to the Fx60 rel notes:

https://developer.mozilla.org/en-US/Firefox/Releases/60#DOM

Let me know if you'd like to see any other changes.
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #10)
> (In reply to Johann Hofmann [:johannh] from comment #9)
> > It would be good to remove this from prominently appearing on MDN everywhere
> > (and add a deprecation note):
> > 
> > https://developer.mozilla.org/en-US/docs/Web/API/IDBFactory/open#Syntax
> > 
> > https://developer.mozilla.org/en-US/docs/Web/API/IndexedDB_API/
> > Browser_storage_limits_and_eviction_criteria#Different_types_of_data_storage
> 
> Thanks for the help with the docs!
> 
> I've adjusted the wording and layout a bit, and also added a note to the
> Fx60 rel notes:
> 
> https://developer.mozilla.org/en-US/Firefox/Releases/60#DOM
> 
> Let me know if you'd like to see any other changes.

That seems great, thank you!
You need to log in before you can comment on or make changes to this bug.