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)

enhancement
Not set
normal

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: nobody → shuang
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.
Whiteboard: [storage-v1]
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)
Attachment #8850529 - Flags: review?(jvarga) → review+
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.
(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
I will study this promise question a bit more and update to this bug.
Ok, never mind then.
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.
Attachment #8856504 - Flags: review+
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
This patch should be landed together with Bug 1286717.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: