[Autofill] Offer credit card suggestions for some form action.

A previous change had mistakenly filtered out some forms from showing
credit card suggestions. We now fix this and allow empty form actions
(which post to the current page) as well as "javascript:" handlers, for
example, all within pages that have a secure (and perhaps Mixed Passive) context.

BUG=652334
TEST=AutofillManagerTest,AutofillAssistantTest

Review-Url: https://chromiumcodereview.appspot.com/2432063003
Cr-Commit-Position: refs/heads/master@{#426915}

components/autofill/core/browser/autofill_assistant.cc

@@ -29,9 +29,12 @@ bool AutofillAssistant::CanShowCreditCardAssist(
     const std::vector<std::unique_ptr<FormStructure>>& form_structures) {
   if (form_structures.empty() || credit_card_form_data_ != nullptr ||
       !IsAutofillCreditCardAssistEnabled() ||
-      !autofill_manager_->client()->IsContextSecure(
-          form_structures.front()->source_url()) ||
-      !form_structures.front()->target_url().SchemeIs("https")) {
+      // Context of the page is not secure or target URL is valid but not
+      // secure.
+      !(autofill_manager_->client()->IsContextSecure(
+            form_structures.front()->source_url()) &&
+        (!form_structures.front()->target_url().is_valid() ||
+         !form_structures.front()->target_url().SchemeIs("http")))) {
     return false;
   }
 

components/autofill/core/browser/autofill_assistant_unittest.cc

@@ -145,34 +145,79 @@ TEST_F(AutofillAssistantTest, CanShowCreditCardAssist_FeatureOn) {
 }
 
 // Tests that with the feature enabled and proper input,
-// CanShowCreditCardAssist() behaves as expected for secure vs insecure
-// contexts.
+// CanShowCreditCardAssist() behaves as expected for secure contexts.
+TEST_F(AutofillAssistantTest, CanShowCreditCardAssist_FeatureOn_Secure) {
+  EnableAutofillCreditCardAssist();
+
+  // Can be shown if the context is secure.
+  FormData form = CreateValidCreditCardFormData();
+  std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
+  form_structure->DetermineHeuristicTypes();
+
+  std::vector<std::unique_ptr<FormStructure>> form_structures;
+  form_structures.push_back(std::move(form_structure));
+  EXPECT_TRUE(autofill_assistant_.CanShowCreditCardAssist(form_structures));
+}
+
+// Tests that with the feature enabled and proper input,
+// CanShowCreditCardAssist() behaves as expected for insecure contexts.
 TEST_F(AutofillAssistantTest, CanShowCreditCardAssist_FeatureOn_NotSecure) {
   EnableAutofillCreditCardAssist();
 
-  {
-    // Cannot be shown if the context is not secure.
-    FormData form = CreateValidCreditCardFormData();
-    form.action = GURL("http://myform.com");
-    form.action = GURL("http://myform.com/submit");
-    std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
-    form_structure->DetermineHeuristicTypes();
+  // Cannot be shown if the context is not secure.
+  FormData form = CreateValidCreditCardFormData();
+  form.origin = GURL("http://myform.com");
+  form.action = GURL("http://myform.com/submit");
+  std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
+  form_structure->DetermineHeuristicTypes();
 
-    std::vector<std::unique_ptr<FormStructure>> form_structures;
-    form_structures.push_back(std::move(form_structure));
-    EXPECT_FALSE(autofill_assistant_.CanShowCreditCardAssist(form_structures));
-  }
+  std::vector<std::unique_ptr<FormStructure>> form_structures;
+  form_structures.push_back(std::move(form_structure));
+  EXPECT_FALSE(autofill_assistant_.CanShowCreditCardAssist(form_structures));
+}
 
-  {
-    // Can be shown if the context is secure.
-    FormData form = CreateValidCreditCardFormData();
-    std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
-    form_structure->DetermineHeuristicTypes();
+TEST_F(AutofillAssistantTest, CanShowCreditCardAssist_FeatureOn_Javascript) {
+  EnableAutofillCreditCardAssist();
 
-    std::vector<std::unique_ptr<FormStructure>> form_structures;
-    form_structures.push_back(std::move(form_structure));
-    EXPECT_TRUE(autofill_assistant_.CanShowCreditCardAssist(form_structures));
-  }
+  // Can be shown if the context is secure and the form action is a javascript
+  // function (which is a valid url).
+  FormData form = CreateValidCreditCardFormData();
+  form.action = GURL("javascript:alert('hello');");
+  std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
+  form_structure->DetermineHeuristicTypes();
+
+  std::vector<std::unique_ptr<FormStructure>> form_structures;
+  form_structures.push_back(std::move(form_structure));
+  EXPECT_TRUE(autofill_assistant_.CanShowCreditCardAssist(form_structures));
+}
+
+TEST_F(AutofillAssistantTest, CanShowCreditCardAssist_FeatureOn_WeirdJs) {
+  EnableAutofillCreditCardAssist();
+
+  // Can be shown if the context is secure and the form action is a javascript
+  // function that may or may not be valid.
+  FormData form = CreateValidCreditCardFormData();
+  form.action = GURL("javascript:myFunc");
+  std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
+  form_structure->DetermineHeuristicTypes();
+
+  std::vector<std::unique_ptr<FormStructure>> form_structures;
+  form_structures.push_back(std::move(form_structure));
+  EXPECT_TRUE(autofill_assistant_.CanShowCreditCardAssist(form_structures));
+}
+
+TEST_F(AutofillAssistantTest, CanShowCreditCardAssist_FeatureOn_EmptyAction) {
+  EnableAutofillCreditCardAssist();
+
+  // Can be shown if the context is secure and the form action is empty.
+  FormData form = CreateValidCreditCardFormData();
+  form.action = GURL();
+  std::unique_ptr<FormStructure> form_structure(new FormStructure(form));
+  form_structure->DetermineHeuristicTypes();
+
+  std::vector<std::unique_ptr<FormStructure>> form_structures;
+  form_structures.push_back(std::move(form_structure));
+  EXPECT_TRUE(autofill_assistant_.CanShowCreditCardAssist(form_structures));
 }
 
 TEST_F(AutofillAssistantTest, ShowAssistForCreditCard_ValidCard_CancelCvc) {

components/autofill/core/browser/autofill_manager.cc

@@ -556,7 +556,8 @@ void AutofillManager::OnQueryFormFieldAutofill(int query_id,
     if (!suggestions.empty()) {
       bool is_context_secure =
           client_->IsContextSecure(form_structure->source_url()) &&
-          form_structure->target_url().SchemeIs("https");
+          (!form_structure->target_url().is_valid() ||
+           !form_structure->target_url().SchemeIs("http"));
       if (is_filling_credit_card)
         AutofillMetrics::LogIsQueriedCreditCardFormSecure(is_context_secure);
 

components/autofill/core/browser/autofill_manager_unittest.cc

@@ -1576,10 +1576,10 @@ TEST_F(AutofillManagerTest, GetCreditCardSuggestions_NonCCNumber) {
 
 // Test that we return a warning explaining that credit card profile suggestions
 // are unavailable when the page and the form target URL are not secure.
-TEST_F(AutofillManagerTest, GetCreditCardSuggestions_NonHTTPS) {
+TEST_F(AutofillManagerTest, GetCreditCardSuggestions_NonSecureContext) {
   // Set up our form data.
   FormData form;
-  CreateTestCreditCardFormData(&form, false, false);
+  CreateTestCreditCardFormData(&form, /* is_https */ false, false);
   std::vector<FormData> forms(1, form);
   FormsSeen(forms);
 
@@ -1601,9 +1601,10 @@ TEST_F(AutofillManagerTest, GetCreditCardSuggestions_NonHTTPS) {
 }
 
 // Test that we return a warning explaining that credit card profile suggestions
-// are unavailable when the page is secure, but the form target URL is not
-// secure.
-TEST_F(AutofillManagerTest, GetCreditCardSuggestions_TargetURLNonHTTPS) {
+// are unavailable when the page is secure, but the form action URL is valid but
+// not secure.
+TEST_F(AutofillManagerTest,
+       GetCreditCardSuggestions_SecureContext_FormActionNotHTTPS) {
   // Set up our form data.
   FormData form;
   CreateTestCreditCardFormData(&form, /* is_https= */ true, false);
@@ -1628,6 +1629,60 @@ TEST_F(AutofillManagerTest, GetCreditCardSuggestions_TargetURLNonHTTPS) {
   EXPECT_FALSE(external_delegate_->on_suggestions_returned_seen());
 }
 
+// Test that we return credit card suggestions for secure pages that have an
+// empty form action target URL.
+TEST_F(AutofillManagerTest,
+       GetCreditCardSuggestions_SecureContext_EmptyFormAction) {
+  // Set up our form data.
+  FormData form;
+  CreateTestCreditCardFormData(&form, true, false);
+  // Clear the form action.
+  form.action = GURL();
+  std::vector<FormData> forms(1, form);
+  FormsSeen(forms);
+
+  FormFieldData field = form.fields[1];
+  GetAutofillSuggestions(form, field);
+
+  // Test that we sent the right values to the external delegate.
+  external_delegate_->CheckSuggestions(
+      kDefaultPageID, Suggestion("Visa\xC2\xA0\xE2\x8B\xAF"
+                                 "3456",
+                                 "04/99", kVisaCard,
+                                 autofill_manager_->GetPackedCreditCardID(4)),
+      Suggestion("MasterCard\xC2\xA0\xE2\x8B\xAF"
+                 "8765",
+                 "10/98", kMasterCard,
+                 autofill_manager_->GetPackedCreditCardID(5)));
+}
+
+// Test that we return credit card suggestions for secure pages that have a
+// form action set to "javascript:something".
+TEST_F(AutofillManagerTest,
+       GetCreditCardSuggestions_SecureContext_JavascriptFormAction) {
+  // Set up our form data.
+  FormData form;
+  CreateTestCreditCardFormData(&form, true, false);
+  // Have the form action be a javascript function (which is a valid URL).
+  form.action = GURL("javascript:alert('Hello');");
+  std::vector<FormData> forms(1, form);
+  FormsSeen(forms);
+
+  FormFieldData field = form.fields[1];
+  GetAutofillSuggestions(form, field);
+
+  // Test that we sent the right values to the external delegate.
+  external_delegate_->CheckSuggestions(
+      kDefaultPageID, Suggestion("Visa\xC2\xA0\xE2\x8B\xAF"
+                                 "3456",
+                                 "04/99", kVisaCard,
+                                 autofill_manager_->GetPackedCreditCardID(4)),
+      Suggestion("MasterCard\xC2\xA0\xE2\x8B\xAF"
+                 "8765",
+                 "10/98", kMasterCard,
+                 autofill_manager_->GetPackedCreditCardID(5)));
+}
+
 // Test that we return all credit card suggestions in the case that two cards
 // have the same obfuscated number.
 TEST_F(AutofillManagerTest, GetCreditCardSuggestions_RepeatedObfuscatedNumber) {