Closed
Bug 1304966
Opened 8 years ago
Closed 8 years ago
Enable Storage API only for nightly bulid
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla52
People
(Reporter: shawnjohnjr, Assigned: shawnjohnjr)
References
Details
(Whiteboard: [storage-v1])
Attachments
(2 files, 3 obsolete files)
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.
Assignee | ||
Comment 1•8 years ago
|
||
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.
Updated•8 years ago
|
Assignee: nobody → shuang
Comment 2•8 years ago
|
||
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.)
![]() |
||
Comment 3•8 years ago
|
||
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....
![]() |
||
Comment 4•8 years ago
|
||
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.
![]() |
||
Comment 5•8 years ago
|
||
The spec also claims service workers are only available in secure contexts; I can't recall what our state on that is.
Comment 6•8 years ago
|
||
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.
![]() |
||
Comment 7•8 years ago
|
||
Yes, Ben's review is the only thing that bug depends on as far as I know.
Assignee | ||
Updated•8 years ago
|
Summary: Disable Storage estimate() function due to lack of support SecureContext on workers → Disable Storage API due to lack of support SecureContext on workers
Assignee | ||
Comment 8•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8804198 -
Attachment is obsolete: true
Assignee | ||
Comment 9•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
status-firefox51:
--- → affected
Assignee | ||
Comment 10•8 years ago
|
||
Assignee | ||
Updated•8 years ago
|
Attachment #8804199 -
Attachment is obsolete: true
Assignee | ||
Comment 11•8 years ago
|
||
Assignee | ||
Comment 12•8 years ago
|
||
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)
Assignee | ||
Updated•8 years ago
|
Summary: Disable Storage API due to lack of support SecureContext on workers → Enable Storage API only for nightly bulid
Assignee | ||
Comment 13•8 years ago
|
||
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))
Assignee | ||
Comment 14•8 years ago
|
||
(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.
Assignee | ||
Updated•8 years ago
|
Whiteboard: storage-v1
Comment 15•8 years ago
|
||
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•8 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
Assignee | ||
Updated•8 years ago
|
Attachment #8804204 -
Attachment is obsolete: true
Assignee | ||
Comment 17•8 years ago
|
||
Assignee | ||
Comment 18•8 years ago
|
||
[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.
tracking-firefox51:
--- → ?
Comment 19•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Comment 20•8 years ago
|
||
Track 51+ as enabling the API for nightly.
Hi :shawnjohnjr,
Can you help create the uplift request for 51 aurora?
Flags: needinfo?(shuang)
Assignee | ||
Comment 21•8 years ago
|
||
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 22•8 years ago
|
||
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 23•8 years ago
|
||
bugherder uplift |
Comment 24•8 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
Comment 26•8 years ago
|
||
bugherder |
Updated•8 years ago
|
Whiteboard: storage-v1 → [storage-v1]
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•