From e5ce54350a6811a902f1dd0eb80bde016b72e7bf Mon Sep 17 00:00:00 2001
From: Slobodan Pejic <slobodan@chromium.org>
Date: Tue, 04 Aug 2026 14:15:06 -0700
Subject: [PATCH] Freeze SPC payment details at app creation time

Before this change the total amount was retrieved twice from the spec
object, once to display the total in the Secure Payment Confirmation
dialog, then once more to be included in the signed client data. The
Android dialog retrieved the raw total for display whereas the
SecurePaymentConfirmationApp retrieved the total with modifiers applied
for inclusion into client data, which can result in a mismatch of the
displayed value and the value included in client data.

After this change, SecurePaymentConfirmationApp holds a frozen central
copy of the total for use in both display and inclusion in client data.
And, the Android version now retrieves the total from
SecurePaymentConfirmationApp instead of the spec.

Tradeoff: The generic PaymentApp interface has a new getTotalForSpc()
method that is only used by Secure Payment Confirmation. Note that this
is already the case for the getPaymentEntitiesLogos() method on the same
PaymentApp interface.

This complements the fix in crrev.com/c/7984525 which rejects
updateWith() calls after the SPC dialog has been shown.

Bug: 522304737
Change-Id: I93b88ee164d40dcbc2f877358141c14e0098dd46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7989196
Commit-Queue: Slobodan Pejic <slobodan@chromium.org>
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1673662}
---

diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/ChromePaymentRequestService.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/ChromePaymentRequestService.java
index fbdc8cb..0dd24f9f 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/ChromePaymentRequestService.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/ChromePaymentRequestService.java
@@ -373,13 +373,13 @@
                     }
                     mSpcController = null;
                 };
-        PaymentItem rawTotal = mSpec.getRawTotal();
-        assert rawTotal != null;
         PaymentApp selectedPaymentApp = getSelectedPaymentApp();
         assert selectedPaymentApp != null;
         assert selectedPaymentApp.getLabel() != null;
         assert selectedPaymentApp.getDrawableIcon() != null;
 
+        PaymentItem total = selectedPaymentApp.getTotalForSpc();
+
         mSpcController =
                 new SecurePaymentConfirmationController(
                         windowAndroid,
@@ -388,7 +388,7 @@
                         getPayeeOrigin(spcMethodData.securePaymentConfirmation),
                         selectedPaymentApp.getLabel(),
                         selectedPaymentApp.getSublabel(),
-                        rawTotal,
+                        total,
                         selectedPaymentApp.getDrawableIcon(),
                         spcMethodData.securePaymentConfirmation.rpId,
                         spcMethodData.securePaymentConfirmation.showOptOut,
@@ -471,6 +471,8 @@
                         }
                         mSpcController = null;
                     };
+            PaymentItem total = selectedPaymentApp.getTotalForSpc();
+
             mSpcController =
                     new SecurePaymentConfirmationController(
                             windowAndroid,
@@ -479,7 +481,7 @@
                             getPayeeOrigin(spcMethodData.securePaymentConfirmation),
                             selectedPaymentApp.getLabel(),
                             selectedPaymentApp.getSublabel(),
-                            mSpec.getRawTotal(),
+                            total,
                             selectedPaymentApp.getDrawableIcon(),
                             spcMethodData.securePaymentConfirmation.rpId,
                             spcMethodData.securePaymentConfirmation.showOptOut,
diff --git a/components/payments/content/android/java/src/org/chromium/components/payments/JniPaymentApp.java b/components/payments/content/android/java/src/org/chromium/components/payments/JniPaymentApp.java
index 462b3f4c..fdb597ce 100644
--- a/components/payments/content/android/java/src/org/chromium/components/payments/JniPaymentApp.java
+++ b/components/payments/content/android/java/src/org/chromium/components/payments/JniPaymentApp.java
@@ -31,6 +31,7 @@
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 
 /** Wrapper around a C++ payment app. */
@@ -196,6 +197,13 @@
     }
 
     @Override
