Do not show Scan or Sign In options when credit card form is non-secure

components/autofill/core/browser/autofill_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/autofill/core/browser/autofill_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 291 matching lines @@
       base::ContainsOnlyChars(CreditCard::StripSeparators(field.value),
                               base::ASCIIToUTF16("0123456789"));
 
   bool is_scannable_name_on_card_field =
       autofill_field->Type().GetStorableType() == CREDIT_CARD_NAME_FULL &&
       base::FeatureList::IsEnabled(kAutofillScanCardholderName);
 
   if (!is_card_number_field && !is_scannable_name_on_card_field)
     return false;
 
+  if (IsFormNonSecure(form))
+    return false;
+
   static const int kShowScanCreditCardMaxValueLength = 6;
   return field.value.size() <= kShowScanCreditCardMaxValueLength;
 }
 
 bool AutofillManager::IsCreditCardPopup(const FormData& form,
                                         const FormFieldData& field) {
   AutofillField* autofill_field = GetAutofillField(form, field);
   return autofill_field && autofill_field->Type().group() == CREDIT_CARD;
 }
 
 bool AutofillManager::ShouldShowCreditCardSigninPromo(

[Mathieu, 2017/02/06 18:21:26]

also see if the unit test for this needs to be updated?

[elawrence, 2017/02/07 20:16:48]

I added two new tests that check the new restriction.

     const FormData& form,
     const FormFieldData& field) {
   // Check whether we are dealing with a credit card field and whether it's
   // appropriate to show the promo (e.g. the platform is supported).
   AutofillField* autofill_field = GetAutofillField(form, field);
   if (!autofill_field || autofill_field->Type().group() != CREDIT_CARD ||
       !client_->ShouldShowSigninPromo())
     return false;
 
+  if (IsFormNonSecure(form))
+    return false;
+
   // The last step is checking if we are under the limit of impressions.
   int impression_count = client_->GetPrefs()->GetInteger(
       prefs::kAutofillCreditCardSigninPromoImpressionCount);
   if (impression_count < kCreditCardSigninPromoImpressionLimit) {
     // The promo will be shown. Increment the impression count.
     client_->GetPrefs()->SetInteger(
         prefs::kAutofillCreditCardSigninPromoImpressionCount,
         impression_count + 1);
     return true;
   }
@@ skipping 178 matching lines @@
     if (!user_did_edit_autofilled_field_) {
       user_did_edit_autofilled_field_ = true;
       AutofillMetrics::LogUserHappinessMetric(
           AutofillMetrics::USER_DID_EDIT_AUTOFILLED_FIELD_ONCE);
     }
   }
 
   UpdateInitialInteractionTimestamp(timestamp);
 }
 
+bool AutofillManager::IsFormNonSecure(const FormData& form) const {

[Mathieu, 2017/02/06 18:16:41]

short unit test? AutofillClient functionality will be easy to mock out with
TestAutofillClient.

[elawrence, 2017/02/07 20:16:47]

My new Unit tests exercise the IsFormNonSecure function which seemed safer than
"Friend"ing into the class to get access to this private member. Please let me
know if that's not okay.

+  if (!client_->IsContextSecure(form.origin))
+    return true;
+  if (form.action.is_valid() && form.action.SchemeIs("http"))

[Mathieu, 2017/02/06 18:16:41]

return !client_->IsContextSecure(form.origin) || (form.action.is_valid() &&
form.action.SchemeIs("http"));

[elawrence, 2017/02/07 20:16:47]

Done.

+    return true;
+
+  return false;
+}
+
 void AutofillManager::OnQueryFormFieldAutofill(int query_id,
                                                const FormData& form,
                                                const FormFieldData& field,
                                                const gfx::RectF& bounding_box) {
   if (!IsValidFormData(form) || !IsValidFormFieldData(field))
     return;
 
   gfx::RectF transformed_box =
       driver_->TransformBoundingBoxToViewportCoordinates(bounding_box);
 
@@ skipping 16 matching lines @@
     if (autofill_field->Type().group() == CREDIT_CARD) {
       is_filling_credit_card = true;
       driver_->DidInteractWithCreditCardForm();
       credit_card_form_event_logger_->OnDidInteractWithAutofillableForm();
     } else {
       address_form_event_logger_->OnDidInteractWithAutofillableForm();
     }
   }
 
   std::vector<Suggestion> suggestions;
-  const bool is_context_secure =
-      !form_structure ||
-      (client_->IsContextSecure(form_structure->source_url()) &&
-       (!form_structure->target_url().is_valid() ||
-        !form_structure->target_url().SchemeIs("http")));
+  const bool is_context_secure = !IsFormNonSecure(form);
+
   const bool is_http_warning_enabled =
       security_state::IsHttpWarningInFormEnabled();
 
   // TODO(rogerm): Early exit here on !driver_->RendererIsAvailable()?
   // We skip populating autofill data, but might generate warnings and or
   // signin promo to show over the unavailable renderer. That seems a mistake.
 
   if (is_autofill_possible &&
       driver_->RendererIsAvailable() &&
       got_autofillable_form) {
@@ skipping 1589 matching lines @@
     if (i > 0)
       fputs("Next oldest form:\n", file);
   }
   fputs("\n", file);
 
   fclose(file);
 }
 #endif  // ENABLE_FORM_DEBUG_DUMP
 
 }  // namespace autofill