Timeout if website never completes the transaction.

Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
TEST=CompleteTest.RejectCompletePromiseAfterTimeout

Committed: https://crrev.com/f981e164be677b2d201a715c5ca96236ee5ac0ab
Cr-Commit-Position: refs/heads/master@{#409116}

[please use gerrit instead, 2016-07-31 19:44:23]

Description was changed from

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

BUG=632708
==========

to

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
==========

[please use gerrit instead, 2016-07-31 19:44:33]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-31 19:44:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-31 22:23:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-31 22:23:11]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-08-01 18:35:43]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[please use gerrit instead, 2016-08-01 18:36:25]

Description was changed from

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
==========

to

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
TEST=CompleteTest.RejectCompletePromiseAfterTimeout
==========

[commit-bot: I haz the power, 2016-08-01 18:36:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[please use gerrit instead, 2016-08-01 18:36:43]

rouslan@chromium.org changed reviewers:
+ mek@chromium.org

[please use gerrit instead, 2016-08-01 18:36:44]

Marijn, ptal patch 2.

[commit-bot: I haz the power, 2016-08-01 18:39:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-01 18:39:38]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[please use gerrit instead, 2016-08-01 20:29:18]

The CQ bit was checked by rouslan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 20:29:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-01 22:17:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-01 22:17:53]

Dry run: This issue passed the CQ dry run.

[Marijn Kruisselbrink, 2016-08-01 22:42:25]

lgtm

https://codereview.chromium.org/2192413002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.h (right):

https://codereview.chromium.org/2192413002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:76: void
onTimerFired(TimerBase*);
Especially if this is going to be a public method "onTimerFired" doesn't seem
like a very good name for it. Maybe something like onCompleteTimeout would be
better? Or at least something to indicate what this timer is.

Also if you don't like having this as a public method, a possibility would be to
make this method private and have a public onCompleteTimeoutForTesting method
that just calls the private method (we have presubmit checks that ensure
ForTesting methods are only called from tests).

https://codereview.chromium.org/2192413002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:108:
Timer<PaymentRequest> m_completeTimer;
(in the future I believe using Timer directly for this is going to be
deprecated, and instead stuff will run on frame specific task runners (see
https://bugs.chromium.org/p/chromium/issues/detail?id=624694 for some info). But
I don't exactly know what is happening there, and it hasn't happened yet anyway,
so for now I believe this is still the correct usage of Timer).

[please use gerrit instead, 2016-08-01 23:55:36]

https://codereview.chromium.org/2192413002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.h (right):

https://codereview.chromium.org/2192413002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:76: void
onTimerFired(TimerBase*);
On 2016/08/01 22:42:25, Marijn Kruisselbrink wrote:
> Especially if this is going to be a public method "onTimerFired" doesn't seem
> like a very good name for it. Maybe something like onCompleteTimeout would be
> better? Or at least something to indicate what this timer is.
> 
> Also if you don't like having this as a public method, a possibility would be
to
> make this method private and have a public onCompleteTimeoutForTesting method
> that just calls the private method (we have presubmit checks that ensure
> ForTesting methods are only called from tests).

Done.

https://codereview.chromium.org/2192413002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:108:
Timer<PaymentRequest> m_completeTimer;
On 2016/08/01 22:42:25, Marijn Kruisselbrink wrote:
> (in the future I believe using Timer directly for this is going to be
> deprecated, and instead stuff will run on frame specific task runners (see
> https://bugs.chromium.org/p/chromium/issues/detail?id=624694 for some info).
But
> I don't exactly know what is happening there, and it hasn't happened yet
anyway,
> so for now I believe this is still the correct usage of Timer).

Acknowledged.

[please use gerrit instead, 2016-08-01 23:55:45]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-08-01 23:55:45]

The patchset sent to the CQ was uploaded after l-g-t-m from mek@chromium.org
Link to the patchset: https://codereview.chromium.org/2192413002/#ps60001
(title: "Marijn's comments")

[commit-bot: I haz the power, 2016-08-01 23:56:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-02 01:32:58]

Description was changed from

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
TEST=CompleteTest.RejectCompletePromiseAfterTimeout
==========

to

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
TEST=CompleteTest.RejectCompletePromiseAfterTimeout
==========

[commit-bot: I haz the power, 2016-08-02 01:32:59]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-08-02 01:35:53]

Description was changed from

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
TEST=CompleteTest.RejectCompletePromiseAfterTimeout
==========

to

==========
Timeout if website never completes the transaction.

If the website does not call complete() within 60 seconds of show()
being resolved, behave as if complete("fail") has been called.

Also in this patch
 - Moved complete() related tests to CompleteTest.cpp.
 - Moved PaymentComple enum inside of PaymentCompleter.

BUG=632708
TEST=CompleteTest.RejectCompletePromiseAfterTimeout

Committed: https://crrev.com/f981e164be677b2d201a715c5ca96236ee5ac0ab
Cr-Commit-Position: refs/heads/master@{#409116}
==========

[commit-bot: I haz the power, 2016-08-02 01:35:54]

Patchset 4 (id:??) landed as
https://crrev.com/f981e164be677b2d201a715c5ca96236ee5ac0ab
Cr-Commit-Position: refs/heads/master@{#409116}