No rate limit for canMakePayment() on localhost and file://.

No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:

  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."

The integration tests call
PaymentRequestImpl.setIsLocalCanMakePaymentQueryQuotaEnforcedForTest()
to mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
Review-Url: https://codereview.chromium.org/2836443002
Cr-Commit-Position: refs/heads/master@{#466734}
Committed: https://chromium.googlesource.com/chromium/src/+/deb6be8f732a157329490e49fbfd41826757a0fd

[please use gerrit instead, 2017-04-20 20:47:23]

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

[commit-bot: I haz the power, 2017-04-20 20:47:55]

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, 2017-04-20 20:48:42]

rouslan@chromium.org changed reviewers:
+ gogerald@chromium.org, mathp@chromium.org, palmer@chromium.org

[please use gerrit instead, 2017-04-20 20:48:43]

Patch 2:

Chris, ptal origin_security_checker.cc.

Ganggui, ptal removal of the APP_FILTER flag.

Mathieu, ptal EVERYTHING!

[please use gerrit instead, 2017-04-20 20:49:02]

Description was changed from

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:
  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."
The integration tests enable the "WebPaymentsLocalQuota" feature flag to
mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up a no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
==========

to

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:
  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."
The integration tests enable the "WebPaymentsLocalQuota" feature flag to
mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
==========

[commit-bot: I haz the power, 2017-04-20 21:19:44]

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

[commit-bot: I haz the power, 2017-04-20 21:19:45]

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

[palmer, 2017-04-20 21:37:40]

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

