Closed
Bug 1348874
Opened 8 years ago
Closed 8 years ago
Timeout navigator.storage.persist() in sandboxed iframe should reject with TypeError
Categories
(Core :: Storage: Quota Manager, enhancement)
Core
Storage: Quota Manager
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: shawnjohnjr, Assigned: shawnjohnjr)
References
(Blocks 1 open bug)
Details
(Whiteboard: [storage-v1])
Attachments
(1 file, 1 obsolete file)
This test case just recently added.
Timeout navigator.storage.persist() in sandboxed iframe should reject with TypeError Test timed out
Not Run navigator.storage.persisted() in non-sandboxed iframe should not reject
Not Run navigator.storage.persisted() in sandboxed iframe should reject with TypeError
Not Run navigator.storage.estimate() in non-sandboxed iframe should not reject
Not Run navigator.storage.estimate() in sandboxed iframe should reject with TypeError
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → shuang
Assignee | ||
Comment 1•8 years ago
|
||
We have this issue just because storage standard has been updated last month.
https://github.com/whatwg/storage/commit/0dbd146e774316d44182051faf21654ef23ed7cf
Disable the API in opaque origins
Also consistently queue a task to reject and resolve promises from in parallel steps.
Tests: w3c/web-platform-tests#4919.
Assignee | ||
Updated•8 years ago
|
Whiteboard: [storage-v1]
Assignee | ||
Comment 2•8 years ago
|
||
Assignee | ||
Comment 3•8 years ago
|
||
Comment on attachment 8850529 [details] [diff] [review]
Bug 1348874 - If origin is an opaque origin, reject promise with a TypeError
The specification updated 28 days ago.
And the new wpt test case has been imported into our codebase.
This patch fixes opaque-origin.https.html test case.
In addition, prefs are enabled for automatic test purpose, otherwise this test case will be blocked by permission request dialog.
Attachment #8850529 -
Flags: review?(jvarga)
Updated•8 years ago
|
Attachment #8850529 -
Flags: review?(jvarga) → review+
Assignee | ||
Updated•8 years ago
|
Attachment #8850529 -
Attachment is obsolete: true
Assignee | ||
Comment 4•8 years ago
|
||
Rebase again since Bug 1286717 Part 1 got r+.
Comment 5•8 years ago
|
||
Comment on attachment 8856504 [details] [diff] [review]
Bug 1348874 - If origin is an opaque origin, reject promise with a TypeError, r=janv
Review of attachment 8856504 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/quota/StorageManager.cpp
@@ +289,5 @@
>
> + // Storage Standard 7. API
> + // If origin is an opaque origin, then reject promise with a TypeError.
> + if (principal->GetIsNullPrincipal()) {
> + promise->MaybeReject(NS_ERROR_DOM_TYPE_ERR);
Isn't this the same problem as with private browsing ?
I'm not sure, but I think we shouldn't call MaybeResolve/Reject() before we actually return the promise to the caller.
Assignee | ||
Updated•8 years ago
|
Attachment #8856504 -
Attachment is obsolete: true
Assignee | ||
Comment 6•8 years ago
|
||
(In reply to Jan Varga [:janv] from comment #5)
> Comment on attachment 8856504 [details] [diff] [review]
> Bug 1348874 - If origin is an opaque origin, reject promise with a
> TypeError, r=janv
>
> Review of attachment 8856504 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> ::: dom/quota/StorageManager.cpp
> @@ +289,5 @@
> >
> > + // Storage Standard 7. API
> > + // If origin is an opaque origin, then reject promise with a TypeError.
> > + if (principal->GetIsNullPrincipal()) {
> > + promise->MaybeReject(NS_ERROR_DOM_TYPE_ERR);
>
> Isn't this the same problem as with private browsing ?
> I'm not sure, but I think we shouldn't call MaybeResolve/Reject() before we
> actually return the promise to the caller.
I think MaybeReject() call will send to microtask queue. So this won't be executed immediately.
There are many places did the same thing.
http://searchfox.org/mozilla-central/source/dom/file/FileCreatorHelper.cpp#60
http://searchfox.org/mozilla-central/source/dom/html/HTMLInputElement.cpp#6058
http://searchfox.org/mozilla-central/source/dom/media/eme/MediaKeySession.cpp#357
Assignee | ||
Updated•8 years ago
|
Attachment #8856504 -
Attachment is obsolete: false
Assignee | ||
Comment 7•8 years ago
|
||
I will study this promise question a bit more and update to this bug.
Comment 8•8 years ago
|
||
Ok, never mind then.
Assignee | ||
Comment 9•8 years ago
|
||
Some notes for reject promise.
http://searchfox.org/mozilla-central/source/js/src/jsapi.cpp#4986
return PromiseObject::reject(cx, promise, rejectionValue);
http://searchfox.org/mozilla-central/source/js/src/builtin/Promise.cpp#3023
return RejectMaybeWrappedPromise(cx, promise, rejectionValue);
http://searchfox.org/mozilla-central/source/js/src/builtin/Promise.cpp#784
return ResolvePromise(cx, promise, reason, JS::PromiseState::Rejected);
http://searchfox.org/mozilla-central/source/js/src/builtin/Promise.cpp#627
return TriggerPromiseReactions(cx, reactionsVal, state, valueOrReason);
http://searchfox.org/mozilla-central/source/js/src/builtin/Promise.cpp#826
return EnqueuePromiseReactionJob(cx, reactions, valueOrReason, state);
http://searchfox.org/mozilla-central/source/js/src/vm/Runtime.cpp#711
return cx->runtime()->enqueuePromiseJobCallback(cx, job, allocationSite, incumbentGlobal, data);
http://searchfox.org/mozilla-central/source/xpcom/base/CycleCollectedJSContext.cpp#1059
self->DispatchToMicroTask(runnable.forget());
After calling DispatchToMicroTask(), this is async I think.
Updated•8 years ago
|
Attachment #8856504 -
Flags: review+
Comment 10•8 years ago
|
||
Pushed by shuang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/aa2f6f1f7bd6
If origin is an opaque origin, reject promise with a TypeError, r=janv
Assignee | ||
Comment 11•8 years ago
|
||
This patch should be landed together with Bug 1286717.
![]() |
||
Comment 12•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•