[Web Payment] App store billing never shows browser UI.

Before this patch, immediately resolving the promise passed into
PaymentRequest.show() (the so-called "show() promise"), e.g., by calling
PaymentRequest.show(Promise.resolve({})), would have the possibility of
both showing the browser payment sheet and invoking the payment app,
because the code did not expect that the promise would be resolved
faster than enumerating the locally installed payment apps.

This patch prevents invoking a payment app or showing the browser
payment sheet when the show() promise resolves before the locally
installed payment apps have been enumerated.

After this patch, immediately resolving the show() promise will not
result in both showing the browser payment sheet and invoking the
payment app at the same time.

Bug: 1237921
Change-Id: I132700146d6f9334a5c56d136f80fcdc62873313
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3160759
Reviewed-by: Jeevan Shikaram <jshikaram@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#921617}

chrome/browser/payments/android_payment_app_factory_browsertest.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <memory>
+
 #include "build/chromeos_buildflags.h"
 
 #if BUILDFLAG(IS_CHROMEOS_ASH)
@@ -26,8 +28,25 @@ class AndroidPaymentAppFactoryTest
 
   ~AndroidPaymentAppFactoryTest() override = default;
 
+  void SetUpOnMainThread() override {
+    PaymentRequestPlatformBrowserTestBase::SetUpOnMainThread();
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+    overlay_manager_ = std::make_unique<ash::ArcOverlayManager>();
+#endif
+  }
+
+  // PaymentRequestTestObserver:
+  void OnUIDisplayed() override {
+    PaymentRequestPlatformBrowserTestBase::OnUIDisplayed();
+    FAIL() << "Browser UI should never be displayed for Android payment apps "
+              "invoked here.";
+  }
+
  private:
   base::test::ScopedFeatureList feature_list_;
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  std::unique_ptr<ash::ArcOverlayManager> overlay_manager_;
+#endif
 };
 
 // Even if a service worker app for app store payment method is installed, it
@@ -50,10 +69,6 @@ IN_PROC_BROWSER_TEST_F(AndroidPaymentAppFactoryTest,
 // goods purchase.
 IN_PROC_BROWSER_TEST_F(AndroidPaymentAppFactoryTest,
                        IgnoreOtherPaymentAppsInTwaWhenHaveAppStoreBilling) {
-#if BUILDFLAG(IS_CHROMEOS_ASH)
-  ash::ArcOverlayManager overlay_manager_;
-#endif
-
   std::string method_name = https_server()->GetURL("a.com", "/").spec();
   method_name = method_name.substr(0, method_name.length() - 1);
   ASSERT_NE('/', method_name[method_name.length() - 1]);
@@ -88,5 +103,55 @@ IN_PROC_BROWSER_TEST_F(AndroidPaymentAppFactoryTest,
                     method_name)));
 }
 
+// Passing a promise into PaymentRequest.show() should skip browser sheet with
+// https://play.google.com/billing payment method.
+IN_PROC_BROWSER_TEST_F(AndroidPaymentAppFactoryTest,
+                       ShowPromiseShouldSkipBrowserPaymentSheet) {
+  std::string response = "App store payment method app response for test.";
+  test_controller()->SetTwaPackageName("com.example.app");
+  test_controller()->SetTwaPaymentApp("https://play.google.com/billing",
+                                      "{\"status\": \"" + response + "\"}");
+
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  std::string expected_response = response;
+#else
+  std::string expected_response =
+      "The payment method \"https://play.google.com/billing\" is not "
+      "supported.";
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
+  NavigateTo("b.com", "/payment_handler_status.html");
+  ASSERT_EQ(expected_response,
+            content::EvalJs(
+                GetActiveWebContents(),
+                "getStatusForMethodDataWithShowPromise([{supportedMethods:"
+                "'https://play.google.com/billing'}])"));
+}
+
+// PaymentRequest.show(Promise.resolve({})) should skip browser sheet with
+// https://play.google.com/billing payment method.
+IN_PROC_BROWSER_TEST_F(AndroidPaymentAppFactoryTest,
+                       EmptyShowPromiseShouldSkipBrowserPaymentSheet) {
+  std::string response = "App store payment method app response for test.";
+  test_controller()->SetTwaPackageName("com.example.app");
+  test_controller()->SetTwaPaymentApp("https://play.google.com/billing",
+                                      "{\"status\": \"" + response + "\"}");
+
+#if BUILDFLAG(IS_CHROMEOS_ASH)
+  std::string expected_response = response;
+#else
+  std::string expected_response =
+      "The payment method \"https://play.google.com/billing\" is not "
+      "supported.";
+#endif  // BUILDFLAG(IS_CHROMEOS_ASH)
+
+  NavigateTo("b.com", "/payment_handler_status.html");
+  ASSERT_EQ(expected_response,
+            content::EvalJs(
+                GetActiveWebContents(),
+                "getStatusForMethodDataWithEmptyShowPromise([{supportedMethods:"
+                "'https://play.google.com/billing'}])"));
+}
+
 }  // namespace
 }  // namespace payments

