rAc - hardcode wallet billing address to US in i18n mode

chrome/browser/ui/autofill/autofill_dialog_common.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_common.h"
 
+#include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #include "components/autofill/core/browser/autofill_country.h"
 #include "components/autofill/core/browser/autofill_field.h"
 #include "components/autofill/core/browser/autofill_type.h"
 #include "grit/chromium_strings.h"
 #include "grit/component_strings.h"
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "grit/webkit_resources.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if !defined(OS_ANDROID)
 #include "chrome/browser/ui/autofill/autofill_dialog_i18n_input.h"
 #endif  // !defined(OS_ANDROID)
 
 namespace autofill {
 namespace common {
 

[Dan Beam, 2014/02/07 01:30:44]

namespace {
const char kHardcodedCountryCode[] = "US";
}

[Evan Stade, 2014/02/07 01:52:43]

why?

[Dan Beam, 2014/02/07 03:33:23]

because of all the other comments in this file.  alternatively, why not just
inline "US" then?  it's shorter...

[Evan Stade, 2014/02/07 21:20:48]

ah, I missed the last one. I don't think we need to share the constant in these
two cases as they are distinct. One is because we were previously
technologically unable to accept i18n input. The other is for a business reason
(Wallet can't offer the service outside of the US). Therefore they are unlikely
to be updated at the same time; sharing a constant would falsely suggest
otherwise.

The reason I didn't inline "US" is because

a) I was trying to make it harder to break the code (two different inlines would
make it easier to update L211 without also updating L216)
b) the variable name helps document its purpose.

but if the new code only used "US" once I probably would have just inlined it.

 // Returns true if |input| should be shown when |field_type| has been requested.
 bool ServerTypeMatchesFieldType(ServerFieldType type,
                                 const AutofillType& field_type) {
   // If any credit card expiration info is asked for, show both month and year
   // inputs.
   ServerFieldType server_type = field_type.GetStorableType();
   if (server_type == CREDIT_CARD_EXP_4_DIGIT_YEAR ||
       server_type == CREDIT_CARD_EXP_2_DIGIT_YEAR ||
       server_type == CREDIT_CARD_EXP_DATE_4_DIGIT_YEAR ||
       server_type == CREDIT_CARD_EXP_DATE_2_DIGIT_YEAR ||
@@ skipping 158 matching lines @@
     case SECTION_BILLING:
       if (IsI18nInputEnabled())
         BuildI18nAddressInputs(ADDRESS_TYPE_BILLING, country_code, inputs);
       else
         BuildInputs(kBillingInputs, arraysize(kBillingInputs), inputs);
 
       BuildInputs(kBillingPhoneInputs, arraysize(kBillingPhoneInputs), inputs);
       BuildInputs(kEmailInputs, arraysize(kEmailInputs), inputs);
       break;
 
     case SECTION_CC_BILLING:

[Dan Beam, 2014/02/07 01:30:44]

^ in general I'd prefer to minimize the amount of assumptions we make that
SECTION_CC_BILLING == Wallet but I guess we're a ways from needing to break that
assumption now...

[Evan Stade, 2014/02/07 01:52:43]

Wallet is the only reason SECTION_CC_BILLING even exists. It should really be
called SECTION_WALLET_BILLING.

[Dan Beam, 2014/02/07 03:33:23]

it's not a big deal until we encounter another data source that requires
combined billing (which we have no current prospects to do)

       BuildInputs(kCCInputs, arraysize(kCCInputs), inputs);
 
-      if (IsI18nInputEnabled())
-        BuildI18nAddressInputs(ADDRESS_TYPE_BILLING, country_code, inputs);
-      else
+      if (IsI18nInputEnabled()) {
+        // Wallet only supports US billing addresses.
+        const std::string hardcoded_country_code = "US";
+        BuildI18nAddressInputs(ADDRESS_TYPE_BILLING,
+                               hardcoded_country_code,
+                               inputs);
+        DCHECK_EQ(inputs->back().type, ADDRESS_HOME_COUNTRY);
+        inputs->back().length = DetailInput::NONE;
+        inputs->back().initial_value =
+            base::ASCIIToUTF16(hardcoded_country_code);

[Dan Beam, 2014/02/07 01:30:44]

s/hardcoded_country_code/kHardcodedCountryCode/g

+      } else {
         BuildInputs(kBillingInputs, arraysize(kBillingInputs), inputs);
+      }
 
       BuildInputs(kBillingPhoneInputs, arraysize(kBillingPhoneInputs), inputs);
       break;
 
     case SECTION_SHIPPING:
       if (IsI18nInputEnabled())
         BuildI18nAddressInputs(ADDRESS_TYPE_SHIPPING, country_code, inputs);
       else
         BuildInputs(kShippingInputs, arraysize(kShippingInputs), inputs);
 
@@ skipping 38 matching lines @@
     case SECTION_CC:
       return AutofillMetrics::DIALOG_UI_CC_SELECTED_SUGGESTION_CHANGED;
   }
 
   NOTREACHED();
   return AutofillMetrics::NUM_DIALOG_UI_EVENTS;
 }
 
 base::string16 GetHardcodedValueForType(ServerFieldType type) {
   // TODO(dbeam): remove this entire function when i18n inputs are the default.
-  if (IsI18nInputEnabled())
-    return base::string16();
+  DCHECK(!IsI18nInputEnabled());
 
   if (AutofillType(type).GetStorableType() == ADDRESS_HOME_COUNTRY) {
     AutofillCountry country("US", g_browser_process->GetApplicationLocale());

[Dan Beam, 2014/02/07 01:30:44]

s/"US"/kHardcodedCountryCode/

     return country.name();
   }
 
   return base::string16();
 }
 
 std::vector<ServerFieldType> TypesFromInputs(const DetailInputs& inputs) {
   std::vector<ServerFieldType> types;
   for (size_t i = 0; i < inputs.size(); ++i) {
     types.push_back(inputs[i].type);
   }
   return types;
 }
 
 }  // namespace common
 }  // namespace autofill