--
--
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?
On Wed, Jun 19, 2019 at 11:25 PM Kentaro Hara <har...@chromium.org> wrote: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 :)
I think the specs currently are a bit broken. Some of them, like PaymentRequest, were written with a nice behavior of using rejection to signal a failure due to inactivity. But we didn't realize that this would just do nothing, because the event loop on inactive documents is suspended. The result is then that, for documents which become inactive, the promises will stay unsettled, but then if the document becomes active again, the promise will end up rejected. This is weird.There is a related problem with fetches; there is nothing that really states in the specs that fetches should be aborted when the document becomes inactive, or that you shouldn't be able to start new fetches from inactive documents. (E.g. via `inactiveIframe.contentWindow.eval("fetch('...')")`.)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.
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.
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.
My message was based on the premise that the spec as-is is broken. It's asking to do something that's basically impossible (*), because you can't reject a promise in an inactive document (since the event loop, which is responsible for promise rejections, does not run).So my suggestion was that specs should: (1) stop doing impossible things purposefully, i.e. remove the reject-after-inactive requirements; (2) also get a blanket update so that even if they try to do impossible things accidentally, nothing bad happens.To expand on (2): you can imagine that there are tons of conditions throughout specs like "If the fetch fails, then reject the promise". To be more correct, we would have to edit every spec to say "If the fetch fails, and the promise's global object's associated Document is active, then reject the promise." This is burdensome to do, and even more burdensome to remember to do, for all future spec updates. Instead we should just make it automatic that "If the fetch fails, then reject the promise" implicitly does nothing when it doesn't work.(*) 'basically' impossible: I am still pretty sure a document could become active again after transitioning out of bfcache...