Autofill - don't prompt to unmask a credit card if the number isn't needed.

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 <limits>
 #include <map>
@@ skipping 533 matching lines @@
         suggestions);
   } else {
     // Autocomplete is disabled for this field; only pass back Autofill
     // suggestions.
     autocomplete_history_manager_->CancelPendingQuery();
     external_delegate_->OnSuggestionsReturned(
         query_id, suggestions);
   }
 }
 
+bool AutofillManager::WillFillCreditCardNumber(const FormData& form,
+                                               const FormFieldData& field) {
+  FormStructure* form_structure = NULL;

[brettw, 2015/03/24 22:30:47]

I thought we were preferring nullptr for new code. Not critical though.

[Evan Stade, 2015/03/24 22:34:01]

old habits die hard

+  AutofillField* autofill_field = NULL;
+  if (!GetCachedFormAndField(form, field, &form_structure, &autofill_field))
+    return false;
+
+  if (autofill_field->Type().GetStorableType() == CREDIT_CARD_NUMBER)
+    return true;
+
+  // If the relevant section is autofilled, we'll only fill |autofill_field|.

[brettw, 2015/03/24 22:30:47]

Everything here makes sense to me except these three lines. I'm assuming is
misunderstand what is_autofilled means, but I don't see any documentation. If
this means that we already autofilled this section and now we're doing it again
so only this field will change, maybe the comment should say "If the relevant
section is already autofilled, this new operation will only fill
|autofill_field|."

[Evan Stade, 2015/03/24 22:34:01]

your understanding is correct, I'll apply your new comment

+  if (SectionIsAutofilled(*form_structure, form, autofill_field->section()))
+    return false;
+
+  DCHECK_EQ(form_structure->field_count(), form.fields.size());
+  for (size_t i = 0; i < form_structure->field_count(); ++i) {
+    if (form_structure->field(i)->section() == autofill_field->section() &&
+        form_structure->field(i)->Type().GetStorableType() ==
+            CREDIT_CARD_NUMBER &&
+        form.fields[i].value.empty()) {
+      return true;
+    }
+  }
+
+  return false;
+}
+
 void AutofillManager::FillOrPreviewCreditCardForm(
     AutofillDriver::RendererFormDataAction action,
     int query_id,
     const FormData& form,
     const FormFieldData& field,
     const CreditCard& credit_card,
     size_t variant) {
   if (action == AutofillDriver::FORM_DATA_ACTION_FILL) {
-    if (credit_card.record_type() == CreditCard::MASKED_SERVER_CARD) {
+    if (credit_card.record_type() == CreditCard::MASKED_SERVER_CARD &&
+        WillFillCreditCardNumber(form, field)) {
       unmasking_card_ = credit_card;
       unmasking_query_id_ = query_id;
       unmasking_form_ = form;
       unmasking_field_ = field;
       real_pan_client_.Prepare();
       client()->ShowUnmaskPrompt(unmasking_card_,
                                  weak_ptr_factory_.GetWeakPtr());
       credit_card_form_event_logger_->OnDidSelectMaskedServerCardSuggestion();
       return;
     }
@@ skipping 889 matching lines @@
     return false;
 
   // Disregard forms that we wouldn't ever autofill in the first place.
   if (!form.ShouldBeParsed())
     return false;
 
   return true;
 }
 
 }  // namespace autofill