[Web Payment] At most one IS_READY_TO_PAY service

Before this patch, the platform-specific Chrome OS Android app
communication code was checking the number of IS_READY_TO_PAY services
in an Android app, which could cause duplication of logic between Chrome
OS and Android.

This patch moves the check for number of IS_READY_TO_PAY services from
the platform specific Chrome OS Android app communication to the
cross-platform Android payment app factory.

After this patch, the logic for checking the number of IS_READY_TO_PAY
services is cross-platform.

Bug: 1061503
Change-Id: I72f3b3040771174d61e72bd71afe5665c5a30865
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2302369
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Reviewed-by: Sahel Sharify <sahel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798584}

components/payments/content/android_app_communication_chrome_os.cc

@@ -56,12 +56,6 @@ void OnIsImplemented(
     return;
   }
 
-  if (response->get_valid()->service_names.size() > 1U) {
-    std::move(callback).Run(errors::kMoreThanOneService,
-                            /*app_descriptions=*/{});
-    return;
-  }
-
   auto activity = std::make_unique<AndroidActivityDescription>();
   activity->name = response->get_valid()->activity_names.front();
 

components/payments/content/android_app_communication_unittest.cc

@@ -175,17 +175,28 @@ TEST_F(AndroidAppCommunicationTest, TwoServicesInPackage) {
       base::BindOnce(&AndroidAppCommunicationTest::OnGetAppDescriptionsResponse,
                      base::Unretained(this)));
 
+  EXPECT_FALSE(error_.has_value());
   if (support_->AreAndroidAppsSupportedOnThisPlatform()) {
-    ASSERT_TRUE(error_.has_value());
-    EXPECT_EQ(
-        "Found more than one IS_READY_TO_PAY service in the Trusted Web "
-        "Activity, but at most one service is supported.",
-        error_.value());
+    ASSERT_EQ(1u, apps_.size());
+    ASSERT_NE(nullptr, apps_.front().get());
+    EXPECT_EQ("com.example.app", apps_.front()->package);
+
+    // The logic for checking for multiple services is cross-platform in
+    // android_payment_app_factory.cc, so the platform-specific implementations
+    // of android_app_communication.h do not check for this error condition.
+    std::vector<std::string> expected_service_names = {
+        "com.example.app.ServiceOne", "com.example.app.ServiceTwo"};
+    EXPECT_EQ(expected_service_names, apps_.front()->service_names);
+
+    ASSERT_EQ(1u, apps_.front()->activities.size());
+    ASSERT_NE(nullptr, apps_.front()->activities.front().get());
+    EXPECT_EQ("com.example.app.Activity",
+              apps_.front()->activities.front()->name);
+    EXPECT_EQ("https://play.google.com/billing",
+              apps_.front()->activities.front()->default_payment_method);
   } else {
-    EXPECT_FALSE(error_.has_value());
+    EXPECT_TRUE(apps_.empty());
   }
