[Autofill Auth] Changing OnCVCAuthenticationComplete() to take in struct.

Changing CreditCardAccessManager::OnCVCAuthenticationComplete() to take in a
struct. Eventually will need to include an authorization token as part of the
response, so changing this to a struct will keep things clean.

Bug: 949269
Change-Id: I682c6abfe474427c36b035e3416168f08aed2c58
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761368Reviewed-by: Jared Saul <jsaul@google.com>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Commit-Queue: Manas Verma <manasverma@google.com>
Cr-Commit-Position: refs/heads/master@{#692964}

components/autofill/core/browser/payments/credit_card_access_manager.cc

@@ -294,23 +294,25 @@ CreditCardAccessManager::GetOrCreateFIDOAuthenticator() {
 #endif
 
 void CreditCardAccessManager::OnCVCAuthenticationComplete(
-    bool did_succeed,
-    const CreditCard* card,
-    const base::string16& cvc,
-    base::Value creation_options) {
+    const CreditCardCVCAuthenticator::CVCAuthenticationResponse& response) {
   is_authentication_in_progress_ = false;
-  accessor_->OnCreditCardFetched(did_succeed, card, cvc);
+  accessor_->OnCreditCardFetched(response.did_succeed, response.card,
+                                 response.cvc);
 
-  if (!did_succeed)
+  if (!response.did_succeed)
     return;
 
 #if defined(OS_ANDROID)
   // Now that unmask flow is complete, on Android, if GetRealPan includes
   // |creation_options|, completely hand over registration flow to
   // CreditCardFIDOAuthenticator.
-  if (creation_options.is_dict())
-    GetOrCreateFIDOAuthenticator()->Register(std::move(creation_options));
-#elif !defined(OS_IOS)  // CreditCardFIDOAuthenticator does not exist on iOS.
+  if (response.creation_options.has_value()) {
+    DCHECK(response.creation_options->is_dict());
+    GetOrCreateFIDOAuthenticator()->Register(
+        response.creation_options->Clone());
+  }
+#elif !defined(OS_IOS)
+  // CreditCardFIDOAuthenticator does not exist on iOS.
   // On desktop, prompts dialog to show the authentication offer.
   if (unmask_details_.offer_fido_opt_in)
     GetOrCreateFIDOAuthenticator()->ShowWebauthnOfferDialog();

components/autofill/core/browser/payments/credit_card_access_manager.h

@@ -135,10 +135,8 @@ class CreditCardAccessManager : public CreditCardCVCAuthenticator::Requester,
 
   // CreditCardCVCAuthenticator::Requester:
   void OnCVCAuthenticationComplete(
-      bool did_succeed,
-      const CreditCard* card = nullptr,
-      const base::string16& cvc = base::string16(),
-      base::Value creation_options = base::Value()) override;
+      const CreditCardCVCAuthenticator::CVCAuthenticationResponse& response)
+      override;
 
 #if !defined(OS_IOS)
   // CreditCardFIDOAuthenticator::Requester:

components/autofill/core/browser/payments/credit_card_access_manager_unittest.cc

@@ -230,10 +230,10 @@ class CreditCardAccessManagerTest : public testing::Test {
     if (fido_opt_in) {
       response.fido_creation_options =
           base::Value(base::Value::Type::DICTIONARY);
-      response.fido_creation_options.SetKey("relying_party_id",
-                                            base::Value(kGooglePaymentsRpid));
-      response.fido_creation_options.SetKey("challenge",
-                                            base::Value(kTestChallenge));
+      response.fido_creation_options->SetKey("relying_party_id",
+                                             base::Value(kGooglePaymentsRpid));
+      response.fido_creation_options->SetKey("challenge",
+                                             base::Value(kTestChallenge));
     }
 #endif
     full_card_request->OnDidGetRealPan(result,

components/autofill/core/browser/payments/credit_card_cvc_authenticator.cc

@@ -11,6 +11,11 @@
 
 namespace autofill {
 
+CreditCardCVCAuthenticator::CVCAuthenticationResponse::
+    CVCAuthenticationResponse() {}
+CreditCardCVCAuthenticator::CVCAuthenticationResponse::
+    ~CVCAuthenticationResponse() {}
+
 CreditCardCVCAuthenticator::CreditCardCVCAuthenticator(AutofillClient* client)
     : client_(client) {}
 
@@ -37,12 +42,18 @@ void CreditCardCVCAuthenticator::OnFullCardRequestSucceeded(
     const CreditCard& card,
     const base::string16& cvc) {
   requester_->OnCVCAuthenticationComplete(
-      /*did_succeed=*/true, &card, cvc,
-      full_card_request.GetFIDOCreationOptions());
+      CVCAuthenticationResponse()
+          .with_did_succeed(true)
+          .with_card(&card)
+          .with_cvc(cvc)
+          .with_creation_options(
+              std::move(full_card_request.unmask_response_details()
+                            .fido_creation_options)));
 }
 
 void CreditCardCVCAuthenticator::OnFullCardRequestFailed() {
-  requester_->OnCVCAuthenticationComplete(/*did_succeed=*/false);
+  requester_->OnCVCAuthenticationComplete(
+      CVCAuthenticationResponse().with_did_succeed(false));
 }
 
 void CreditCardCVCAuthenticator::ShowUnmaskPrompt(

components/autofill/core/browser/payments/credit_card_cvc_authenticator.h

@@ -20,14 +20,38 @@ class CreditCardCVCAuthenticator
     : public payments::FullCardRequest::ResultDelegate,
       public payments::FullCardRequest::UIDelegate {
  public:
+  struct CVCAuthenticationResponse {
+    CVCAuthenticationResponse();
+    ~CVCAuthenticationResponse();
+
+    CVCAuthenticationResponse& with_did_succeed(bool b) {
+      did_succeed = b;
+      return *this;
+    }
+    // Data pointed to by |c| must outlive this object.
+    CVCAuthenticationResponse& with_card(const CreditCard* c) {
+      card = c;
+      return *this;
+    }
+    CVCAuthenticationResponse& with_cvc(const base::string16 s) {
+      cvc = base::string16(s);
+      return *this;
+    }
+    CVCAuthenticationResponse& with_creation_options(
+        base::Optional<base::Value> v) {
+      creation_options = std::move(v);
+      return *this;
+    }
+    bool did_succeed = false;
+    const CreditCard* card = nullptr;
+    base::string16 cvc = base::string16();
+    base::Optional<base::Value> creation_options = base::nullopt;
+  };
   class Requester {
    public:
     virtual ~Requester() {}
     virtual void OnCVCAuthenticationComplete(
-        bool did_succeed,
-        const CreditCard* card = nullptr,
-        const base::string16& cvc = base::string16(),
-        base::Value creation_options = base::Value()) = 0;
+        const CVCAuthenticationResponse& response) = 0;
   };
   explicit CreditCardCVCAuthenticator(AutofillClient* client);
   ~CreditCardCVCAuthenticator() override;

components/autofill/core/browser/payments/full_card_request.cc

@@ -200,11 +200,10 @@ void FullCardRequest::OnDidGetRealPan(
         personal_data_manager_->UpdateServerCreditCard(request_->card);
 
       // TODO(crbug/949269): Once |fido_opt_in| is added to
-      // UserProvidedUnmaskDetails, add a check here that
-      // |user_response.fido_opt_in| is true before copying over
-      // |creation_options|.
-      if (response_details.fido_creation_options.is_dict())
-        fido_creation_options_ = response_details.fido_creation_options.Clone();
+      // UserProvidedUnmaskDetails, clear out |creation_options| from
+      // |response_details_| if |user_response.fido_opt_in| was not set to true
+      // to avoid an unwanted registration prompt.
+      unmask_response_details_ = response_details;
       if (result_delegate_)
         result_delegate_->OnFullCardRequestSucceeded(
             *this, request_->card, request_->user_response.cvc);
@@ -218,10 +217,6 @@ void FullCardRequest::OnDidGetRealPan(
   }
 }
 
-base::Value FullCardRequest::GetFIDOCreationOptions() const {
-  return fido_creation_options_.Clone();
-}
-
 void FullCardRequest::Reset() {
   weak_ptr_factory_.InvalidateWeakPtrs();
   payments_client_->CancelRequest();
@@ -229,7 +224,7 @@ void FullCardRequest::Reset() {
   ui_delegate_ = nullptr;
   request_.reset();
   should_unmask_card_ = false;
-  fido_creation_options_ = base::Value();
+  unmask_response_details_ = payments::PaymentsClient::UnmaskResponseDetails();
 }
 
 }  // namespace payments

components/autofill/core/browser/payments/full_card_request.h

@@ -93,8 +93,10 @@ class FullCardRequest final : public CardUnmaskDelegate {
       AutofillClient::PaymentsRpcResult result,
       payments::PaymentsClient::UnmaskResponseDetails& response_details);
 
-  // Returns a copy of |fido_creation_options_|.
-  base::Value GetFIDOCreationOptions() const;
+  payments::PaymentsClient::UnmaskResponseDetails unmask_response_details()
+      const {
+    return unmask_response_details_;
+  }
 
   base::TimeTicks form_parsed_timestamp() const {
     return form_parsed_timestamp_;
@@ -161,9 +163,8 @@ class FullCardRequest final : public CardUnmaskDelegate {
   // The timestamp when the form is parsed. For histograms.
   base::TimeTicks form_parsed_timestamp_;
 
-  // Includes a challenge for enrolling user into FIDO Authentication for card
-  // unmasking.
-  base::Value fido_creation_options_;
+  // Includes all details from GetRealPan response.
+  payments::PaymentsClient::UnmaskResponseDetails unmask_response_details_;
 
   // Enables destroying FullCardRequest while CVC prompt is showing or a server
   // communication is pending.

components/autofill/core/browser/payments/payments_client.cc

@@ -957,10 +957,19 @@ PaymentsClient::UnmaskRequestDetails::~UnmaskRequestDetails() {}
 PaymentsClient::UnmaskResponseDetails::UnmaskResponseDetails() {}
 PaymentsClient::UnmaskResponseDetails::UnmaskResponseDetails(
     const UnmaskResponseDetails& other) {
-  real_pan = other.real_pan;
-  fido_creation_options = other.fido_creation_options.Clone();
+  *this = other;
 }
 PaymentsClient::UnmaskResponseDetails::~UnmaskResponseDetails() {}
+PaymentsClient::UnmaskResponseDetails& PaymentsClient::UnmaskResponseDetails::
+operator=(const PaymentsClient::UnmaskResponseDetails& other) {
+  real_pan = other.real_pan;
+  if (other.fido_creation_options.has_value()) {
+    fido_creation_options = other.fido_creation_options->Clone();
+  } else {
+    fido_creation_options.reset();
+  }
+  return *this;
+}
 
 PaymentsClient::OptChangeRequestDetails::OptChangeRequestDetails() {}
 PaymentsClient::OptChangeRequestDetails::OptChangeRequestDetails(

components/autofill/core/browser/payments/payments_client.h

@@ -99,6 +99,7 @@ class PaymentsClient {
     UnmaskResponseDetails();
     UnmaskResponseDetails(const UnmaskResponseDetails& other);
     ~UnmaskResponseDetails();
+    UnmaskResponseDetails& operator=(const UnmaskResponseDetails& other);
 
     UnmaskResponseDetails& with_real_pan(std::string r) {
       real_pan = r;
@@ -106,7 +107,7 @@ class PaymentsClient {
     }
 
     std::string real_pan;
-    base::Value fido_creation_options;
+    base::Optional<base::Value> fido_creation_options;
   };
 
   // Information required to either opt-in or opt-out a user for FIDO

components/autofill/core/browser/payments/payments_client_unittest.cc

@@ -434,7 +434,7 @@ TEST_F(PaymentsClientTest, UnmaskSuccessViaCVCWithCreationOptions) {
   EXPECT_EQ(AutofillClient::SUCCESS, result_);
   EXPECT_EQ("1234", unmask_response_details_->real_pan);
   EXPECT_EQ("google.com",
-            *unmask_response_details_->fido_creation_options.FindStringKey(
+            *unmask_response_details_->fido_creation_options->FindStringKey(
                 "relying_party_id"));
 }
 

components/autofill/core/browser/payments/test_authentication_requester.cc

@@ -19,14 +19,11 @@ TestAuthenticationRequester::GetWeakPtr() {
 }
 
 void TestAuthenticationRequester::OnCVCAuthenticationComplete(
-    bool did_succeed,
-    const CreditCard* card,
-    const base::string16& cvc,
-    base::Value creation_options) {
-  did_succeed_ = did_succeed;
+    const CreditCardCVCAuthenticator::CVCAuthenticationResponse& response) {
+  did_succeed_ = response.did_succeed;
   if (did_succeed_) {
-    DCHECK(card);
-    number_ = card->number();
+    DCHECK(response.card);
+    number_ = response.card->number();
   }
 }
 

components/autofill/core/browser/payments/test_authentication_requester.h

@@ -37,10 +37,8 @@ class TestAuthenticationRequester
 
   // CreditCardCVCAuthenticator::Requester:
   void OnCVCAuthenticationComplete(
-      bool did_succeed,
-      const CreditCard* card = nullptr,
-      const base::string16& cvc = base::string16(),
-      base::Value creation_options = base::Value()) override;
+      const CreditCardCVCAuthenticator::CVCAuthenticationResponse& response)
+      override;
 
 #if !defined(OS_IOS)
   // CreditCardFIDOAuthenticator::Requester: