New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 745765 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Payment app is not available some of the time

Project Member Reported by rouslan@chromium.org, Jul 18 2017

Issue description

Chrome Version       : 61.0.3160.0
OS Version: Android 7.1.2 Nougat, Nexus 6p
URL: https://rsolomakhin.github.io/pr/bob/

What steps will reproduce the problem?
1. Install BobPay from https://drive.google.com/a/google.com/file/d/0B3ISiXgGE1MNc1FCNlZkN3FMMWs/view?usp=sharing
2. Open https://rsolomakhin.github.io/pr/bob/
3. Tap "Buy" on page and "X" in the PaymentRequest UI several times.

What is the expected result?
BobPay should always be available. PaymentRequest UI should always show up.

What happens instead of that?
BobPay is not available some of the time: PaymentRequest UI does show, .show() promise rejects with NotSupportedError.

 
Screenshot (Jul 18, 2017 10-54-11 AM).png
235 KB View Download
intermittent.mp4
16.9 MB Download
Labels: M-60
Owner: gogerald@chromium.org
Status: Assigned (was: Untriaged)
Ganggui: Could this have anything to do with the way we cache the web app manifest?
Status: Started (was: Assigned)
For record:

This seems because our bobpay app have two manifests (https://rsolomakhin.github.io/bobpay/payment-manifest.json), both of them will be downloaded, parsed and cached independently, then one will override the other. To support multiple signature keys, we should put them in one manifest.

Cc: -rouslan@chromium.org
Owner: rouslan@chromium.org
Found a couple of issues in the manifest parsing and caching. Will send out CLs shortly.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9e00df079b7f71c9e2f478352eefb5713e8f50df

commit 9e00df079b7f71c9e2f478352eefb5713e8f50df
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Tue Jul 18 21:37:33 2017

[Payments] Multiple sections in web app manifest.

Before this patch, parsing a web app manifest with multiple
"related_applications" would return an empty manifest. Multiple
different "fingerprints" would return the first fingerprint multiple
times. These bugs occurred because of incorrect usage of index "i"
instead of "j" for "fingerprints". The "i" index is used only for
"related_applications".

This patch adds tests TwoRelatedApplicationsWellFormed for parsing two
"related_applications" and TwoDifferentSignaturesWellFormed for parsing
multiple different "fingerprints" in the web app manifest. Before the
fix, the tests fail. The tests pass after the fix, which is to use "j"
instead of "i" index for "fingerprints".

After this patch, parsing a web app manifest with multiple
"related_applications" returns a manifest with two sections. Parsing
multiple different "fingerprints" returns different parsed fingerprints.

Bug:  745765 
Change-Id: I43953748a405e5d3ef2329e22ba3ff449b10f104
Reviewed-on: https://chromium-review.googlesource.com/575715
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487611}
[modify] https://crrev.com/9e00df079b7f71c9e2f478352eefb5713e8f50df/components/payments/content/utility/payment_manifest_parser.cc
[modify] https://crrev.com/9e00df079b7f71c9e2f478352eefb5713e8f50df/components/payments/content/utility/payment_manifest_parser_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jul 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/504ac1c39579d5f7f54fd963309be347001e8d71

commit 504ac1c39579d5f7f54fd963309be347001e8d71
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Tue Jul 18 22:37:55 2017

[Payments] Cache multiple package names per web app manifest.

Before this patch, caching a web app manifest with multiple "id" values
in "related_applications" sections would not always clear the stale data
from the cache. Multiple "id" values in the same web app manifest are
useful for using both prod and dev version of a payment app in the same
web app manifest, for example.

This patch clears the stale data for all "id" values in
"related_applications" of a web app manifest that is being cached.