components/payments/content/payment_request.cc

@@ -428,16 +428,18 @@ void PaymentRequest::UpdateWith(mojom::PaymentDetailsPtr details) {
     journey_logger_.RecordTransactionAmount(
         spec_->details().total->amount->currency,
         spec_->details().total->amount->value, false /*completed*/);
-    if (SatisfiesSkipUIConstraints()) {
-      Pay();
-    } else {
-      // If not skipping UI, then make sure that the browser payment sheet is
-      // being displayed.
-      if (!display_handle_->was_shown())
-        display_handle_->Show(weak_ptr_factory_.GetWeakPtr());
-
-      if (spec_->request_shipping())
-        state_->SelectDefaultShippingAddressAndNotifyObservers();
+    if (is_requested_methods_supported_invoked_) {
+      if (SatisfiesSkipUIConstraints()) {
+        Pay();
+      } else {
+        // If not skipping UI, then make sure that the browser payment sheet is
+        // being displayed.
+        if (!display_handle_->was_shown())
+          display_handle_->Show(weak_ptr_factory_.GetWeakPtr());
+
+        if (spec_->request_shipping())
+          state_->SelectDefaultShippingAddressAndNotifyObservers();
+      }
     }
   }
 }
@@ -633,6 +635,7 @@ bool PaymentRequest::ChangeShippingAddress(
 void PaymentRequest::AreRequestedMethodsSupportedCallback(
     bool methods_supported,
     const std::string& error_message) {
+  is_requested_methods_supported_invoked_ = true;
   if (is_show_called_ && spec_ && spec_->IsInitialized() &&
       observer_for_testing_) {
     observer_for_testing_->OnAppListReady(weak_ptr_factory_.GetWeakPtr());

components/payments/content/payment_request.h

@@ -266,7 +266,13 @@ class PaymentRequest : public mojom::PaymentRequest,
   // Whether PaymentRequest.show() has been called.
   bool is_show_called_ = false;
 
-  // If not empty, use this error message for rejecting PaymentRequest.show().
+  // Whether PaymentRequestState::AreRequestedMethodsSupported callback has been
+  // invoked. This is distinct from state_->IsInitialized(), because the
+  // callback is asynchronous.
+  bool is_requested_methods_supported_invoked_ = false;
+
+  // If not empty, use this error message for rejecting
+  // PaymentRequest.show().
   std::string reject_show_error_message_;
 
   base::WeakPtrFactory<PaymentRequest> weak_ptr_factory_{this};

components/test/data/payments/payment_handler_status.js

@@ -91,3 +91,26 @@ async function getStatusForMethodDataWithShowPromise(methodData) { // eslint-dis
     return e.message;
   }
 }
+
+/**
+ * Returns the status field from the payment handler's response for given
+ * payment method data. Passes an empty Promise.resolve({}) promise into
+ * PaymentRequest.show().
+ * @param {array<PaymentMethodData>} methodData - The method data to use.
+ * @return {string} - The status field or error message.
+ */
+async function getStatusForMethodDataWithEmptyShowPromise(methodData) { // eslint-disable-line no-unused-vars, max-len
+  try {
+    const details = {total: {label: 'TEST',
+        amount: {currency: 'USD', value: '0.01'}}};
+    const request = new PaymentRequest(methodData, details);
+    const response = await request.show(Promise.resolve({}));
+    await response.complete();
+    if (!response.details.status) {
+      return 'Payment handler did not specify the status.';
+    }
+    return response.details.status;
+  } catch (e) {
+    return e.message;
+  }
+}