-
-  EXPECT_TRUE(apps_.empty());
 }
 
 TEST_F(AndroidAppCommunicationTest, ActivityAndService) {

components/payments/content/android_payment_app_factory.cc

@@ -98,6 +98,11 @@ class AppFinder : public base::SupportsUserData::Data {
     std::vector<std::unique_ptr<AndroidAppDescription>> single_activity_apps;
     for (size_t i = 0; i < app_descriptions.size(); ++i) {
       auto app = std::move(app_descriptions[i]);
+      if (app->service_names.size() > 1U) {
+        delegate_->OnPaymentAppCreationError(errors::kMoreThanOneService);
+        continue;
+      }
+
       SplitPotentiallyMultipleActivities(std::move(app), &single_activity_apps);
     }
 

components/payments/content/android_payment_app_factory_unittest.cc

@@ -357,7 +357,40 @@ TEST_F(AndroidPaymentAppFactoryTest,
   apps.back()->activities.back()->name = "com.twa.app.ActivityTwo";
   apps.back()->activities.back()->default_payment_method =
       "https://example.test";
+  support_->ExpectQueryListOfPaymentAppsAndRespond(std::move(apps));
+
+  factory_.Create(delegate_.GetWeakPtr());
+}
+
+// At most one IS_READY_TO_PAY service is allowed in an Android payment app.
+TEST_F(AndroidPaymentAppFactoryTest, ReturnErrorWhenMoreThanOneServiceInApp) {
+  // Enable invoking Android payment apps on those platforms that support it.
+  auto scoped_initialization_ = support_->CreateScopedInitialization();
+
+  EXPECT_CALL(delegate_, GetTwaPackageName())
+      .WillRepeatedly(testing::Return("com.example.app"));
+  EXPECT_CALL(delegate_, OnDoneCreatingPaymentApps());
 
+  EXPECT_CALL(delegate_, OnPaymentAppCreationError(
+                             "Found more than one IS_READY_TO_PAY service, but "
+                             "at most one service is supported."))
+      .Times(support_->AreAndroidAppsSupportedOnThisPlatform() ? 1 : 0);
+
+  EXPECT_CALL(delegate_, OnPaymentAppCreated(testing::_)).Times(0);
+
+  std::vector<std::unique_ptr<AndroidAppDescription>> apps;
+  apps.emplace_back(std::make_unique<AndroidAppDescription>());
+  apps.back()->package = "com.example.app";
+
+  // Two IS_READY_TO_PAY services:
+  apps.back()->service_names.push_back("com.example.app.ServiceOne");
+  apps.back()->service_names.push_back("com.example.app.ServiceTwo");
+
+  apps.back()->activities.emplace_back(
+      std::make_unique<AndroidActivityDescription>());
+  apps.back()->activities.back()->name = "com.example.app.Activity";
+  apps.back()->activities.back()->default_payment_method =
+      "https://play.google.com/billing";
   support_->ExpectQueryListOfPaymentAppsAndRespond(std::move(apps));
 
   factory_.Create(delegate_.GetWeakPtr());

components/payments/core/chrome_os_error_strings.cc

@@ -15,10 +15,6 @@ const char kMoreThanOneActivity[] =
     "Found more than one PAY activity in the Trusted Web Activity, but at most "
     "one activity is supported.";
 
-const char kMoreThanOneService[] =
-    "Found more than one IS_READY_TO_PAY service in the Trusted Web Activity, "
-    "but at most one service is supported.";
-
 const char kMoreThanOneMethodData[] =
     "At most one payment method specific data is supported.";
 

components/payments/core/chrome_os_error_strings.h

@@ -20,9 +20,6 @@ extern const char kInvalidResponse[];
 // Used when the TWA declares more than one PAY activity.
 extern const char kMoreThanOneActivity[];
 
-// Used when the TWA declares more than one IS_READY_TO_PAY service.
-extern const char kMoreThanOneService[];
-
 // Used when the merchant invokes the Trusted Web Activity with more than set of
 // payment method specific data.
 extern const char kMoreThanOneMethodData[];

components/payments/core/native_error_strings.cc

@@ -206,5 +206,9 @@ const char kUnableToInvokeAndroidPaymentApps[] =
 
 const char kUserClosedPaymentApp[] = "User closed the payment app.";
 
+const char kMoreThanOneService[] =
+    "Found more than one IS_READY_TO_PAY service, but at most one service is "
+    "supported.";
+
 }  // namespace errors
 }  // namespace payments

components/payments/core/native_error_strings.h

@@ -234,6 +234,9 @@ extern const char kUnableToInvokeAndroidPaymentApps[];
 // indicates this by returning Activity.RESULT_CANCELED.
 extern const char kUserClosedPaymentApp[];
 
+// Used when an Android app declares more than one IS_READY_TO_PAY service.
+extern const char kMoreThanOneService[];
+
 }  // namespace errors
 }  // namespace payments