Add canMakeActivePayment() method to web payments.

Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then(result => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7AQAJ

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

The feature is behind
chrome://flags/#enable-experimental-web-platform-features until it is
approved to ship.

BUG=662931, 664619
Committed: https://crrev.com/5132963daf4b866cc573b06ff8e84f9b6ec4534c
Cr-Commit-Position: refs/heads/master@{#433164}

[please use gerrit instead, 2016-11-02 19:25:43]

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

[commit-bot: I haz the power, 2016-11-02 19:26:10]

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-11-02 19:48:57]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-11-02 20:48:45]

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

[commit-bot: I haz the power, 2016-11-02 20:48:46]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[please use gerrit instead, 2016-11-07 16:09:09]

Description was changed from

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then((result) => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

BUG=FILEME
==========

to

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then((result) => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called nevertheless.

This is a complete implementation of an additive feature (does not
change existing functionality), so there's no need to hide it behind a
flag.

BUG=662931
==========

[please use gerrit instead, 2016-11-09 19:27:43]

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

[commit-bot: I haz the power, 2016-11-09 19:28:03]

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-11-09 19:31:16]

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

[commit-bot: I haz the power, 2016-11-09 19:31:17]

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-11-09 23:12:50]

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

[commit-bot: I haz the power, 2016-11-09 23:13:40]

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-11-09 23:14:31]

Patchset #2 (id:40001) has been deleted

[please use gerrit instead, 2016-11-09 23:14:43]

Patchset #1 (id:20001) has been deleted

[please use gerrit instead, 2016-11-09 23:15:24]

Description was changed from

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then((result) => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called nevertheless.

This is a complete implementation of an additive feature (does not
change existing functionality), so there's no need to hide it behind a
flag.

BUG=662931
==========

to

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then((result) => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

This is a complete implementation of an additive feature (does not
change existing functionality), so there's no need to hide it behind a
flag.

BUG=662931
==========

[please use gerrit instead, 2016-11-09 23:17:30]

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

[please use gerrit instead, 2016-11-09 23:17:31]

mek@, ptal WebKit.

ianwen@, ptal Java.

[please use gerrit instead, 2016-11-09 23:19:43]

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

[please use gerrit instead, 2016-11-09 23:19:44]

tsepez@, ptal payment_request.mojom.

[commit-bot: I haz the power, 2016-11-10 00:10:34]

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

[commit-bot: I haz the power, 2016-11-10 00:10:35]

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

[haraken, 2016-11-10 01:54:45]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2016-11-10 01:54:45]

https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:532:
!scriptState->domWindow() || !scriptState->domWindow()->frame()) {

We normally check scriptState->contextIsValid().

[please use gerrit instead, 2016-11-10 14:58:27]

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

[commit-bot: I haz the power, 2016-11-10 14:58:50]

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-11-10 14:59:13]

https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:532:
!scriptState->domWindow() || !scriptState->domWindow()->frame()) {
On 2016/11/10 01:54:45, haraken wrote:
> 
> We normally check scriptState->contextIsValid().

Done.

https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:532:
!scriptState->domWindow() || !scriptState->domWindow()->frame()) {
On 2016/11/10 01:54:45, haraken wrote:
> 
> We normally check scriptState->contextIsValid().

Done.

[Tom Sepez, 2016-11-10 16:53:45]

mojom LGTM

[commit-bot: I haz the power, 2016-11-10 16:55:34]

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

[commit-bot: I haz the power, 2016-11-10 16:55:35]

Dry run: This issue passed the CQ dry run.

[Ian Wen, 2016-11-10 20:21:49]

ianwen@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Ian Wen, 2016-11-10 20:21:50]

I will still pass to dfalcantara since the java tests need to be approved by him
anyway.

Java part mostly l-g-t-m, except one potential memory improvement.

https://codereview.chromium.org/2467393002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2467393002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:941:
if (sCanMakeActivePaymentQueries == null) sCanMakeActivePaymentQueries = new
HashMap<>();
Optional comment:

You might want to consider using ArrayMap from v4 support library. ArrayMap is
android specific and it is designed for better memory efficiency with less data
scale.

I would suggest we use ArrayMap here because:
1. This map is static and it is always kept in memory until process dies.
2. The number of entries in the map is reasonably small.

[please use gerrit instead, 2016-11-10 21:50:31]

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

[commit-bot: I haz the power, 2016-11-10 21:50:53]

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-11-10 21:56:38]

