Hi chromium-dev,--I have a web API that returns a promise, and I'm trying to make it match the current spec: reject the promise if document stops being fully active while the promise is outstanding [1].blink::PaymentRequest is the object that implements the API.blink::PaymentRequest::show() is the method that returns a promise. It creates a ScriptPromiseResolver and saves it in a Member<ScriptPromiseResolver>.To implement the spec behavior, I want to call accept_resolver_->Reject(...) in blink::PaymentRequest::ContextDestroyed().However, the problem is ScriptPromiseResolver::Reject() does nothing if the context is not valid. This leaves the promise in a pending state.It seems that what I need may be a new ContextWillBeDestroyed() method on ContextLifecycleObserver?Is there a better alternative?Or is this spec behavior unusual? Should I try to change the spec?Thanks,Danyao[1] See bottom of show() algorithm: https://w3c.github.io/payment-request/#show-method
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAErabb_WeE5pHEQqYhQwzLM0uYSD%3DUaR0Xz1B%2BddhoSQFO8Bvw%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CACuR13db6pHsZ2Summ_CaBExL%3D%3DnjjpbmXNREDUzgWDCtz0q5A%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAF3XrKo_L7YKcGPOg%2BM6WGGjZsz%3DdAVwKfg1mdNBuEDP-%2Bo2_Q%40mail.gmail.com.
I would honestly prefer to just not reject the promise in this case at all (since it feels like another unload event and could complicate things like bfcache), but it might be helpful to understand the use cases where this reject behavior is useful.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAPVAxLWXQ%3Dhj6_LAbUHO0YQS7PozPOQ51dDV4MunP9Tw1-mw_A%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jxoYx%3D6NGEtJ_oSfEX7musSpKL%3DTht11%3DSUjeq2H%3DobLQ%40mail.gmail.com.
Maybe a naive question: why don't we reject a promise immediately after its associated context is invalid?
What is a use case of a promise that is passed to a different context and can still resolve?
Maybe a naive question: why don't we reject a promise immediately after its associated context is invalid?Promises need to be resolved / rejected in microtasks, which are invoked at the end of the event loop. It's too late to reject the promises at that point because the associated context is already shut down. In other words, there's no way to reject promises after its associated context becomes invalid but before the associated context is shut down.What is a use case of a promise that is passed to a different context and can still resolve?I personally hope that there's no use case. I think that a promise should stop working (i.e., neither resolved or rejected) after its associated context becomes invalid. That is the current implementation of ScriptPromiseResolver::ResolveOrReject.But Domenic is an expert in this area :)
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAM0wra-YUSr-zsqx9X%3DcmRVMuvw03QLdobg-Q1kdpRtPCY6vEg%40mail.gmail.com.
What are we referring to as "inactive" here?
You can't move from a destroyed context to a new context without a reload of the document. So Domenic your reference from inactive to active seems odd to me.
On Thu, Jun 20, 2019 at 11:43 AM Dave Tapuska <dtap...@chromium.org> wrote:What are we referring to as "inactive" here?Documents that are not https://html.spec.whatwg.org/multipage/browsers.html#active-document (or maybe documents that are not https://html.spec.whatwg.org/multipage/browsers.html#fully-active)You can't move from a destroyed context to a new context without a reload of the document. So Domenic your reference from inactive to active seems odd to me.In the spec, this is not true, I believe mostly (entirely?) because of bfcache.
On Thu, Jun 20, 2019 at 11:43 AM Dave Tapuska <dtap...@chromium.org> wrote:You can't move from a destroyed context to a new context without a reload of the document. So Domenic your reference from inactive to active seems odd to me.In the spec, this is not true, I believe mostly (entirely?) because of bfcache.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CAM0wra8V8wpadk%2BZvNyzt%2Bw%2BpHxpZctqbRXDvTM33%3DTfKsZsfg%40mail.gmail.com.
On Wed, Jun 19, 2019, 10:51 PM Domenic Denicola <dom...@chromium.org> wrote:On Thu, Jun 20, 2019 at 11:43 AM Dave Tapuska <dtap...@chromium.org> wrote:You can't move from a destroyed context to a new context without a reload of the document. So Domenic your reference from inactive to active seems odd to me.In the spec, this is not true, I believe mostly (entirely?) because of bfcache.Maybe also if you remove an iframe from the DOM (making it inactive) and then add it back to the DOM (making it active) again?
I haven't thought about a solution to this mess very hard so far, but maybe we should, on a spec level, make resolve/reject a no-op if the promise's relevant Realm's global object is a Window, and that Window's associated Document is not fully-active. And then, I guess, remove all uses of rejected promises to signal failures due to document inactivity, like the one opening this thread. This feels quite ad-hoc, but I don't really have any better ideas. Thoughts welcome.
maybe we should, on a spec level, make resolve/reject a no-op if the promise's relevant Realm's global object is a Window, and that Window's associated Document is not fully-active.
+1 to this proposal :)
Is the payment request spec requiring a transition when the page becomes "frozen" or is it when it is actually "detached" ? It isn't clear to me.
If document stops being fully active while the user interface is being shown, or no longer is by the time this step is reached, then:
tests: 1AbortError
" DOMException
.Just thinking out loud....So you are trying to chain a promise from one execution context to another. It seems that we'd probably have some type of wrapper in blink that when you bind two a promise together with another promise from another execution context it would reject on the other execution context being destroyed.
Does this really happen in practice? ie; the payment request origin is same origin with the top level frame?
Or that payment requests don't happen in sandboxed iframes?
Chewing on Domenic's suggestion a bit more, I don't think "make resolve/reject a no-op" when the promise's associated document is not fully-active will solve the payment payment use case. Here's a concrete scenario:
- Top-level frame embeds an iframe that creates a PaymentRequest object, e.g. |request|
- request.show() is called inside the iframe and the returned Promise (e.g. |acceptPromise|) is passed back to the top-level frame, e.g. to update UI after the promise resolves.
- iframe navigates so the document associated with |acceptPromise| is no longer a fully active document
- Top-level frame code hangs because |acceptPromise| stays unsettled
In this case, we actually need |acceptPromise| to be rejected for the top-level frame to not hang.I noticed that WebKit has an ActiveDOMObject concept that offers a stop() hook that is called in Document::prepareForDestruction() just before the event queue is stopped. This allows the PaymentRequest, which is an ActiveDOMObject, to clean up in time.Can we do something similar? Just as a strawman:
- Introduce a blink::ActiveDOMObject class
- Store a list of blink::ActiveDOMObjects in blink::ExecutionContext
- Call stop() on each execution context in Document::Shutdown() in the DocumentLifecycle::kStopping phase
- PaymentRequest will inherit from blink::ActiveDOMObject and settle promises in stop()
On Tue, Jul 16, 2019 at 11:58 AM Danyao Wang <dan...@chromium.org> wrote:Chewing on Domenic's suggestion a bit more, I don't think "make resolve/reject a no-op" when the promise's associated document is not fully-active will solve the payment payment use case. Here's a concrete scenario:
- Top-level frame embeds an iframe that creates a PaymentRequest object, e.g. |request|
- request.show() is called inside the iframe and the returned Promise (e.g. |acceptPromise|) is passed back to the top-level frame, e.g. to update UI after the promise resolves.
- iframe navigates so the document associated with |acceptPromise| is no longer a fully active document
- Top-level frame code hangs because |acceptPromise| stays unsettled
In this case, we actually need |acceptPromise| to be rejected for the top-level frame to not hang.I noticed that WebKit has an ActiveDOMObject concept that offers a stop() hook that is called in Document::prepareForDestruction() just before the event queue is stopped. This allows the PaymentRequest, which is an ActiveDOMObject, to clean up in time.Can we do something similar? Just as a strawman:
- Introduce a blink::ActiveDOMObject class
- Store a list of blink::ActiveDOMObjects in blink::ExecutionContext
- Call stop() on each execution context in Document::Shutdown() in the DocumentLifecycle::kStopping phase
- PaymentRequest will inherit from blink::ActiveDOMObject and settle promises in stop()
This is what ContextLifecycleObserver does. Implementing ContextLifecycleObserver and ActiveScriptWrappable together is a common pattern. There's a comment in context_lifecycle_observer.h that describes this in more detail.(That we have a similar concept is no coincidence: we inherited ActiveDOMObject from WebKit when we forked; we just like renaming/refactoring things.)
On Tue, Jul 16, 2019 at 2:49 PM Jeremy Roman <jbr...@chromium.org> wrote:On Tue, Jul 16, 2019 at 11:58 AM Danyao Wang <dan...@chromium.org> wrote:Chewing on Domenic's suggestion a bit more, I don't think "make resolve/reject a no-op" when the promise's associated document is not fully-active will solve the payment payment use case. Here's a concrete scenario:
- Top-level frame embeds an iframe that creates a PaymentRequest object, e.g. |request|
- request.show() is called inside the iframe and the returned Promise (e.g. |acceptPromise|) is passed back to the top-level frame, e.g. to update UI after the promise resolves.
- iframe navigates so the document associated with |acceptPromise| is no longer a fully active document
- Top-level frame code hangs because |acceptPromise| stays unsettled
In this case, we actually need |acceptPromise| to be rejected for the top-level frame to not hang.I noticed that WebKit has an ActiveDOMObject concept that offers a stop() hook that is called in Document::prepareForDestruction() just before the event queue is stopped. This allows the PaymentRequest, which is an ActiveDOMObject, to clean up in time.Can we do something similar? Just as a strawman:
- Introduce a blink::ActiveDOMObject class
- Store a list of blink::ActiveDOMObjects in blink::ExecutionContext
- Call stop() on each execution context in Document::Shutdown() in the DocumentLifecycle::kStopping phase
- PaymentRequest will inherit from blink::ActiveDOMObject and settle promises in stop()
This is what ContextLifecycleObserver does. Implementing ContextLifecycleObserver and ActiveScriptWrappable together is a common pattern. There's a comment in context_lifecycle_observer.h that describes this in more detail.(That we have a similar concept is no coincidence: we inherited ActiveDOMObject from WebKit when we forked; we just like renaming/refactoring things.)Got it. There is still one thing missing: ContextLifecycleObserver doesn't provide a "context-will-be-destroyed" callback a la WebKit's ActiveDOMObject::stop() method. How about this?
- Add ExecutionContext::NotifyContextWillBeDestroyed() to call a new pure virtual ContextLifecycleObserver::ContextWillBeDestroyed() method
- PaymentRequest implements ContextLifecycleObserver::ContextWillBeDestroyed() to settle the promises
- Call ExecutionContext::NotifyContextWillBeDestroyed() in Document::Shutdown() in the DocumentLifecycle::kStopping phase to allow observers to clean up.
On Tue, Jul 16, 2019 at 3:41 PM Danyao Wang <dan...@chromium.org> wrote:On Tue, Jul 16, 2019 at 2:49 PM Jeremy Roman <jbr...@chromium.org> wrote:On Tue, Jul 16, 2019 at 11:58 AM Danyao Wang <dan...@chromium.org> wrote:Chewing on Domenic's suggestion a bit more, I don't think "make resolve/reject a no-op" when the promise's associated document is not fully-active will solve the payment payment use case. Here's a concrete scenario:
- Top-level frame embeds an iframe that creates a PaymentRequest object, e.g. |request|
- request.show() is called inside the iframe and the returned Promise (e.g. |acceptPromise|) is passed back to the top-level frame, e.g. to update UI after the promise resolves.
- iframe navigates so the document associated with |acceptPromise| is no longer a fully active document
- Top-level frame code hangs because |acceptPromise| stays unsettled
In this case, we actually need |acceptPromise| to be rejected for the top-level frame to not hang.I noticed that WebKit has an ActiveDOMObject concept that offers a stop() hook that is called in Document::prepareForDestruction() just before the event queue is stopped. This allows the PaymentRequest, which is an ActiveDOMObject, to clean up in time.Can we do something similar? Just as a strawman:
- Introduce a blink::ActiveDOMObject class
- Store a list of blink::ActiveDOMObjects in blink::ExecutionContext
- Call stop() on each execution context in Document::Shutdown() in the DocumentLifecycle::kStopping phase
- PaymentRequest will inherit from blink::ActiveDOMObject and settle promises in stop()
This is what ContextLifecycleObserver does. Implementing ContextLifecycleObserver and ActiveScriptWrappable together is a common pattern. There's a comment in context_lifecycle_observer.h that describes this in more detail.(That we have a similar concept is no coincidence: we inherited ActiveDOMObject from WebKit when we forked; we just like renaming/refactoring things.)Got it. There is still one thing missing: ContextLifecycleObserver doesn't provide a "context-will-be-destroyed" callback a la WebKit's ActiveDOMObject::stop() method. How about this?
- Add ExecutionContext::NotifyContextWillBeDestroyed() to call a new pure virtual ContextLifecycleObserver::ContextWillBeDestroyed() method
- PaymentRequest implements ContextLifecycleObserver::ContextWillBeDestroyed() to settle the promises
- Call ExecutionContext::NotifyContextWillBeDestroyed() in Document::Shutdown() in the DocumentLifecycle::kStopping phase to allow observers to clean up.
It's not clear to me that this would solve anything (the promises' handlers still wouldn't run until the next microtask checkpoint).
As Dave brought to my attention, this is an instance of the broader question of whether promises returned by HTML/WebIDL operations should work after the context that produced them has gone away.I think the bindings team has historically opposed allowing WebIDL interfaces to continue to operate under those conditions (e.g., I think functions that take success/error callbacks or event handlers don't call them on shutdown), so maybe leaving it unresolved is the right choice here.