Bug:  745765 
Change-Id: I116e356d5181792668aa26c6ff4da7258cd11d49
Reviewed-on: https://chromium-review.googlesource.com/575845
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487632}
[modify] https://crrev.com/504ac1c39579d5f7f54fd963309be347001e8d71/components/payments/android/web_app_manifest_section_table.cc
[modify] https://crrev.com/504ac1c39579d5f7f54fd963309be347001e8d71/components/payments/android/web_app_manifest_section_table_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a92039e93eec04fd21b3a9a07131222e5315b939

commit a92039e93eec04fd21b3a9a07131222e5315b939
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Wed Jul 19 01:17:20 2017

[Payments] No random "NotSupportedError" rejects.

Before this patch, if a single payment app "com.bobpay.prod" is
installed for the payment method "https://bobpay.com/webpay", but cache
contains two web app manifests for "https://bobpay.com/webpay"
("com.bobpay.prod" and "com.bobpay.dev"), then only one of the manifests
will be used at random on every cache fetch.

The problem arose from incorrectly fetching both of the
"com.bobpay.prod" and "com.bobpay.dev" web app manifests for
"https://bobpay.com/webpay" from cache, instead of only the manifest for
"com.bobpay.prod", which is the installed app, while also assuming that
only one manifest will be fetched. Once any of the two manifests have
been fetched, it is compared to the installed app. Because the ordering
of the manifest fetches from cache is not deterministic, there is a good
chance that "com.bobpay.dev" will be fetched first and compared to
"com.bobpay.prod" app. (This is a race condition). There will be no
match in this case. To users this looks like an intermittent rejection
of the PaymentRequest.show() promise with "NotSupportedError".

The fix is to fetch from cache only the manifest for the installed
payment app "com.bobpay.prod", instead of all manifests for
"https://bobpay.com/webpay" payment method.

After this patch, if a single payment app "com.bobpay.prod" is installed
for the payment method "https://bobpay.com/webpay", but cache contains
two web app manifests, then only "com.bobpay.prod" manifest will be
fetched from cache. Because the fetched manifest can only match the
installed app, there is no race condition and PaymentRequest.show()
promise is not rejected intermittently with "NotSupportedError".

Bug:  745765 
Change-Id: I10f5fdae13ce7e55ef24bcedd5ce89aeafa3f226
Reviewed-on: https://chromium-review.googlesource.com/575281
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487700}
[modify] https://crrev.com/a92039e93eec04fd21b3a9a07131222e5315b939/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java

Labels: -M-60 M-61
Status: Fixed (was: Started)
Fixed in M-61. Can't merge into M-60, which will be pushed to stable soon.
Verified on Chrome:61.0.3162.0 Device:HTC U11/NMF26X
Project Member

Comment 10 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0db6cad84ebbfee67373f894d6240d81ac2f1d18

commit 0db6cad84ebbfee67373f894d6240d81ac2f1d18
Author: Rouslan Solomakhin <rouslan@chromium.org>
Date: Thu Jul 20 20:59:28 2017

[Payments] Multiple payment apps in single web app manifest.

Before this patch, if a single web app manifest specified multiple
sections of "related_applications" with different "id" (package name)
fields and the user had several of these payment apps installed, then
Chrome would display only one of these payment apps.

The problem arose from an erroneous assumption of 1:1 relationship
between web app manifests and payment apps. In fact, a single web app
manifest can correspond to multiple payment apps, e.g., a dev and a prod
version of the same app.

The fix is to loop through the full web app manifest when looking for
matching payment apps, instead of early-exiting when the first matching
payment app is found.

After this patch, if a single web app manifest specified multiple
sections of "related_applications" with different "id" fields and the
user had several of these payment apps installed, then Chrome displays
all of the matching payment apps.

Bug:  745765 
Change-Id: I70f287cb792ac8b0b073c22e9c08eebec6ffae3e
Reviewed-on: https://chromium-review.googlesource.com/579964
Reviewed-by: Ganggui Tang <gogerald@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488399}
[modify] https://crrev.com/0db6cad84ebbfee67373f894d6240d81ac2f1d18/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java

Sign in to add a comment