Highlight fields we know to be invalid but have no way of locally checking.

chrome/browser/ui/autofill/autofill_dialog_controller_impl.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 "chrome/browser/ui/autofill/autofill_dialog_controller_impl.h"
 
 #include <algorithm>
 #include <map>
 #include <string>
 
@@ skipping 1208 matching lines @@
 }
 
 // TODO(estade): Replace all the error messages here with more helpful and
 // translateable ones. TODO(groby): Also add tests.
 ValidityData AutofillDialogControllerImpl::InputsAreValid(
     const DetailOutputMap& inputs, ValidationType validation_type) const {
   ValidityData invalid_messages;
   std::map<AutofillFieldType, string16> field_values;
   for (DetailOutputMap::const_iterator iter = inputs.begin();
        iter != inputs.end(); ++iter) {
-    // Skip empty fields in edit mode.
-    if (validation_type == VALIDATE_EDIT && iter->second.empty())
+    // Skip empty fields in edit mode or when selecting a suggestion.
+    if ((validation_type == VALIDATE_EDIT ||
+         validation_type == VALIDATE_SUGGESTION) && iter->second.empty()) {

[Evan Stade, 2013/06/01 00:39:19]

I thought you suggested we could get rid of this

[Dan Beam, 2013/06/01 01:15:04]

Done.

       continue;
+    }
 
     const AutofillFieldType type = iter->first->type;
     string16 message = InputValidityMessage(type, iter->second);
     if (!message.empty())
       invalid_messages[type] = message;
     else
       field_values[type] = iter->second;
   }
 
   // Validate the date formed by month and year field. (Autofill dialog is
   // never supposed to have 2-digit years, so not checked).
-  if (field_values.count(CREDIT_CARD_EXP_MONTH) &&
-      field_values.count(CREDIT_CARD_EXP_4_DIGIT_YEAR)) {
-    if (!autofill::IsValidCreditCardExpirationDate(
-            field_values[CREDIT_CARD_EXP_4_DIGIT_YEAR],
-            field_values[CREDIT_CARD_EXP_MONTH],
-            base::Time::Now())) {
-      invalid_messages[CREDIT_CARD_EXP_MONTH] =
-          ASCIIToUTF16("more complicated message");
-      invalid_messages[CREDIT_CARD_EXP_4_DIGIT_YEAR] =
-          ASCIIToUTF16("more complicated message");
-    }
+  if (field_values.count(CREDIT_CARD_EXP_4_DIGIT_YEAR) &&
+      field_values.count(CREDIT_CARD_EXP_MONTH) &&
+      !IsCCExpirationValid(field_values[CREDIT_CARD_EXP_4_DIGIT_YEAR],
+                           field_values[CREDIT_CARD_EXP_MONTH])) {
+    invalid_messages[CREDIT_CARD_EXP_4_DIGIT_YEAR] =
+        ASCIIToUTF16("more complicated message");
+    invalid_messages[CREDIT_CARD_EXP_MONTH] =
+        ASCIIToUTF16("more complicated message");
   }
 
   // If there is a credit card number and a CVC, validate them together.
   if (field_values.count(CREDIT_CARD_NUMBER) &&
       field_values.count(CREDIT_CARD_VERIFICATION_CODE) &&
       InputValidityMessage(CREDIT_CARD_NUMBER,
-                           field_values[CREDIT_CARD_NUMBER]).empty()) {
-    if (!autofill::IsValidCreditCardSecurityCode(
-            field_values[CREDIT_CARD_VERIFICATION_CODE],
-            field_values[CREDIT_CARD_NUMBER])) {
-      invalid_messages[CREDIT_CARD_VERIFICATION_CODE] =
-          ASCIIToUTF16("CVC doesn't match card type!");
-    }
+                           field_values[CREDIT_CARD_NUMBER]).empty() &&
+      !autofill::IsValidCreditCardSecurityCode(
+          field_values[CREDIT_CARD_VERIFICATION_CODE],
+          field_values[CREDIT_CARD_NUMBER])) {
+    invalid_messages[CREDIT_CARD_VERIFICATION_CODE] =
+        ASCIIToUTF16("CVC doesn't match card type!");
   }
 
   // Validate the phone number against the country code of the address.
   if (field_values.count(ADDRESS_HOME_COUNTRY) &&
       field_values.count(PHONE_HOME_WHOLE_NUMBER)) {
     i18n::PhoneObject phone_object(
         field_values[PHONE_HOME_WHOLE_NUMBER],
         AutofillCountry::GetCountryCode(
             field_values[ADDRESS_HOME_COUNTRY],
             g_browser_process->GetApplicationLocale()));
@@ skipping 1134 matching lines @@
 bool AutofillDialogControllerImpl::SectionIsValid(
     DialogSection section) const {
   if (!IsManuallyEditingSection(section))
     return true;
 
   DetailOutputMap detail_outputs;
   view_->GetUserInput(section, &detail_outputs);
   return InputsAreValid(detail_outputs, VALIDATE_EDIT).empty();
 }
 
+bool AutofillDialogControllerImpl::IsCCExpirationValid(

[Dan Beam, 2013/06/01 00:25:56]

whoops, return values are inverted, fixing

[Evan Stade, 2013/06/01 00:39:19]

nit: IsCreditCardExpirationValid (preferred) or IsCcExpirationValid

+    const base::string16& year,
+    const base::string16& month) const {
+  // If the expiration is in the past as per the local clock, it's invalid.
+  base::Time now = base::Time::Now();
+  if (!autofill::IsValidCreditCardExpirationDate(year, month, now))
+    return true;
+
+  if (IsPayingWithWallet() && IsEditingExistingData(SECTION_CC_BILLING)) {
+    const SuggestionsMenuModel* model =
+        SuggestionsMenuModelForSection(SECTION_CC_BILLING);
+    int instrument_index = -1;
+    base::StringToInt(model->GetItemKeyForCheckedItem(), &instrument_index);

[Evan Stade, 2013/06/01 00:39:19]

I see a lot of uses of StringToInt in this file for converting from section to
active instrument. Can you pull it into a helper function?

+    const wallet::WalletItems::MaskedInstrument* instrument =
+        wallet_items_->instruments()[instrument_index];
+    const std::string& locale = g_browser_process->GetApplicationLocale();
+    if (instrument->status() ==
+            wallet::WalletItems::MaskedInstrument::EXPIRED &&
+        year == instrument->GetInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR, locale) &&
+        month == instrument->GetInfo(CREDIT_CARD_EXP_MONTH, locale)) {
+      // Otherwise, if the user is editing an instrument that's deemed expired
+      // by the Online Wallet server, mark it invalid on selection.
+      return true;
+    }
+  }
+
+  return false;
+}
+
 bool AutofillDialogControllerImpl::ShouldUseBillingForShipping() {
   return suggested_shipping_.GetItemKeyForCheckedItem() == kSameAsBillingKey;
 }
 
 bool AutofillDialogControllerImpl::ShouldSaveDetailsLocally() {
   // It's possible that the user checked [X] Save details locally before
   // switching payment methods, so only ask the view whether to save details
   // locally if that checkbox is showing (currently if not paying with wallet).
   // Also, if the user isn't editing any sections, there's no data to save
   // locally.
@@ skipping 455 matching lines @@
         AutofillMetrics::DIALOG_USER_SIGNED_IN_NO_WALLET_NO_AUTOFILL;
   }
 
   // Has Wallet items.
   return has_autofill_profiles ?
       AutofillMetrics::DIALOG_USER_SIGNED_IN_HAS_WALLET_HAS_AUTOFILL :
       AutofillMetrics::DIALOG_USER_SIGNED_IN_HAS_WALLET_NO_AUTOFILL;
 }
 
 }  // namespace autofill