On 2016/11/10 20:21:50, Ian Wen wrote:
> I will still pass to dfalcantara since the java tests need to be approved by
him
> anyway.

I own the payments tests btw:

https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromiu...

https://codereview.chromium.org/2467393002/diff/80001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2467393002/diff/80001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:941:
if (sCanMakeActivePaymentQueries == null) sCanMakeActivePaymentQueries = new
HashMap<>();
On 2016/11/10 20:21:50, Ian Wen wrote:
> Optional comment:
> 
> You might want to consider using ArrayMap from v4 support library. ArrayMap is
> android specific and it is designed for better memory efficiency with less
data
> scale.
> 
> I would suggest we use ArrayMap here because:
> 1. This map is static and it is always kept in memory until process dies.
> 2. The number of entries in the map is reasonably small.

Done.

[commit-bot: I haz the power, 2016-11-10 22:14:32]

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

[commit-bot: I haz the power, 2016-11-10 22:14:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Ian Wen, 2016-11-10 23:50:14]

ianwen@chromium.org changed reviewers:
- dfalcantara@chromium.org

[Ian Wen, 2016-11-10 23:50:16]

Oops, my extension failed me.

lgtm java code, after comments. ;)

https://codereview.chromium.org/2467393002/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2467393002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:955:
mHandler.postDelayed(new Runnable() {
I think we should move the postDelayed() to the else block above.

[Marijn Kruisselbrink, 2016-11-11 00:00:05]

I don't understand what you mean with "This is a complete implementation of an
additive feature (does not change existing functionality), so there's no need to
hide it behind a flag."?

Any API change (new api, change to existing api, new feature for existing api,
etc), especially to a feature that has already shipped is considered a
non-trivial change that has to go through the blink intents process
(https://www.chromium.org/blink#trivial-changes). So you either have to send an
intent-to-implement, implement behind a flag, and send an intent-to-ship, or get
an approved intent-to-implement-and-ship before landing this without a flag.

https://codereview.chromium.org/2467393002/diff/100001/components/payments/pa...
File components/payments/payment_request.mojom (right):

https://codereview.chromium.org/2467393002/diff/100001/components/payments/pa...
components/payments/payment_request.mojom:166: CanMakeActivePayment();
Would it make sense to make this CanMakeActivePayment() =>
(ActivePaymentQueryResult result), rather than sending the result through a
separate method in PaymentRequestClient? Although I guess the separate method
thing also makes sense since it's the same pattern the existing code uses.

[please use gerrit instead, 2016-11-11 21:09:24]

Description was changed from

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then((result) => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

This is a complete implementation of an additive feature (does not
change existing functionality), so there's no need to hide it behind a
flag.

BUG=662931
==========

to

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then((result) => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

BUG=662931,664619
==========

[please use gerrit instead, 2016-11-11 21:11:50]

On 2016/11/11 00:00:05, Marijn Kruisselbrink wrote:
> Any API change (new api, change to existing api, new feature for existing api,
> etc), especially to a feature that has already shipped is considered a
> non-trivial change that has to go through the blink intents process
> (https://www.chromium.org/blink#trivial-changes). So you either have to send
an
> intent-to-implement, implement behind a flag, and send an intent-to-ship, or
get
> an approved intent-to-implement-and-ship before landing this without a flag.

Sent out an intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

https://codereview.chromium.org/2467393002/diff/100001/components/payments/pa...
File components/payments/payment_request.mojom (right):

https://codereview.chromium.org/2467393002/diff/100001/components/payments/pa...
components/payments/payment_request.mojom:166: CanMakeActivePayment();
On 2016/11/11 00:00:05, Marijn Kruisselbrink wrote:
> Would it make sense to make this CanMakeActivePayment() =>
> (ActivePaymentQueryResult result), rather than sending the result through a
> separate method in PaymentRequestClient? Although I guess the separate method
> thing also makes sense since it's the same pattern the existing code uses.

I'd prefer to stick to the pre-existing pattern in this file.

[please use gerrit instead, 2016-11-11 21:58:14]

Description was changed from

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then((result) => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

BUG=662931,664619
==========

to

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then(result => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

BUG=662931,664619
==========

[please use gerrit instead, 2016-11-11 22:22:26]

https://codereview.chromium.org/2467393002/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2467393002/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:955:
mHandler.postDelayed(new Runnable() {
On 2016/11/10 23:50:15, Ian Wen wrote:
> I think we should move the postDelayed() to the else block above.

Done.

[please use gerrit instead, 2016-11-12 00:13:03]

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

[commit-bot: I haz the power, 2016-11-12 00:13:57]

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

[haraken, 2016-11-12 01:27:35]

On 2016/11/10 14:59:13, rouslan wrote:
>
https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):
> 
>
https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:532:
> !scriptState->domWindow() || !scriptState->domWindow()->frame()) {
> On 2016/11/10 01:54:45, haraken wrote:
> > 
> > We normally check scriptState->contextIsValid().
> 
> Done.
> 
>
https://codereview.chromium.org/2467393002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:532:
> !scriptState->domWindow() || !scriptState->domWindow()->frame()) {
> On 2016/11/10 01:54:45, haraken wrote:
> > 
> > We normally check scriptState->contextIsValid().
> 
> Done.

Thanks, LGTM on my side.

[commit-bot: I haz the power, 2016-11-12 02:01:22]

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

[commit-bot: I haz the power, 2016-11-12 02:01:23]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[please use gerrit instead, 2016-11-15 16:16:30]

On 2016/11/11 21:11:50, rouslan wrote:
> Sent out an intent to implement and ship:
>
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Marijn, should this be landed behind a runtime flag while waiting for approval
to ship?

[Marijn Kruisselbrink, 2016-11-16 19:25:43]

On 2016/11/15 at 16:16:30, rouslan wrote:
> On 2016/11/11 21:11:50, rouslan wrote:
> > Sent out an intent to implement and ship:
> >
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...
> 
> Marijn, should this be landed behind a runtime flag while waiting for approval
to ship?

That's up to you I think. Either wait for the approval to ship before landing,
or land behind a flag and flip the flag when approved to ship. As an aside it's
a bit unfortunate that the whole PaymentRequest API is currently only exposed on
android, as that (apparently) means it's API service isn't guarded by the tests
in virtual/stable/webexposed, which generally enforces that you'll at least get
some API owner to approve any change that changes web exposed API that isn't
behind any flags

[Marijn Kruisselbrink, 2016-11-16 19:25:43]

On 2016/11/15 at 16:16:30, rouslan wrote:
> On 2016/11/11 21:11:50, rouslan wrote:
> > Sent out an intent to implement and ship:
> >
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...
> 
> Marijn, should this be landed behind a runtime flag while waiting for approval
to ship?

That's up to you I think. Either wait for the approval to ship before landing,
or land behind a flag and flip the flag when approved to ship. As an aside it's
a bit unfortunate that the whole PaymentRequest API is currently only exposed on
android, as that (apparently) means it's API service isn't guarded by the tests
in virtual/stable/webexposed, which generally enforces that you'll at least get
some API owner to approve any change that changes web exposed API that isn't
behind any flags

[please use gerrit instead, 2016-11-17 00:41:00]

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

[commit-bot: I haz the power, 2016-11-17 00:42:22]

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-11-17 00:43:05]

Marijn, ptal patch 5.

I've hidden the feature behind a flag and notified blink-dev:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/etSuMidk...

[commit-bot: I haz the power, 2016-11-17 02:26:21]

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

[commit-bot: I haz the power, 2016-11-17 02:26:22]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[please use gerrit instead, 2016-11-17 18:03:59]

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

[commit-bot: I haz the power, 2016-11-17 18:04:34]

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

[Marijn Kruisselbrink, 2016-11-17 18:06:21]

Two small questions around expected behavior from a spec point of view, but I
imagine that once the spec is finalized you'll have to go over to make sure the
implementation actually matches it anyway, so it's probably fine to just leave
this as is for now too. LGTM

https://codereview.chromium.org/2467393002/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2467393002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:953:
if (sCanMakeActivePaymentQueries.containsKey(mOrigin)) {
Is mOrigin the correct key to use here? I realize the spec is still a bit in
flux, but currently is written in terms of TLD, not origin. (and also is
WebContents::GetVisibleURL() rather than GetLastCommittedURL() the correct URL
to use to make security decisions?)

https://codereview.chromium.org/2467393002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2467393002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:881:
m_canMakeActivePaymentResolver->reject("Query quota exceeded");
I would expect the spec to say to reject with a QuotaExceededError, rather than
reject with a plain string.

[please use gerrit instead, 2016-11-17 19:52:56]

Description was changed from

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then(result => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

BUG=662931,664619
==========

to

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then(result => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

The feature is behind
chrome://flags/#enable-experimental-web-platform-features until it is
approved to ship.

BUG=662931,664619
==========

[please use gerrit instead, 2016-11-17 20:07:21]

Addressed the comments. Noted in CL description that the feature is behind a
flag. Sending to CQ.

https://codereview.chromium.org/2467393002/diff/140001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2467393002/diff/140001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:953:
if (sCanMakeActivePaymentQueries.containsKey(mOrigin)) {
After reading the comments on these methods, I should always be using
GetLastCommittedURL() for mOrigin.

https://codereview.chromium.org/2467393002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2467393002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:881:
m_canMakeActivePaymentResolver->reject("Query quota exceeded");
On 2016/11/17 18:06:21, Marijn Kruisselbrink wrote:
> I would expect the spec to say to reject with a QuotaExceededError, rather
than
> reject with a plain string.

Done.

[please use gerrit instead, 2016-11-17 20:08:44]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-11-17 20:08:45]

The patchset sent to the CQ was uploaded after l-g-t-m from tsepez@chromium.org,
ianwen@chromium.org, haraken@chromium.org, mek@chromium.org
Link to the patchset: https://codereview.chromium.org/2467393002/#ps200001
(title: "Address comments")

[commit-bot: I haz the power, 2016-11-17 20:10:04]

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-11-17 20:29:19]

The CQ bit was unchecked by rouslan@chromium.org

[please use gerrit instead, 2016-11-17 20:30:02]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-11-17 20:30:03]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, ianwen@chromium.org, mek@chromium.org, tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2467393002/#ps220001
(title: "Fix typo")

[commit-bot: I haz the power, 2016-11-17 20:30:56]

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-11-17 21:56:33]

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

[commit-bot: I haz the power, 2016-11-17 21:56:35]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[please use gerrit instead, 2016-11-17 21:57:57]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2016-11-17 21:59:39]

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-11-17 22:35:33]

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

[commit-bot: I haz the power, 2016-11-17 22:35:34]

Try jobs failed on following builders:
  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-11-17 22:42:16]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2016-11-17 22:43:12]

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-11-18 00:06:22]

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

[commit-bot: I haz the power, 2016-11-18 00:06:24]

Try jobs failed on following builders:
  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-11-18 00:10:33]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-11-18 00:10:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, ianwen@chromium.org, mek@chromium.org, tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2467393002/#ps240001
(title: "Fix FindBugs")

[commit-bot: I haz the power, 2016-11-18 00:11:49]

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-11-18 06:08:27]

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

[commit-bot: I haz the power, 2016-11-18 06:08:29]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[please use gerrit instead, 2016-11-18 09:30:16]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2016-11-18 09:30:47]

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-11-18 10:04:31]

