[Payment Handler] Cannot have two instruments per SW |
||||
Issue descriptionThe registration function below doesn't work for either BobPay or AliPay. However, if only one instrument is specified, it works for either. According to the example in the spec, it should work: https://w3c.github.io/payment-handler/#register-example function addInstruments(registration) { const instrumentPromises = [ registration.paymentManager.instruments.set( "dc2de27a-ca5e-4fbd-883e-b6ded6c69d4f", { name: "My Bob Pay Account", icons: [{ src:"/pay/bobpay.png", sizes:"32x32", type:"image/png"} ], enabledMethods: ["https://emerald-eon.appspot.com/bobpay"] }), registration.paymentManager.instruments.set( "c8126178-3bba-4d09-8f00-0771deadbeef", { name: "AliPay (via BobPay)", icons: [{ src:"https://i.alipayobjects.com/common/favicon/favicon.ico", sizes:"32x32", type:"image/png"} ], enabledMethods: ["https://www.alipay.com/webpay"] }), ]; return Promise.all(instrumentPromises); };
,
Aug 30 2017
Ah, sorry I didn't know this issue. I'll check it.
,
Sep 1 2017
Uploaded a patch: https://chromium-review.googlesource.com/647948
,
Sep 2 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ebee332dee901d9acff04911b04deb8e65ba50a8 commit ebee332dee901d9acff04911b04deb8e65ba50a8 Author: Jinho Bang <jinho.bang@samsung.com> Date: Sat Sep 02 02:02:32 2017 PaymentHandler: Fix a bug when setting multiple instruments with icons per SW. It is caused that the callback is duplicated when requesting fetching icon multiple times. So, this patch makes multiple requests to have each independent fetcher. Bug: 752836 Change-Id: Idf123f6e44f2258543c41bd6c11d3cf6dd4db796 Reviewed-on: https://chromium-review.googlesource.com/647948 Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org> Commit-Queue: Jinho Bang <jinho.bang@samsung.com> Cr-Commit-Position: refs/heads/master@{#499373} [modify] https://crrev.com/ebee332dee901d9acff04911b04deb8e65ba50a8/content/browser/payments/payment_app_database.cc [modify] https://crrev.com/ebee332dee901d9acff04911b04deb8e65ba50a8/content/browser/payments/payment_app_database.h [modify] https://crrev.com/ebee332dee901d9acff04911b04deb8e65ba50a8/content/browser/payments/payment_instrument_icon_fetcher.cc [modify] https://crrev.com/ebee332dee901d9acff04911b04deb8e65ba50a8/content/browser/payments/payment_instrument_icon_fetcher.h
,
Sep 5 2017
Should we mark as fixed?
,
Sep 7 2017
The fix looks crashing debug build since weak pointer is not supposed to be invalidated on different threads (https://cs.chromium.org/chromium/src/base/memory/weak_ptr.cc?rcl=00987a165052cb52688e4632b19f239bfc77a35a&l=20). I remembered that's why I used 'base::RefCountedThreadSafe'
,
Sep 11 2017
I uploaded a refactoring patch for PaymentInstrumentIconFetcher. https://chromium-review.googlesource.com/c/chromium/src/+/658201
,
Sep 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/55bacf13d9c8991bca74c1f434dfe73a4afb351e commit 55bacf13d9c8991bca74c1f434dfe73a4afb351e Author: Jinho Bang <jinho.bang@samsung.com> Date: Wed Sep 13 10:45:30 2017 PaymentHandler: Refactor PaymentInstrumentIconFetcher class. The current PaymentInstrumentIconFetcher class is too complicated and can not ensure thread-safety due to misuse some APIs. So, this patch refactors the class to use ManifestIconDownloader internally. After this patch, we can remove much duplicated codes and resolve some crashes in debug mode as well. Bug: 752836 Change-Id: I04c01d433d093ae138148e6aa2849bc1713c1c85 Reviewed-on: https://chromium-review.googlesource.com/658201 Commit-Queue: Jinho Bang <jinho.bang@samsung.com> Reviewed-by: Ganggui Tang <gogerald@chromium.org> Reviewed-by: Rouslan Solomakhin <rouslan@chromium.org> Reviewed-by: Avi Drissman <avi@chromium.org> Cr-Commit-Position: refs/heads/master@{#501589} [modify] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/browser/payments/payment_app_database.cc [modify] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/browser/payments/payment_app_database.h [modify] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/browser/payments/payment_instrument_icon_fetcher.cc [modify] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/browser/payments/payment_instrument_icon_fetcher.h [add] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/test/data/payments/icon-1-5x.png [add] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/test/data/payments/icon-1x.png [add] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/test/data/payments/icon-2x.png [add] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/test/data/payments/icon-3x.png [add] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/test/data/payments/icon-4x.png [add] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/test/data/payments/invalid-icon.png [modify] https://crrev.com/55bacf13d9c8991bca74c1f434dfe73a4afb351e/content/test/data/payments/payment_app_invocation.html
,
Sep 13 2017
,
Sep 13 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by ma...@chromium.org
, Aug 29 2017