Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2640233003:
PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. (Closed)
DescriptionPaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer.
Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with
the frame's MiscPlatformAPI timer task.
The spec link:
https://w3c.github.io/browser-payment-api/#complete-method
This CL is migrating tasks/timers from a per-renderer scheduler to a per-frame
scheduler:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/80dsbEEOcWY
BUG=624694
Review-Url: https://codereview.chromium.org/2640233003
Cr-Commit-Position: refs/heads/master@{#445036}
Committed: https://chromium.googlesource.com/chromium/src/+/b94f7d0a43b09ec3d70328c53d5a08312b1e0baf
Patch Set 1 #Patch Set 2 : Use MiscPlatformAPI #
Messages
Total messages: 28 (12 generated)
The CQ bit was checked by jinho.bang@samsung.com to run a CQ dry run
Description was changed from ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's UserIteraction timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method BUG=624694 ========== to ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's UserIteraction timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method BUG=624694 ==========
jinho.bang@samsung.com changed reviewers: + haraken@chromium.org, rouslan@chromium.org
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL
LGTM The spec is not explicitly saying that it should use the user interaction task source, but "Calling the complete() method tells the user agent that the user interaction is over" indicates that it should use the user interaction task source.
On 2017/01/19 14:04:25, haraken wrote: > The spec is not explicitly saying that it should use the user interaction task > source, but "Calling the complete() method tells the user agent that the user > interaction is over" indicates that it should use the user interaction task > source. Yeah.. So, I was so confusing. Should we change it to MiscPlatformAPI? It closes the UI but the delay can happen in other reasons (e.g. server response delay in merchant side).
On 2017/01/19 14:09:30, zino wrote: > On 2017/01/19 14:04:25, haraken wrote: > > The spec is not explicitly saying that it should use the user interaction task > > source, but "Calling the complete() method tells the user agent that the user > > interaction is over" indicates that it should use the user interaction task > > source. > > Yeah.. So, I was so confusing. Should we change it to MiscPlatformAPI? > It closes the UI but the delay can happen in other reasons (e.g. server response > delay in merchant side). Ah, sorry. I got it. Please ignore my previous comment. Thanks.
Description was changed from ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's UserIteraction timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method BUG=624694 ========== to ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's UserIteraction timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method The spec is not explicitly saying that it should use the user interaction task source, but "Calling the complete() method tells the user agent that the user interaction is over" indicates that it should use the user interaction task source. BUG=624694 ==========
On 2017/01/19 14:22:32, zino wrote: > On 2017/01/19 14:09:30, zino wrote: > > On 2017/01/19 14:04:25, haraken wrote: > > > The spec is not explicitly saying that it should use the user interaction > task > > > source, but "Calling the complete() method tells the user agent that the > user > > > interaction is over" indicates that it should use the user interaction task > > > source. > > > > Yeah.. So, I was so confusing. Should we change it to MiscPlatformAPI? > > It closes the UI but the delay can happen in other reasons (e.g. server > response > > delay in merchant side). > > Ah, sorry. I got it. Please ignore my previous comment. > > Thanks. Let's wait for a comment from rouslan.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The spec should say that the transaction is over after PaymentInstrument.complete(result) is called, not user interaction. In fact, user agent blocks user from interacting with the webpage until this call. This is when the usage agent shows a "Transaction processing..." message. I've filed https://github.com/w3c/browser-payment-api/issues/402 to fix the spec. Let's not land this patch?
On 2017/01/19 15:17:07, rouslan wrote: > The spec should say that the transaction is over after > PaymentInstrument.complete(result) is called, not user interaction. In fact, > user agent blocks user from interacting with the webpage until this call. This > is when the usage agent shows a "Transaction processing..." message. I've filed > https://github.com/w3c/browser-payment-api/issues/402 to fix the spec. Let's not > land this patch? Then shall we use MiscPlatformAPI instead? It's a TaskType for tasks that are speced but not yet associated with any task source in the spec.
On 2017/01/19 15:27:15, haraken wrote: > On 2017/01/19 15:17:07, rouslan wrote: > > The spec should say that the transaction is over after > > PaymentInstrument.complete(result) is called, not user interaction. In fact, > > user agent blocks user from interacting with the webpage until this call. This > > is when the usage agent shows a "Transaction processing..." message. I've > filed > > https://github.com/w3c/browser-payment-api/issues/402 to fix the spec. Let's > not > > land this patch? > > Then shall we use MiscPlatformAPI instead? It's a TaskType for tasks that are > speced but not yet associated with any task source in the spec. OK.
Description was changed from ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's UserIteraction timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method The spec is not explicitly saying that it should use the user interaction task source, but "Calling the complete() method tells the user agent that the user interaction is over" indicates that it should use the user interaction task source. BUG=624694 ========== to ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's UserIteraction timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method This CL is migrating tasks/timers from a per-renderer scheduler to a per-frame scheduler: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/80dsbEEOcWY BUG=624694 ==========
On 2017/01/19 15:28:16, rouslan wrote: > On 2017/01/19 15:27:15, haraken wrote: > > On 2017/01/19 15:17:07, rouslan wrote: > > > The spec should say that the transaction is over after > > > PaymentInstrument.complete(result) is called, not user interaction. In fact, > > > user agent blocks user from interacting with the webpage until this call. > This > > > is when the usage agent shows a "Transaction processing..." message. I've > > filed > > > https://github.com/w3c/browser-payment-api/issues/402 to fix the spec. Let's > > not > > > land this patch? > > > > Then shall we use MiscPlatformAPI instead? It's a TaskType for tasks that are > > speced but not yet associated with any task source in the spec. > > OK. This CL was also changed to MiscPlatformAPI PTAL. Thanks!
lgtm
lgtm
Please update the description.
LGTM
Description was changed from ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's UserIteraction timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method This CL is migrating tasks/timers from a per-renderer scheduler to a per-frame scheduler: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/80dsbEEOcWY BUG=624694 ========== to ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's MiscPlatformAPI timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method This CL is migrating tasks/timers from a per-renderer scheduler to a per-frame scheduler: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/80dsbEEOcWY BUG=624694 ==========
On 2017/01/19 19:07:54, rouslan wrote: > Please update the description. Done
The CQ bit was checked by jinho.bang@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484904480302070,
"parent_rev": "1f80af17c49d09e2d4fbfca567cf81aca19a23ea", "commit_rev":
"b94f7d0a43b09ec3d70328c53d5a08312b1e0baf"}
Message was sent while issue was closed.
Description was changed from ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's MiscPlatformAPI timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method This CL is migrating tasks/timers from a per-renderer scheduler to a per-frame scheduler: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/80dsbEEOcWY BUG=624694 ========== to ========== PaymentRequest: Move m_completeTimer to frame-specific TaskRunnerTimer. Move m_completeTimer to frame-specific TaskRunnerTimer. This associates it with the frame's MiscPlatformAPI timer task. The spec link: https://w3c.github.io/browser-payment-api/#complete-method This CL is migrating tasks/timers from a per-renderer scheduler to a per-frame scheduler: https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/80dsbEEOcWY BUG=624694 Review-Url: https://codereview.chromium.org/2640233003 Cr-Commit-Position: refs/heads/master@{#445036} Committed: https://chromium.googlesource.com/chromium/src/+/b94f7d0a43b09ec3d70328c53d5a... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/b94f7d0a43b09ec3d70328c53d5a... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