https://codereview.chromium.org/2836443002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1330:
} else if (shouldCanMakePaymentQueryQuotaBeEnforced()
Nit: The name |shouldCanMakePaymentQueryQuotaBeEnforced| is a bit of a mouthful.
Maybe |shouldEnforceCanMakePaymentQueryQuota|?

https://codereview.chromium.org/2836443002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1350:
assert query != null;
This will crash the browser, right? Is that the only or best way to handle this?
(Maybe it is, I don't know.)

https://codereview.chromium.org/2836443002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1375:
* enforced only on https:// scheme origins. However, if WEB_PAYMENTS_LOCAL_QUOTA
feature flag
Does this feature flag need to exist?

[please use gerrit instead, 2017-04-21 17:40:44]

Description was changed from

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:
  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."
The integration tests enable the "WebPaymentsLocalQuota" feature flag to
mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
==========

to

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:

  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."

The integration tests call
PaymentRequestImpl.setIsLocalCanMakePaymentQueryEnabledForTest() to
mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
==========

[please use gerrit instead, 2017-04-21 17:54:14]

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

[commit-bot: I haz the power, 2017-04-21 17:54:34]

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, 2017-04-21 17:57:47]

Chris, ptal patch 3.

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

https://codereview.chromium.org/2836443002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1330:
} else if (shouldCanMakePaymentQueryQuotaBeEnforced()
On 2017/04/20 21:37:40, palmer wrote:
> Nit: The name |shouldCanMakePaymentQueryQuotaBeEnforced| is a bit of a
mouthful.
> Maybe |shouldEnforceCanMakePaymentQueryQuota|?

Done.

https://codereview.chromium.org/2836443002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1350:
assert query != null;
On 2017/04/20 21:37:40, palmer wrote:
> This will crash the browser, right? Is that the only or best way to handle
this?
> (Maybe it is, I don't know.)

Thank you for focusing my attention here. After some thought, I've found one
case where it's valid for query to be null. So we should not be asserting it.
I've added a comment about that case.

https://codereview.chromium.org/2836443002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1375:
* enforced only on https:// scheme origins. However, if WEB_PAYMENTS_LOCAL_QUOTA
feature flag
On 2017/04/20 21:37:40, palmer wrote:
> Does this feature flag need to exist?

Ideally we would have a host_resolver() a la browser_test_base, so we can
simulate opening URLs like https://example.com/test-page.html. However Java does
not have the bindings for it and adding them appears relatively complex.
Therefore, I've added a test-only boolean for PaymentRequestImpl instead. This
removes the overhead of a feature flag.

[please use gerrit instead, 2017-04-21 17:58:11]

Description was changed from

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:

  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."

The integration tests call
PaymentRequestImpl.setIsLocalCanMakePaymentQueryEnabledForTest() to
mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
==========

to

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:

  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."

The integration tests call
PaymentRequestImpl.setIsLocalCanMakePaymentQueryQuotaEnforcedForTest()
to mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
==========

[palmer, 2017-04-21 18:13:24]

lgtm

[commit-bot: I haz the power, 2017-04-21 20:08:25]

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

[commit-bot: I haz the power, 2017-04-21 20:08:26]

Dry run: This issue passed the CQ dry run.

[gogerald1, 2017-04-21 21:29:43]

removal of the APP_FILTER flag lgtm

[Mathieu, 2017-04-23 08:33:30]

lgtm with comments

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

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1320:
if (query == null) {
This function would benefit from comments, which will help people modifying this
code in the future. Example:

// When there are no pending queries, start one, etc.

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1331:
mClient.onCanMakePayment(CanMakePaymentQueryResult.QUERY_QUOTA_EXCEEDED);
// There are no cached queries for the method data and the query quota is
enforced.

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1376:
* @return Whether canMakePayment() query quota should be enforeced. By default,
the quota is
*enforced

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1377:
* enforced only on https:// scheme origins. However, if WEB_PAYMENTS_LOCAL_QUOTA
feature flag
where is this flag defined?

[please use gerrit instead, 2017-04-24 18:40:13]

Sending to CQ.

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

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1320:
if (query == null) {
On 2017/04/23 08:33:30, Mathieu wrote:
> This function would benefit from comments, which will help people modifying
this
> code in the future. Example:
> 
> // When there are no pending queries, start one, etc.

Done.

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1331:
mClient.onCanMakePayment(CanMakePaymentQueryResult.QUERY_QUOTA_EXCEEDED);
On 2017/04/23 08:33:30, Mathieu wrote:
> // There are no cached queries for the method data and the query quota is
> enforced.

Done.

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1376:
* @return Whether canMakePayment() query quota should be enforeced. By default,
the quota is
On 2017/04/23 08:33:30, Mathieu wrote:
> *enforced

It's the... American spelling? LOL JK (Done.)

https://codereview.chromium.org/2836443002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1377:
* enforced only on https:// scheme origins. However, if WEB_PAYMENTS_LOCAL_QUOTA
feature flag
On 2017/04/23 08:33:30, Mathieu wrote:
> where is this flag defined?

This flag has been removed in later iterations of the patch. I've updated the
comment.

[please use gerrit instead, 2017-04-24 18:40:33]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2017-04-24 18:40:34]

The patchset sent to the CQ was uploaded after l-g-t-m from palmer@chromium.org,
mathp@chromium.org, gogerald@chromium.org
Link to the patchset: https://codereview.chromium.org/2836443002/#ps60001
(title: "Comments")

[commit-bot: I haz the power, 2017-04-24 18:41:25]

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, 2017-04-24 20:22:09]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493059233856390,
"parent_rev": "55fc2829afb24aa310dde477d64d07d35a32e46e", "commit_rev":
"deb6be8f732a157329490e49fbfd41826757a0fd"}

[commit-bot: I haz the power, 2017-04-24 20:22:17]

Description was changed from

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:

  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."

The integration tests call
PaymentRequestImpl.setIsLocalCanMakePaymentQueryQuotaEnforcedForTest()
to mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217
==========

to

==========
No rate limit for canMakePayment() on localhost and file://.

Before this patch, when web developers are working on localhost or in
file:// scheme origins and calling PaymentRequest.canMakePayment() with
different payment methods, they would quickly hit a rate limit, even
though they are querying their own payment methods.

The fix is to warn the web developers on localhost and file:// scheme
origins that the quota has been reached, but to not enforce it. This
warning is printed to the developer console:

  "Quota reached for PaymentRequest.canMakePayment(). This would
   normally reject the promise, but allowing continued usage on
   localhost and file:// scheme origins."

The integration tests call
PaymentRequestImpl.setIsLocalCanMakePaymentQueryQuotaEnforcedForTest()
to mimic the production behavior.

After this patch, the web developers have the freedom to tinker with
their own computer and are aware that excessive querying in production
website will be rejected.

(Note this patch also cleans up the no longer used feature flag
"AndroidPaymentAppsFilter".)

BUG=713217

Review-Url: https://codereview.chromium.org/2836443002
Cr-Commit-Position: refs/heads/master@{#466734}
Committed:
https://chromium.googlesource.com/chromium/src/+/deb6be8f732a157329490e49fbfd...
==========

[commit-bot: I haz the power, 2017-04-24 20:22:21]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/deb6be8f732a157329490e49fbfd...

[findit-for-me, 2017-04-25 06:02:51]

Findit(https://goo.gl/kROfz5) identified this CL at revision 466734 as the
culprit for
failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3Itb...