Enable Storage API only for nightly bulid

RESOLVED FIXED in Firefox 51

Status

()

defect
RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: shawnjohnjr, Assigned: shawnjohnjr)

Tracking

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox51+ fixed, firefox52 fixed)

Details

(Whiteboard: [storage-v1])

Attachments

(2 attachments, 3 obsolete attachments)

Disable Storage estimate() function due to lack of support SecureContext on workers. Because ​Storage API needs to have SecureContext support, but currently
not having isSecureContext available in Workers (bug 1269052) is
problematic. And we cannot ship estimate without SecureContext support.
Per discussion with Olli, he thought we should wait for until we have SecureContext also in workers, so I will try to perf-off estimate.
Assignee: nobody → shuang
Can we possibly use some temporary hack to enable estimate() in workers? I mean, if SecureContext isn't enabled for .webidl in workers or so, could we detect SecureContext in the worker somehow?

(I'm not familiar with our SecureContext impl, and I'm surprised that it doesn't work in workers. bz might recall the reasoning.)
I don't think there was any reasoning beyond "jwatt hasn't gotten to implementing this yet".

In practice, a worker is a secure context per spec if and only if whatever document created it is a secure context....
So actually, for workers the setup is as follows, per spec:

1. Service workers are always secure contexts.
2. Dedicated workers are secure contexts if and only if the document that created them or the worker that created them is a secure context.
3. Shared workers are secure contexts if and only if the _first_ document that created them is a secure context.  When a document tries to attach to an existing shared worker, this should throw if the secure contextness of the document doesn't match the secure contextness of the worker.
The spec also claims service workers are only available in secure contexts; I can't recall what our state on that is.
We'll have SecureContext in Workers soon, right? (bug 1269052) I imagine Ben will get to reviews when he's back from PTO next week.
Yes, Ben's review is the only thing that bug depends on as far as I know.
Summary: Disable Storage estimate() function due to lack of support SecureContext on workers → Disable Storage API due to lack of support SecureContext on workers
Comment on attachment 8804204 [details] [diff] [review]
Bug 1304966 - Enable Storage API only for nightly bulid

Review of attachment 8804204 [details] [diff] [review]:
-----------------------------------------------------------------

Hi baku,
This patch add a preference value dom.storageManager.enabled and only enable Storage API for nightly. This patch will also apply to Aurora channel, because we're not going to ship Storage API without SecureContext.
Attachment #8804204 - Flags: review?(amarchesini)
Summary: Disable Storage API due to lack of support SecureContext on workers → Enable Storage API only for nightly bulid
I'm a bit not sure why one test case busted.

 11:28:09  WARNING - TEST-UNEXPECTED-ERROR | test_preferences.PreferencesTest.test_read_prefs_ttw | HTTPError: HTTP Error 404: File not found
 11:28:09     INFO - Traceback (most recent call last):
 11:28:09     INFO -   File "z:\task_1477389680\build\src\testing\mozbase\mozprofile\tests\test_preferences.py", line 372, in test_read_prefs_ttw
 11:28:09     INFO -     read = prefs.read_prefs('http://%s:%d/prefs_with_comments.js' % (host, port))
(In reply to Shawn Huang [:shawnjohnjr] from comment #13)
> I'm a bit not sure why one test case busted.
> 
>  11:28:09  WARNING - TEST-UNEXPECTED-ERROR |
> test_preferences.PreferencesTest.test_read_prefs_ttw | HTTPError: HTTP Error
> 404: File not found
>  11:28:09     INFO - Traceback (most recent call last):
>  11:28:09     INFO -   File
> "z:
> \task_1477389680\build\src\testing\mozbase\mozprofile\tests\test_preferences.
> py", line 372, in test_read_prefs_ttw
>  11:28:09     INFO -     read =
> prefs.read_prefs('http://%s:%d/prefs_with_comments.js' % (host, port))

After i triggered the job again, it can pass. So I guess that file just missed on that server.
Comment on attachment 8804204 [details] [diff] [review]
Bug 1304966 - Enable Storage API only for nightly bulid

Review of attachment 8804204 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/quota/StorageManager.cpp
@@ +336,5 @@
> +bool
> +StorageManager::PrefEnabled(JSContext* aCx, JSObject* aObj)
> +{
> +  if (NS_IsMainThread()) {
> +    return Preferences::GetBool("dom.storageManager.enabled");

no }else{ after a return.
Attachment #8804204 - Flags: review?(amarchesini) → review+

Comment 16

3 years ago
Pushed by shuang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6817b7629eea
Enable Storage API only for nightly bulid, r=baku
[Tracking Requested - why for this release]:We don't want to expose Storage API without SecureContext support. Storage API is only available for nightly build. So we should merge this patch to 51.

Comment 19

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6817b7629eea
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Track 51+ as enabling the API for nightly.

Hi :shawnjohnjr,
Can you help create the uplift request for 51 aurora?
Flags: needinfo?(shuang)
Approval Request Comment
[Feature/regressing bug #]:1267941
[User impact if declined]:Exposing incomplete web api.
[Describe test coverage new/current, TreeHerder]:Enable only for nightly, test cases remains the same.
[Risks and why]: No risk, simply pref-off new api.
[String/UUID change made/needed]:None.
Flags: needinfo?(shuang)
Attachment #8805057 - Flags: approval-mozilla-aurora?
Comment on attachment 8805057 [details] [diff] [review]
Bug 1304966 - Enable Storage API only for nightly bulid, r=baku (aurora)

Enable storage API in nightly only. Take it in 51 aurora.
Attachment #8805057 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

Comment 24

3 years ago
Pushed by kwierso@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f43fa0be439
Mark Storage APIs as nightly-only r=bz a=me CLOSED TREE
Followup landed to https://hg.mozilla.org/releases/mozilla-aurora/rev/531d3f05ebf0dc52d6ff076b8c413010c7ed1240 to let the tests know these APIs are nightly-only.

Also landed it to trunk so we don't have problems on merge day: https://hg.mozilla.org/integration/mozilla-inbound/rev/4f43fa0be439
Whiteboard: storage-v1 → [storage-v1]
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.