Committed patchset #10 (id:240001)

[commit-bot: I haz the power, 2016-11-18 10:06:52]

Description was changed from

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then(result => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

The feature is behind
chrome://flags/#enable-experimental-web-platform-features until it is
approved to ship.

BUG=662931,664619
==========

to

==========
Add canMakeActivePayment() method to web payments.

canMakeActivePayment() is a proposed function on PaymentRequest object.

Proposal:
https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md

Example usage:
pr.canMakeActivePayment()
  .then(result => { if (result) return pr.show(); })
  .catch(error => { console.log(error); });

When canMakeActivePayment() is called, Chrome stores the website origin
and the payment methods that it's checking in memory. That's shared
across the whole browser, in global state. (Not storing this on disk, so
the user can clear this data via browser restart.) Then Chrome starts a
timer for 30 minutes. When the timer fires, Chrome removes that origin
and the payment methods that it was checking from the list. If the same
origin tries to check different payment methods within the 30 minute
window, Chrome rejects that request. If canMakeActivePayment() is
rejected, then pr.show() can still be called regardless.

Intent to implement and ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/IoIxRpn6l9g/ux1C1Cj7...

Tag review:
https://github.com/w3ctag/spec-reviews/issues/146

OWP launch tracking bug:
http://crbug.com/664619

Link to entry on the feature dashboard:
https://www.chromestatus.com/feature/5702608073261056

The feature is behind
chrome://flags/#enable-experimental-web-platform-features until it is
approved to ship.

BUG=662931,664619

Committed: https://crrev.com/5132963daf4b866cc573b06ff8e84f9b6ec4534c
Cr-Commit-Position: refs/heads/master@{#433164}
==========

[commit-bot: I haz the power, 2016-11-18 10:06:54]

Patchset 10 (id:??) landed as
https://crrev.com/5132963daf4b866cc573b06ff8e84f9b6ec4534c
Cr-Commit-Position: refs/heads/master@{#433164}