AutofillSaveCardInfoBar on mobile should not appear when legal messages don't parse correctly

BUG=734219

Review-Url: https://codereview.chromium.org/2945563003
Cr-Commit-Position: refs/heads/master@{#480980}

chrome/browser/autofill/autofill_save_card_infobar_delegate_mobile_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <memory>
 
+#include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/test/histogram_tester.h"
 #include "chrome/browser/autofill/personal_data_manager_factory.h"
@@ -60,6 +61,11 @@ class AutofillSaveCardInfoBarDelegateMobileTest
       bool is_uploading,
       prefs::PreviousSaveCreditCardPromptUserDecision
           previous_save_credit_card_prompt_user_decision);
+  std::unique_ptr<ConfirmInfoBarDelegate> CreateDelegateWithLegalMessage(
+      bool is_uploading,
+      std::string legal_message_string,
+      prefs::PreviousSaveCreditCardPromptUserDecision
+          previous_save_credit_card_prompt_user_decision);
 
   std::unique_ptr<TestPersonalDataManager> personal_data_;
 
@@ -101,9 +107,26 @@ AutofillSaveCardInfoBarDelegateMobileTest::CreateDelegate(
     bool is_uploading,
     prefs::PreviousSaveCreditCardPromptUserDecision
         previous_save_credit_card_prompt_user_decision) {
-  base::HistogramTester histogram_tester;
+  return CreateDelegateWithLegalMessage(
+      is_uploading, "", previous_save_credit_card_prompt_user_decision);
+}
+
+std::unique_ptr<ConfirmInfoBarDelegate>
+AutofillSaveCardInfoBarDelegateMobileTest::CreateDelegateWithLegalMessage(
+    bool is_uploading,
+    std::string legal_message_string,
+    prefs::PreviousSaveCreditCardPromptUserDecision
+        previous_save_credit_card_prompt_user_decision) {
   CreditCard credit_card;
   std::unique_ptr<base::DictionaryValue> legal_message;
+  if (!legal_message_string.empty()) {
+    std::unique_ptr<base::Value> value(
+        base::JSONReader::Read(legal_message_string));
+    EXPECT_TRUE(value);
+    base::DictionaryValue* dictionary;
+    EXPECT_TRUE(value->GetAsDictionary(&dictionary));
+    legal_message = dictionary->CreateDeepCopy();
+  }
   std::unique_ptr<ConfirmInfoBarDelegate> delegate(
       new AutofillSaveCardInfoBarDelegateMobile(
           is_uploading, credit_card, std::move(legal_message),
@@ -125,9 +148,6 @@ AutofillSaveCardInfoBarDelegateMobileTest::CreateDelegate(
                 prefs::PREVIOUS_SAVE_CREDIT_CARD_PROMPT_USER_DECISION_NONE);
       break;
   }
-  histogram_tester.ExpectUniqueSample(
-      "Autofill.CreditCardInfoBar" + destination + previous_response,
-      AutofillMetrics::INFOBAR_SHOWN, 1);
   return delegate;
 }
 
@@ -135,17 +155,29 @@ AutofillSaveCardInfoBarDelegateMobileTest::CreateDelegate(
 TEST_F(AutofillSaveCardInfoBarDelegateMobileTest, Metrics_Local) {
   ::testing::InSequence dummy;
 
-  // Accept the infobar.
+  // Infobar is shown.
   {
+    base::HistogramTester histogram_tester;
     std::unique_ptr<ConfirmInfoBarDelegate> infobar(CreateDelegate(
         /* is_uploading= */ false,
         prefs::PREVIOUS_SAVE_CREDIT_CARD_PROMPT_USER_DECISION_NONE));
+
+    histogram_tester.ExpectUniqueSample("Autofill.CreditCardInfoBar.Local",
+                                        AutofillMetrics::INFOBAR_SHOWN, 1);
+  }
+
+  // Accept the infobar.
+  {
+    std::unique_ptr<ConfirmInfoBarDelegate> infobar(CreateDelegate(
+        /* is_uploading= */ false,
+        prefs::PREVIOUS_SAVE_CREDIT_CARD_PROMPT_USER_DECISION_DENIED));
     EXPECT_CALL(*personal_data_, SaveImportedCreditCard(_));
 
     base::HistogramTester histogram_tester;
     EXPECT_TRUE(infobar->Accept());
-    histogram_tester.ExpectUniqueSample("Autofill.CreditCardInfoBar.Local",
-                                        AutofillMetrics::INFOBAR_ACCEPTED, 1);
+    histogram_tester.ExpectUniqueSample(
+        "Autofill.CreditCardInfoBar.Local.PreviouslyDenied",
+        AutofillMetrics::INFOBAR_ACCEPTED, 1);
   }
 
   // Cancel the infobar.
@@ -192,6 +224,59 @@ TEST_F(AutofillSaveCardInfoBarDelegateMobileTest, Metrics_Local) {
 TEST_F(AutofillSaveCardInfoBarDelegateMobileTest, Metrics_Server) {
   ::testing::InSequence dummy;
 
+  // Infobar is shown.
+  {
+    base::HistogramTester histogram_tester;
+    std::unique_ptr<ConfirmInfoBarDelegate> infobar(CreateDelegate(
+        /* is_uploading= */ true,
+        prefs::PREVIOUS_SAVE_CREDIT_CARD_PROMPT_USER_DECISION_NONE));
+
+    histogram_tester.ExpectUniqueSample("Autofill.CreditCardInfoBar.Server",
+                                        AutofillMetrics::INFOBAR_SHOWN, 1);
+  }
+
+  // Infobar is still shown when the legal message is successfully parsed.
+  {
+    base::HistogramTester histogram_tester;
+    std::string good_legal_message =
+        "{"
+        "  \"line\" : [ {"
+        "     \"template\": \"This is the entire message.\""
+        "  } ]"
+        "}";
+    std::unique_ptr<ConfirmInfoBarDelegate> infobar(
+        CreateDelegateWithLegalMessage(
+            /* is_uploading= */ true, std::move(good_legal_message),
+            prefs::PREVIOUS_SAVE_CREDIT_CARD_PROMPT_USER_DECISION_DENIED));
+
+    histogram_tester.ExpectUniqueSample(
+        "Autofill.CreditCardInfoBar.Server.PreviouslyDenied",
+        AutofillMetrics::INFOBAR_SHOWN, 1);
+  }
+
+  // Infobar is not shown because the provided legal message is invalid.
+  {
+    base::HistogramTester histogram_tester;
+    // Legal message is invalid because it's missing the url.
+    std::string bad_legal_message =
+        "{"
+        "  \"line\" : [ {"
+        "     \"template\": \"Panda {0}.\","
+        "     \"template_parameter\": [ {"
+        "        \"display_text\": \"bear\""
+        "     } ]"
+        "  } ]"
+        "}";
+    std::unique_ptr<ConfirmInfoBarDelegate> infobar(
+        CreateDelegateWithLegalMessage(
+            /* is_uploading= */ true, std::move(bad_legal_message),
+            prefs::PREVIOUS_SAVE_CREDIT_CARD_PROMPT_USER_DECISION_DENIED));
+
+    histogram_tester.ExpectUniqueSample(
+        "Autofill.CreditCardInfoBar.Server.PreviouslyDenied",
+        AutofillMetrics::INFOBAR_NOT_SHOWN_INVALID_LEGAL_MESSAGE, 1);
+  }
+
   // Accept the infobar.
   {
     std::unique_ptr<ConfirmInfoBarDelegate> infobar(CreateDelegate(
@@ -201,8 +286,9 @@ TEST_F(AutofillSaveCardInfoBarDelegateMobileTest, Metrics_Server) {
 
     base::HistogramTester histogram_tester;
     EXPECT_TRUE(infobar->Accept());
-    histogram_tester.ExpectUniqueSample("Autofill.CreditCardInfoBar.Server",
-                                        AutofillMetrics::INFOBAR_ACCEPTED, 1);
+    histogram_tester.ExpectUniqueSample(
+        "Autofill.CreditCardInfoBar.Server.PreviouslyDenied",
+        AutofillMetrics::INFOBAR_ACCEPTED, 1);
   }
 
   // Cancel the infobar.

chrome/browser/ui/autofill/chrome_autofill_client.cc

@@ -206,10 +206,15 @@ void ChromeAutofillClient::ConfirmSaveCreditCardToCloud(
     bool should_cvc_be_requested,
     const base::Closure& callback) {
 #if defined(OS_ANDROID)
-  InfoBarService::FromWebContents(web_contents())
-      ->AddInfoBar(CreateSaveCardInfoBarMobile(
+  std::unique_ptr<AutofillSaveCardInfoBarDelegateMobile>
+      save_card_info_bar_delegate_mobile =
           base::MakeUnique<AutofillSaveCardInfoBarDelegateMobile>(
-              true, card, std::move(legal_message), callback, GetPrefs())));
+              true, card, std::move(legal_message), callback, GetPrefs());
+  if (save_card_info_bar_delegate_mobile->LegalMessagesParsedSuccessfully()) {
+    InfoBarService::FromWebContents(web_contents())
+        ->AddInfoBar(CreateSaveCardInfoBarMobile(
+            std::move(save_card_info_bar_delegate_mobile)));
+  }
 #else
   // Do lazy initialization of SaveCardBubbleControllerImpl.
   autofill::SaveCardBubbleControllerImpl::CreateForWebContents(web_contents());

components/autofill/core/browser/autofill_metrics.h

@@ -283,6 +283,9 @@ class AutofillMetrics {
     INFOBAR_DENIED,     // The user explicitly denied the infobar.
     INFOBAR_IGNORED,    // The user completely ignored the infobar (logged on
                         // tab close).
+    INFOBAR_NOT_SHOWN_INVALID_LEGAL_MESSAGE,  // We didn't show the infobar
+                                              // because the provided legal
+                                              // message was invalid.
     NUM_INFO_BAR_METRICS,
   };
 

components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.cc

@@ -43,8 +43,15 @@ AutofillSaveCardInfoBarDelegateMobile::AutofillSaveCardInfoBarDelegateMobile(
 #endif
       card_label_(base::string16(kMidlineEllipsis) + card.LastFourDigits()),
       card_sub_label_(card.AbbreviatedExpirationDateForDisplay()) {
-  if (legal_message)
-    LegalMessageLine::Parse(*legal_message, &legal_messages_);
+  if (legal_message) {
+    if (!LegalMessageLine::Parse(*legal_message, &legal_messages_)) {
+      AutofillMetrics::LogCreditCardInfoBarMetric(
+          AutofillMetrics::INFOBAR_NOT_SHOWN_INVALID_LEGAL_MESSAGE, upload_,
+          pref_service_->GetInteger(
+              prefs::kAutofillAcceptSaveCreditCardPromptState));
+      return;
+    }
+  }
 
   AutofillMetrics::LogCreditCardInfoBarMetric(
       AutofillMetrics::INFOBAR_SHOWN, upload_,
@@ -63,6 +70,12 @@ void AutofillSaveCardInfoBarDelegateMobile::OnLegalMessageLinkClicked(
   infobar()->owner()->OpenURL(url, WindowOpenDisposition::NEW_FOREGROUND_TAB);
 }
 
+bool AutofillSaveCardInfoBarDelegateMobile::LegalMessagesParsedSuccessfully() {
+  // If we are uploading to the server, verify that legal lines have been parsed
+  // into |legal_messages_|.
+  return !upload_ || !legal_messages_.empty();
+}
+
 int AutofillSaveCardInfoBarDelegateMobile::GetIconId() const {
   return IDR_INFOBAR_AUTOFILL_CC;
 }

components/autofill/core/browser/autofill_save_card_infobar_delegate_mobile.h

@@ -45,6 +45,10 @@ class AutofillSaveCardInfoBarDelegateMobile : public ConfirmInfoBarDelegate {
   // Called when a link in the legal message text was clicked.
   void OnLegalMessageLinkClicked(GURL url);
 
+  // Ensures the InfoBar is not shown if legal messages failed to parse.
+  // Legal messages are only specified for the upload case, not for local save.
+  bool LegalMessagesParsedSuccessfully();
+
   // ConfirmInfoBarDelegate:
   int GetIconId() const override;
   base::string16 GetMessageText() const override;