+    public PaymentItem getTotalForSpc() {
+        byte[] byteResult = JniPaymentAppJni.get().getTotalForSpc(mNativeObject);
+        Objects.requireNonNull(byteResult, "The secure payment app must provide the total.");
+        return PaymentItem.deserialize(ByteBuffer.wrap(byteResult));
+    }
+
+    @Override
     public void invokePaymentApp(
             String id,
             String merchantName,
@@ -309,6 +317,8 @@
 
         boolean canPreselect(long nativeJniPaymentApp);
 
+        byte[] getTotalForSpc(long nativeJniPaymentApp);
+
         void invokePaymentApp(long nativeJniPaymentApp, JniPaymentApp callback);
 
         void updateWith(long nativeJniPaymentApp, ByteBuffer responseByteBuffer);
diff --git a/components/payments/content/android/java/src/org/chromium/components/payments/PaymentApp.java b/components/payments/content/android/java/src/org/chromium/components/payments/PaymentApp.java
index 45cbc70e..fa70476 100644
--- a/components/payments/content/android/java/src/org/chromium/components/payments/PaymentApp.java
+++ b/components/payments/content/android/java/src/org/chromium/components/payments/PaymentApp.java
@@ -204,25 +204,24 @@
     /**
      * Invoke the payment app to retrieve the payment details.
      *
-     * The callback will be invoked with the resulting payment details or error.
+     * <p>The callback will be invoked with the resulting payment details or error.
      *
-     * @param id               The unique identifier of the PaymentRequest.
-     * @param merchantName     The name of the merchant.
-     * @param origin           The origin of this merchant.
-     * @param iframeOrigin     The origin of the iframe that invoked PaymentRequest.
+     * @param id The unique identifier of the PaymentRequest.
+     * @param merchantName The name of the merchant.
+     * @param origin The origin of this merchant.
+     * @param iframeOrigin The origin of the iframe that invoked PaymentRequest.
      * @param certificateChain The site certificate chain of the merchant. Can be null when
-     *                         ANDROID_PAYMENT_INTENTS_OMIT_DEPRECATED_PARAMETERS is enabled or for
-     *                         localhost or local file, which are secure contexts without SSL. Each
-     *                         byte array cannot be null.
-     * @param methodDataMap    The payment-method specific data for all applicable payment methods,
-     *                         e.g., whether the app should be invoked in test or production, a
-     *                         merchant identifier, or a public key.
-     * @param total            The total amount.
-     * @param displayItems     The shopping cart items.
-     * @param modifiers        The relevant payment details modifiers.
-     * @param paymentOptions   The payment options of the PaymentRequest.
-     * @param shippingOptions  The shipping options of the PaymentRequest.
-     * @param callback         The object that will receive the payment details.
+     *     ANDROID_PAYMENT_INTENTS_OMIT_DEPRECATED_PARAMETERS is enabled or for localhost or local
+     *     file, which are secure contexts without SSL. Each byte array cannot be null.
+     * @param methodDataMap The payment-method specific data for all applicable payment methods,
+     *     e.g., whether the app should be invoked in test or production, a merchant identifier, or
+     *     a public key.
+     * @param total The total amount.
+     * @param displayItems The shopping cart items.
+     * @param modifiers The relevant payment details modifiers.
+     * @param paymentOptions The payment options of the PaymentRequest.
+     * @param shippingOptions The shipping options of the PaymentRequest.
+     * @param callback The object that will receive the payment details.
      */
     public void invokePaymentApp(
             String id,
@@ -333,6 +332,14 @@
     }
 
     /**
+     * @return The total amount for this payment app. Must only be called for the SPC payment app.
+     */
+    public PaymentItem getTotalForSpc() {
+        throw new IllegalStateException(
+                "getTotalForSpc() must only be called for secure payment app.");
+    }
+
+    /**
      * @return The payment entities logos, an unmodifiable {@link List} (Secure Payment Confirmation
      *     specific).
      */
diff --git a/components/payments/content/android/java/src/org/chromium/components/payments/PaymentRequestService.java b/components/payments/content/android/java/src/org/chromium/components/payments/PaymentRequestService.java
index cd3b47c4..5077bfd 100644
--- a/components/payments/content/android/java/src/org/chromium/components/payments/PaymentRequestService.java
+++ b/components/payments/content/android/java/src/org/chromium/components/payments/PaymentRequestService.java
@@ -986,7 +986,8 @@
             if (mBrowserPaymentRequest.showNoMatchingPaymentCredential()) {
                 if (sNativeObserverForTest != null) {
                     sNativeObserverForTest.onAppListReady(
-                            mBrowserPaymentRequest.getPaymentApps(), mSpec.getRawTotal());
+                            mBrowserPaymentRequest.getPaymentApps(),
+                            mBrowserPaymentRequest.getPaymentApps().get(0).getTotalForSpc());
                 }
                 return null;
             }
diff --git a/components/payments/content/android/jni_payment_app.cc b/components/payments/content/android/jni_payment_app.cc
index 1e79ea5..d0b2d52 100644
--- a/components/payments/content/android/jni_payment_app.cc
+++ b/components/payments/content/android/jni_payment_app.cc
@@ -114,6 +114,12 @@
   return payment_app_->CanPreselect();
 }
 
+ScopedJavaLocalRef<jbyteArray> JniPaymentApp::GetTotalForSpc(JNIEnv* env) {

diff --git a/components/payments/content/secure_payment_confirmation_app_factory_unittest.cc b/components/payments/content/secure_payment_confirmation_app_factory_unittest.cc
index e2d70056..4837b816 100644
--- a/components/payments/content/secure_payment_confirmation_app_factory_unittest.cc
+++ b/components/payments/content/secure_payment_confirmation_app_factory_unittest.cc
@@ -9,6 +9,7 @@
 
 #include "base/base64.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/run_loop.h"
 #include "base/test/gmock_callback_support.h"
 #include "base/test/gmock_move_support.h"
 #include "base/test/metrics/histogram_tester.h"
@@ -46,10 +47,12 @@
 namespace {
 
 using ::base::test::RunOnceCallback;
+using ::base::test::RunOnceClosure;
 using ::testing::_;
 using ::testing::ByMove;
 using ::testing::Eq;
 using ::testing::Field;
+using ::testing::Invoke;
 using ::testing::IsEmpty;
 using ::testing::IsNull;
 using ::testing::Matcher;
@@ -101,6 +104,7 @@
 
     mock_authenticator_ = CreateMockInternalAuthenticator();
     mock_service_ = base::MakeRefCounted<MockWebPaymentsWebDataService>();
+    done_creating_apps_loop_ = std::make_unique<base::RunLoop>();
   }
 
   std::unique_ptr<MockPaymentAppFactoryDelegate> CreateMockDelegate(
@@ -116,6 +120,9 @@
         .WillByDefault(testing::Return(
             mock_content_payment_request_delegate_.GetContentWeakPtr()));
 #endif
+    ON_CALL(*mock_delegate, OnDoneCreatingPaymentApps())
+        .WillByDefault(Invoke(this, &SecurePaymentConfirmationAppFactoryTest::
+                                        OnDoneCreatingPaymentApps));
 
     return mock_delegate;
   }
@@ -131,6 +138,13 @@
     return mock_authenticator;
   }
 
+  void WaitForDoneCreatingApps() {
+    done_creating_apps_loop_->Run();
+    done_creating_apps_loop_ = std::make_unique<base::RunLoop>();
+  }
+
+  void OnDoneCreatingPaymentApps() { done_creating_apps_loop_->Quit(); }
+
   // Creates and returns a minimal SecurePaymentConfirmationRequest object with
   // only required fields filled in to pass parsing.
   //
@@ -187,6 +201,7 @@
       mock_credential_finder_;
   scoped_refptr<FakeBrowserBoundKeyStore> browser_bound_key_store_ =
       base::MakeRefCounted<FakeBrowserBoundKeyStore>();
+  std::unique_ptr<base::RunLoop> done_creating_apps_loop_;
 
  private:
   crypto::ScopedFakeUnexportableKeyProvider scoped_key_provider_;
@@ -241,7 +256,7 @@
   static_cast<content::TestWebContents*>(web_contents_.get())
       ->TestDidDownloadImage(icon, /*http_status_code=*/200,
                              std::move(icon_bitmaps), std::move(icon_sizes));
-
+  WaitForDoneCreatingApps();
   histogram_tester.ExpectUniqueSample(
       "PaymentRequest.SecurePaymentConfirmation."
       "UserVerifyingPlatformAuthenticatorAvailable",
@@ -314,6 +329,8 @@
       ->TestDidDownloadImage(icon, /*http_status_code=*/200,
                              std::move(icon_bitmaps), std::move(icon_sizes));
 
+  WaitForDoneCreatingApps();
+
   ASSERT_TRUE(secure_payment_confirmation_app);
   EXPECT_EQ(secure_payment_confirmation_app->GetSublabel(),
             u"instrument details");
@@ -355,6 +372,8 @@
   static_cast<content::TestWebContents*>(web_contents_.get())
       ->TestDidDownloadImage(icon, /*http_status_code=*/200,
                              std::move(icon_bitmaps), std::move(icon_sizes));
+
+  WaitForDoneCreatingApps();
 }
 
 // Test that SecurePaymentConfirmationAppFactory passes the input credentials,
@@ -427,6 +446,7 @@
       ->TestDidDownloadImage(icon, /*http_status_code=*/200,
                              std::move(icon_bitmaps), std::move(icon_sizes));
 
+  WaitForDoneCreatingApps();
   ASSERT_TRUE(secure_payment_confirmation_app);
   PasskeyBrowserBinder* passkey_browser_binder =
       static_cast<SecurePaymentConfirmationApp*>(
@@ -453,22 +473,15 @@
       CreateSecurePaymentConfirmationRequest();
   GURL icon = method_data->secure_payment_confirmation->instrument->icon;
 
-  std::unique_ptr<webauthn::MockInternalAuthenticator> mock_authenticator =
-      CreateMockInternalAuthenticator(
-          {.is_user_verifying_platform_authenticator_available = false});
+  mock_authenticator_ = CreateMockInternalAuthenticator(
+      {.is_user_verifying_platform_authenticator_available = false});
 
-  auto mock_delegate = std::make_unique<MockPaymentAppFactoryDelegate>(
-      web_contents_, std::move(method_data));
+  std::unique_ptr<MockPaymentAppFactoryDelegate> mock_delegate =
+      CreateMockDelegate(std::move(method_data));
   url::Origin caller_origin = url::Origin::Create(GURL("https://site.example"));
   EXPECT_CALL(*mock_delegate, GetFrameSecurityOrigin())
       .WillRepeatedly(ReturnRef(caller_origin));
 
-  scoped_refptr<MockWebPaymentsWebDataService> mock_service =
-      base::MakeRefCounted<MockWebPaymentsWebDataService>();
-  EXPECT_CALL(*mock_delegate, CreateInternalAuthenticator())
-      .WillOnce(Return(ByMove(std::move(mock_authenticator))));
-  EXPECT_CALL(*mock_delegate, GetWebPaymentsWebDataService())
-      .WillRepeatedly(Return(mock_service));
   std::unique_ptr<PaymentApp> secure_payment_confirmation_app;
   EXPECT_CALL(*mock_delegate, OnPaymentAppCreated(_))
       .WillOnce(MoveArg<0>(&secure_payment_confirmation_app));
@@ -483,6 +496,8 @@
       ->TestDidDownloadImage(icon, /*http_status_code=*/200,
                              std::move(icon_bitmaps), std::move(icon_sizes));
 
+  WaitForDoneCreatingApps();
+
   ASSERT_TRUE(secure_payment_confirmation_app);
   EXPECT_FALSE(secure_payment_confirmation_app->HasEnrolledInstrument());
 
@@ -530,7 +545,7 @@
   static_cast<content::TestWebContents*>(web_contents_.get())
       ->TestDidDownloadImage(icon, /*http_status_code=*/200,
                              std::move(icon_bitmaps), std::move(icon_sizes));
-
+  WaitForDoneCreatingApps();
   ASSERT_TRUE(secure_payment_confirmation_app);
   EXPECT_FALSE(secure_payment_confirmation_app->HasEnrolledInstrument());
 
@@ -584,6 +599,7 @@
   static_cast<content::TestWebContents*>(web_contents_.get())
       ->TestDidDownloadImage(icon, /*http_status_code=*/200,
                              std::move(icon_bitmaps), std::move(icon_sizes));
+  WaitForDoneCreatingApps();
 }
 
 // Class wrapping tests relating to payment entity logos support in
@@ -675,6 +691,8 @@
   FakeImageDownloaded(kPaymentEntity2LogoUrl, /*succeeded=*/true,
                       /*height=*/60);
 
+  WaitForDoneCreatingApps();
+
   // Even though the third and fourth entity logos were not downloaded (and were
   // not attempted to be downloaded), the first two should be sufficient and the
   // payment app should be created.
diff --git a/components/payments/content/secure_payment_confirmation_app_unittest.cc b/components/payments/content/secure_payment_confirmation_app_unittest.cc
index b31cf75e..bc49282 100644
--- a/components/payments/content/secure_payment_confirmation_app_unittest.cc
+++ b/components/payments/content/secure_payment_confirmation_app_unittest.cc
@@ -162,10 +162,11 @@
       std::move(credential_id),
       /*passkey_browser_binder=*/nullptr,
       /*device_supports_browser_bound_keys_in_hardware=*/true,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
-      MakeRequest(), std::move(authenticator),
+      url::Origin::Create(GURL("https://merchant.example")), MakeRequest(),
+      std::move(authenticator),
       /*payment_entities_logos=*/{},
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
 
   std::vector<uint8_t> expected_bytes =
       std::vector<uint8_t>(challenge_bytes_.begin(), challenge_bytes_.end());
@@ -216,10 +217,11 @@
       std::move(credential_id),
       /*passkey_browser_binder=*/nullptr,
       /*device_supports_browser_bound_keys_in_hardware=*/false,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
-      MakeRequest(), std::move(authenticator),
+      url::Origin::Create(GURL("https://merchant.example")), MakeRequest(),
+      std::move(authenticator),
       /*payment_entities_logos=*/{},
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
 
   ASSERT_NE(nullptr, app.authenticator_for_testing());
 
@@ -406,10 +408,11 @@
       /*payment_instrument_icon=*/std::make_unique<SkBitmap>(), credential_id,
       std::move(binder),
       GetParam().device_supports_browser_bound_keys_in_hardware,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
+      url::Origin::Create(GURL("https://merchant.example")),
       MakeRequest(GetParam().credential_parameters), std::move(authenticator),
       /*payment_entities_logos=*/{},
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
   app.SetWaitForGetBrowserBoundKeyForTesting(run_loop.QuitClosure());
   browser_bound_key_store_->PutFakeKey(FakeBrowserBoundKey(
       browser_bound_key_id, public_key_as_cose_key, signature,
@@ -510,10 +513,11 @@
       /*credential_id=*/std::vector<uint8_t>(),
       /*passkey_browser_binder=*/nullptr,
       /*device_supports_browser_bound_keys_in_hardware=*/false,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
-      MakeRequest(), /*authenticator=*/nullptr,
+      url::Origin::Create(GURL("https://merchant.example")), MakeRequest(),
+      /*authenticator=*/nullptr,
       /*payment_entities_logos=*/{},
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
 
   EXPECT_FALSE(app.HasEnrolledInstrument());
   EXPECT_EQ(app.GetId(), "spc");
@@ -531,11 +535,11 @@
       /*payment_instrument_icon=*/std::make_unique<SkBitmap>(), credential_id,
       /*passkey_browser_binder=*/nullptr,
       /*device_supports_browser_bound_keys_in_hardware=*/false,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
-      MakeRequest(),
+      url::Origin::Create(GURL("https://merchant.example")), MakeRequest(),
       std::make_unique<webauthn::MockInternalAuthenticator>(web_contents_),
       /*payment_entities_logos=*/{},
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
 
   EXPECT_TRUE(app.HasEnrolledInstrument());
   EXPECT_EQ(app.GetId(), base::Base64Encode(credential_id));
@@ -567,9 +571,10 @@
       std::move(credential_id),
       /*passkey_browser_binder=*/nullptr,
       /*device_supports_browser_bound_keys_in_hardware=*/false,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
-      std::move(request), std::move(authenticator), std::move(logos),
+      url::Origin::Create(GURL("https://merchant.example")), std::move(request),
+      std::move(authenticator), std::move(logos),
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
 
   blink::mojom::PaymentOptionsPtr payment_options;
   EXPECT_CALL(*mock_authenticator, SetPaymentOptions)
@@ -617,9 +622,10 @@
       std::move(credential_id),
       /*passkey_browser_binder=*/nullptr,
       /*device_supports_browser_bound_keys_in_hardware=*/false,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
-      MakeRequest(), std::move(authenticator), std::move(logos),
+      url::Origin::Create(GURL("https://merchant.example")), MakeRequest(),
+      std::move(authenticator), std::move(logos),
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
 
   blink::mojom::PaymentOptionsPtr payment_options;
   EXPECT_CALL(*mock_authenticator, SetPaymentOptions)
@@ -656,10 +662,11 @@
       std::move(credential_id),
       /*passkey_browser_binder=*/nullptr,
       /*device_supports_browser_bound_keys_in_hardware=*/false,
-      url::Origin::Create(GURL("https://merchant.example")), spec_->AsWeakPtr(),
-      MakeRequest(), std::move(authenticator),
+      url::Origin::Create(GURL("https://merchant.example")), MakeRequest(),
+      std::move(authenticator),
       /*payment_entities_logos=*/{},
       /*is_error_dialog=*/false);
+  app.SetTotal(spec_->GetTotal(&app).Clone());
 
   EXPECT_CALL(*mock_authenticator, GetAssertion(_, _))
       .WillOnce(RunOnceCallback<1>(