Add country combobox to change country and rebuild address inputs.

R=estade@chromium.org
BUG=331544

Review URL: https://codereview.chromium.org/124533003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245434 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/autofill/autofill_dialog_common.cc

@@ -264,6 +264,10 @@ AutofillMetrics::DialogUiEvent DialogSectionToUiSelectionChangedEvent(
 }
 
 base::string16 GetHardcodedValueForType(ServerFieldType type) {
+  // TODO(dbeam): remove this entire function when i18n inputs are the default.
+  if (IsI18nInputEnabled())
+    return base::string16();
+
   if (AutofillType(type).GetStorableType() == ADDRESS_HOME_COUNTRY) {
     AutofillCountry country("US", g_browser_process->GetApplicationLocale());
     return country.name();

chrome/browser/ui/autofill/autofill_dialog_controller_impl.h

@@ -11,6 +11,7 @@
 #include "base/callback.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
+#include "base/scoped_observer.h"
 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "chrome/browser/ui/autofill/account_chooser_model.h"
@@ -36,6 +37,7 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/ssl_status.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/base/models/combobox_model_observer.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/base/ui_base_types.h"
 #include "ui/gfx/animation/animation_delegate.h"
@@ -76,7 +78,8 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
                                      public wallet::WalletSigninHelperDelegate,
                                      public PersonalDataManagerObserver,
                                      public AccountChooserModelDelegate,
-                                     public gfx::AnimationDelegate {
+                                     public gfx::AnimationDelegate,
+                                     public ui::ComboboxModelObserver {
  public:
   virtual ~AutofillDialogControllerImpl();
 
@@ -221,6 +224,9 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   virtual void AnimationEnded(const gfx::Animation* animation) OVERRIDE;
   virtual void AnimationProgressed(const gfx::Animation* animation) OVERRIDE;
 
+  // ui::ComboboxModelObserver implementation.
+  virtual void OnComboboxModelChanged(ui::ComboboxModel* model) OVERRIDE;
+
  protected:
   enum DialogSignedInState {
     NOT_CHECKED,
@@ -269,6 +275,10 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   // Opens the given URL in a new foreground tab.
   virtual void OpenTabWithUrl(const GURL& url);
 
+  // The active billing section for the current state of the dialog (e.g. when
+  // paying for wallet, the combined credit card + billing address section).
+  DialogSection ActiveBillingSection() const;
+
   // Whether |section| was sent into edit mode based on existing data. This
   // happens when a user clicks "Edit" or a suggestion is invalid.
   virtual bool IsEditingExistingData(DialogSection section) const;
@@ -422,6 +432,9 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   DialogSection SectionForSuggestionsMenuModel(
       const SuggestionsMenuModel& model);
 
+  // Gets the CountryComboboxModel for |section|.
+  CountryComboboxModel* CountryComboboxModelForSection(DialogSection section);
+
   // Suggested text and icons for sections. Suggestion text is used to show an
   // abridged overview of the currently used suggestion. Extra text is used when
   // part of a section is suggested but part must be manually input (e.g. during
@@ -449,6 +462,9 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   // Like RequestedFieldsForSection, but returns a pointer.
   DetailInputs* MutableRequestedFieldsForSection(DialogSection section);
 
+  // Returns the country code (e.g. "US") for |section|.
+  std::string CountryCodeForSection(DialogSection section);
+
   // Hides |popup_controller_|'s popup view, if it exists.
   void HidePopup();
 
@@ -649,8 +665,9 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   MonthComboboxModel cc_exp_month_combobox_model_;
   YearComboboxModel cc_exp_year_combobox_model_;
 
-  // Model for the country input.
-  CountryComboboxModel country_combobox_model_;
+  // Models for country input.
+  CountryComboboxModel billing_country_combobox_model_;
+  CountryComboboxModel shipping_country_combobox_model_;
 
   // Models for the suggestion views.
   SuggestionsMenuModel suggested_cc_;
@@ -758,6 +775,8 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   // A username string we display in the card scrambling/generated overlay.
   base::string16 submitted_cardholder_name_;
 
+  ScopedObserver<ui::ComboboxModel, AutofillDialogControllerImpl> observer_;
+
   DISALLOW_COPY_AND_ASSIGN(AutofillDialogControllerImpl);
 };
 

chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/ui/autofill/mock_new_credit_card_bubble_controller.h"
 #include "chrome/browser/ui/autofill/test_generated_credit_card_bubble_controller.h"
 #include "chrome/browser/webdata/web_data_service_factory.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -170,6 +171,7 @@ class TestAutofillDialogView : public AutofillDialogView {
   }
 
   virtual void UpdateSection(DialogSection section) OVERRIDE {
+    section_updates_[section]++;
     EXPECT_GE(updates_started_, 1);
   }
 
@@ -215,8 +217,17 @@ class TestAutofillDialogView : public AutofillDialogView {
     save_details_locally_checked_ = checked;
   }
 
+  void ClearSectionUpdates() {
+    section_updates_.clear();
+  }
+
+  std::map<DialogSection, size_t> section_updates() const {
+    return section_updates_;
+  }
+
  private:
   std::map<DialogSection, FieldValueMap> outputs_;
+  std::map<DialogSection, size_t> section_updates_;
 
   int updates_started_;
   bool save_details_locally_checked_;
@@ -2923,4 +2934,49 @@ TEST_F(AutofillDialogControllerTest, IconReservedForCreditCardField) {
   }
 }
 
+TEST_F(AutofillDialogControllerTest, CountryChangeUpdatesSection) {
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  command_line->AppendSwitch(::switches::kEnableAutofillAddressI18n);
+
+  Reset();
+
+  TestAutofillDialogView* view = controller()->GetView();
+  view->ClearSectionUpdates();
+
+  controller()->UserEditedOrActivatedInput(SECTION_SHIPPING,
+                                           ADDRESS_HOME_COUNTRY,
+                                           gfx::NativeView(),
+                                           gfx::Rect(),
+                                           ASCIIToUTF16("China"),
+                                           true);
+  std::map<DialogSection, size_t> updates = view->section_updates();
+  EXPECT_EQ(1U, updates[SECTION_SHIPPING]);
+  EXPECT_EQ(1U, updates.size());
+
+  view->ClearSectionUpdates();
+
+  controller()->UserEditedOrActivatedInput(SECTION_CC_BILLING,
+                                           ADDRESS_BILLING_COUNTRY,
+                                           gfx::NativeView(),
+                                           gfx::Rect(),
+                                           ASCIIToUTF16("France"),
+                                           true);
+  updates = view->section_updates();
+  EXPECT_EQ(1U, updates[SECTION_CC_BILLING]);
+  EXPECT_EQ(1U, updates.size());
+
+  SwitchToAutofill();
+  view->ClearSectionUpdates();
+
+  controller()->UserEditedOrActivatedInput(SECTION_BILLING,
+                                           ADDRESS_BILLING_COUNTRY,
+                                           gfx::NativeView(),
+                                           gfx::Rect(),
+                                           ASCIIToUTF16("Italy"),
+                                           true);
+  updates = view->section_updates();
+  EXPECT_EQ(1U, updates[SECTION_BILLING]);
+  EXPECT_EQ(1U, updates.size());
+}
+
 }  // namespace autofill

chrome/browser/ui/autofill/autofill_dialog_i18n_input.cc

@@ -98,8 +98,7 @@ void BuildAddressInputs(common::AddressType address_type,
       billing ? ADDRESS_BILLING_COUNTRY : ADDRESS_HOME_COUNTRY;
   base::string16 placeholder_text =
       l10n_util::GetStringUTF16(IDS_AUTOFILL_FIELD_LABEL_COUNTRY);
-  // TODO(dbeam): unhide so users can switch countries. http://crbug.com/331544
-  DetailInput input = { DetailInput::NONE, server_type, placeholder_text };
+  DetailInput input = { DetailInput::LONG, server_type, placeholder_text };
   inputs->push_back(input);
 }
 

chrome/browser/ui/autofill/autofill_dialog_view_delegate.h

@@ -154,9 +154,9 @@ class AutofillDialogViewDelegate {
   virtual ValidityMessages InputsAreValid(DialogSection section,
                                           const FieldValueMap& inputs) = 0;
 
-  // Called when the user changes the contents of a text field or activates it
-  // (by focusing and then clicking it). |was_edit| is true when the function
-  // was called in response to the user editing the text field.
+  // Called when the user edits or activates a textfield or combobox.
+  // |was_edit| is true when the function was called in response to the user
+  // editing the input.
   virtual void UserEditedOrActivatedInput(DialogSection section,
                                           ServerFieldType type,
                                           gfx::NativeView parent_view,

chrome/browser/ui/autofill/country_combobox_model.cc

@@ -4,17 +4,20 @@
 
 #include "chrome/browser/ui/autofill/country_combobox_model.h"
 
+#include "base/logging.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/personal_data_manager.h"
 #include "ui/base/l10n/l10n_util_collator.h"
+#include "ui/base/models/combobox_model_observer.h"
 
 namespace autofill {
 
-CountryComboboxModel::CountryComboboxModel(const PersonalDataManager& manager) {
+CountryComboboxModel::CountryComboboxModel(const PersonalDataManager& manager)
+    : default_index_(0) {
   // Insert the default country at the top as well as in the ordered list.
-  std::string app_locale = g_browser_process->GetApplicationLocale();
+  const std::string& app_locale = g_browser_process->GetApplicationLocale();
   std::string default_country_code =
       manager.GetDefaultCountryCodeForNewAddress();
   DCHECK(!default_country_code.empty());
@@ -61,4 +64,50 @@ bool CountryComboboxModel::IsItemSeparatorAt(int index) {
   return !countries_[index];
 }
 
+int CountryComboboxModel::GetDefaultIndex() const {
+  return default_index_;
+}
+
+void CountryComboboxModel::AddObserver(ui::ComboboxModelObserver* observer) {
+  observers_.AddObserver(observer);
+}
+
+void CountryComboboxModel::RemoveObserver(ui::ComboboxModelObserver* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void CountryComboboxModel::ResetDefault() {
+  SetDefaultIndex(0);
+}
+
+void CountryComboboxModel::SetDefaultCountry(const std::string& country_code) {
+  DCHECK_EQ(2U, country_code.length());
+
+  for (size_t i = 0; i < countries_.size(); ++i) {
+    if (countries_[i] && countries_[i]->country_code() == country_code) {
+      SetDefaultIndex(i);
+      return;
+    }
+  }
+
+  NOTREACHED();
+}
+
+std::string CountryComboboxModel::GetDefaultCountryCode() const {
+  return countries_[default_index_]->country_code();
+}
+
+void CountryComboboxModel::SetDefaultIndex(int index) {
+  DCHECK_GE(index, 0);
+  DCHECK_LT(index, static_cast<int>(countries_.size()));
+  DCHECK(!IsItemSeparatorAt(index));
+
+  if (index == default_index_)
+    return;
+
+  default_index_ = index;
+  FOR_EACH_OBSERVER(ui::ComboboxModelObserver, observers_,
+                    OnComboboxModelChanged(this));
+}
+
 }  // namespace autofill

chrome/browser/ui/autofill/country_combobox_model.h

@@ -5,10 +5,12 @@
 #ifndef CHROME_BROWSER_UI_AUTOFILL_COUNTRY_COMBOBOX_MODEL_H_
 #define CHROME_BROWSER_UI_AUTOFILL_COUNTRY_COMBOBOX_MODEL_H_
 
+#include <string>
 #include <vector>
 
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_vector.h"
+#include "base/observer_list.h"
 #include "base/strings/string16.h"
 #include "ui/base/models/combobox_model.h"
 
@@ -23,20 +25,41 @@ class CountryComboboxModel : public ui::ComboboxModel {
   explicit CountryComboboxModel(const PersonalDataManager& manager);
   virtual ~CountryComboboxModel();
 
-  // ui::Combobox implementation:
+  // ui::ComboboxModel implementation:
   virtual int GetItemCount() const OVERRIDE;
   virtual base::string16 GetItemAt(int index) OVERRIDE;
   virtual bool IsItemSeparatorAt(int index) OVERRIDE;
+  virtual int GetDefaultIndex() const OVERRIDE;
+  virtual void AddObserver(ui::ComboboxModelObserver* observer) OVERRIDE;
+  virtual void RemoveObserver(ui::ComboboxModelObserver* observer) OVERRIDE;
 
   const std::vector<AutofillCountry*>& countries() const {
     return countries_.get();
   }
 
+  // Clears the default country.
+  void ResetDefault();
+
+  // Changes |country_code| to the default country.
+  void SetDefaultCountry(const std::string& country_code);
+
+  // Returns the default country code for this model.
+  std::string GetDefaultCountryCode() const;
+
  private:
+  // Changes |default_index_| to |index| after checking it's sane. Notifies
+  // observers if |default_index_| changed.
+  void SetDefaultIndex(int index);
+
   // The countries to show in the model, including NULL for entries that are
   // not countries (the separator entry).
   ScopedVector<AutofillCountry> countries_;
 
+  // The index of the default country.
+  int default_index_;
+
+  ObserverList<ui::ComboboxModelObserver> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(CountryComboboxModel);
 };
 

chrome/browser/ui/autofill/country_combobox_model_unittest.cc

+// Copyright 2014 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/country_combobox_model.h"
+
+#include "components/autofill/core/browser/autofill_country.h"
+#include "components/autofill/core/browser/test_personal_data_manager.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gmock_mutant.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/models/combobox_model_observer.h"
+
+namespace autofill {
+
+namespace {
+
+const char kTestCountry[] = "AQ";
+
+class MockObserver : public ui::ComboboxModelObserver {
+ public:
+  MOCK_METHOD1(OnComboboxModelChanged, void(ui::ComboboxModel* model));
+};
+
+}  // namespace
+
+TEST(CountryComboboxModel, SetAndResetDefaultCountry) {
+  TestPersonalDataManager manager;
+  CountryComboboxModel model(manager);
+
+  MockObserver observer;
+  model.AddObserver(&observer);
+  EXPECT_CALL(observer, OnComboboxModelChanged(&model)).Times(2);
+
+  std::string default_country = model.GetDefaultCountryCode();
+  ASSERT_NE(default_country, kTestCountry);
+
+  model.SetDefaultCountry(kTestCountry);
+  EXPECT_EQ(kTestCountry, model.GetDefaultCountryCode());
+
+  model.ResetDefault();
+  EXPECT_EQ(default_country, model.GetDefaultCountryCode());
+}
+
+TEST(CountryComboboxModel, RespectsManagerDefaultCountry) {
+  TestPersonalDataManager manager;
+  manager.set_timezone_country_code(kTestCountry);
+
+  CountryComboboxModel model(manager);
+  EXPECT_EQ(kTestCountry, model.GetDefaultCountryCode());
+}
+
+}  // namespace autofill

chrome/browser/ui/cocoa/autofill/autofill_pop_up_button.mm

@@ -71,8 +71,10 @@
 }
 
 - (void)didSelectItem:(id)sender {
-  if (inputDelegate_)
+  if (inputDelegate_) {
+    [inputDelegate_ didChange:self];
     [inputDelegate_ didEndEditing:self];
+  }
 }
 
 @end

chrome/browser/ui/cocoa/autofill/autofill_section_container.mm

@@ -72,9 +72,9 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 
 @interface AutofillSectionContainer ()
 
-// A text field has been edited or activated - inform the delegate that it's
-// time to show a suggestion popup & possibly reset the validity of the input.
-- (void)textfieldEditedOrActivated:(NSControl<AutofillInputField>*)field
+// An input field has been edited or activated - inform the delegate and
+// possibly reset the validity of the input (if it's a textfield).
+- (void)fieldEditedOrActivated:(NSControl<AutofillInputField>*)field
                         edited:(BOOL)edited;
 
 // Convenience method to retrieve a field type via the control's tag.
@@ -104,8 +104,8 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 // Create a button offering input suggestions.
 - (MenuButton*)makeSuggestionButton;
 
-// Create a view with all inputs requested by |delegate_|. Autoreleased.
-- (LayoutView*)makeInputControls;
+// Create a view with all inputs requested by |delegate_| and resets |input_|.
+- (void)makeInputControls;
 
 // Refresh all field icons based on |delegate_| status.
 - (void)updateFieldIcons;
@@ -159,27 +159,8 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 }
 
 - (void)loadView {
-  // Keep a list of weak pointers to DetailInputs.
-  const autofill::DetailInputs& inputs =
-      delegate_->RequestedFieldsForSection(section_);
-
-  // Reverse the order of all the inputs.
-  for (int i = inputs.size() - 1; i >= 0; --i) {
-    detailInputs_.push_back(&(inputs[i]));
-  }
-
-  // Then right the reversal in each row.
-  std::vector<const autofill::DetailInput*>::iterator it;
-  for (it = detailInputs_.begin(); it < detailInputs_.end(); ++it) {
-    std::vector<const autofill::DetailInput*>::iterator start = it;
-    while (it != detailInputs_.end() &&
-           (*it)->length != autofill::DetailInput::LONG) {
-      ++it;
-    }
-    std::reverse(start, it);
-  }
+  [self makeInputControls];
 
-  inputs_.reset([[self makeInputControls] retain]);
   base::string16 labelText = delegate_->LabelForSection(section_);
   label_.reset(
       [[self makeDetailSectionLabel:base::SysUTF16ToNSString(labelText)]
@@ -288,7 +269,7 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 }
 
 - (void)onMouseDown:(NSControl<AutofillInputField>*)field {
-  [self textfieldEditedOrActivated:field edited:NO];
+  [self fieldEditedOrActivated:field edited:NO];
   [validationDelegate_ updateMessageForField:field];
 }
 
@@ -297,7 +278,7 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 }
 
 - (void)didChange:(id)sender {
-  [self textfieldEditedOrActivated:sender edited:YES];
+  [self fieldEditedOrActivated:sender edited:YES];
 }
 
 - (void)didEndEditing:(id)sender {
@@ -419,17 +400,10 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 
 #pragma mark Internal API for AutofillSectionContainer.
 
-- (void)textfieldEditedOrActivated:(NSControl<AutofillInputField>*)field
+- (void)fieldEditedOrActivated:(NSControl<AutofillInputField>*)field
                         edited:(BOOL)edited {
-  AutofillTextField* textfield =
-      base::mac::ObjCCastStrict<AutofillTextField>(field);
-
-  // This only applies to textfields.
-  if (!textfield)
-    return;
-
   autofill::ServerFieldType type = [self fieldTypeForControl:field];
-  base::string16 fieldValue = base::SysNSStringToUTF16([textfield fieldValue]);
+  base::string16 fieldValue = base::SysNSStringToUTF16([field fieldValue]);
 
   // Get the frame rectangle for the designated field, in screen coordinates.
   NSRect textFrameInScreen = [field convertRect:[field bounds] toView:nil];
@@ -449,11 +423,15 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
                                         fieldValue,
                                         edited);
 
-  // If the field is marked as invalid, check if the text is now valid.
-  // Many fields (i.e. CC#) are invalid for most of the duration of editing,
-  // so flagging them as invalid prematurely is not helpful. However,
-  // correcting a minor mistake (i.e. a wrong CC digit) should immediately
-  // result in validation - positive user feedback.
+  AutofillTextField* textfield = base::mac::ObjCCast<AutofillTextField>(field);
+  if (!textfield)
+    return;
+
+  // If the field is marked as invalid, check if the text is now valid. Many
+  // fields (i.e. CC#) are invalid for most of the duration of editing, so
+  // flagging them as invalid prematurely is not helpful. However, correcting a
+  // minor mistake (i.e. a wrong CC digit) should immediately result in
+  // validation - positive user feedback.
   if ([textfield invalid] && edited) {
     base::string16 message = delegate_->InputValidityMessage(section_,
                                                              type,
@@ -485,7 +463,7 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
       return detailInputs_[i];
   }
   // TODO(groby): Needs to be NOTREACHED. Can't, due to the fact that tests
-  // blindly call setFieldValue:forInput:, even for non-existing inputs.
+  // blindly call setFieldValue:forType:, even for non-existing inputs.
   return NULL;
 }
 
@@ -514,6 +492,9 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 }
 
 - (void)updateAndClobber:(BOOL)shouldClobber {
+  if (shouldClobber) {
+    [self makeInputControls];
+  } else {
     const autofill::DetailInputs& updatedInputs =
         delegate_->RequestedFieldsForSection(section_);
 
@@ -522,15 +503,13 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
          ++iter) {
       NSControl<AutofillInputField>* field = [inputs_ viewWithTag:iter->type];
       DCHECK(field);
-
-    if (shouldClobber || [field isDefault]) {
+      if ([field isDefault])
         [field setFieldValue:base::SysUTF16ToNSString(iter->initial_value)];
     }
-    if (shouldClobber)
-      [field setValidityMessage:@""];
+    [self updateFieldIcons];
   }
+
   [self updateEditability];
-  [self updateFieldIcons];
   [self modelChanged];
 }
 
@@ -571,7 +550,38 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 
 // TODO(estade): we should be using Chrome-style constrained window padding
 // values.
-- (LayoutView*)makeInputControls {
+- (void)makeInputControls {
+  if (inputs_) {
+    // When |inputs_| is replaced in response to a country change, there's a
+    // didEndEditing dispatched that segfaults or DCHECKS() as it's operating on
+    // stale input fields. Nil out the input delegate so this doesn't happen.
+    for (NSControl<AutofillInputField>* input in [inputs_ subviews]) {
+      [input setInputDelegate:nil];
+    }
+  }
+
+  detailInputs_.clear();
+
+  // Keep a list of weak pointers to DetailInputs.
+  const autofill::DetailInputs& inputs =
+      delegate_->RequestedFieldsForSection(section_);
+
+  // Reverse the order of all the inputs.
+  for (int i = inputs.size() - 1; i >= 0; --i) {
+    detailInputs_.push_back(&(inputs[i]));
+  }
+
+  // Then right the reversal in each row.
+  std::vector<const autofill::DetailInput*>::iterator it;
+  for (it = detailInputs_.begin(); it < detailInputs_.end(); ++it) {
+    std::vector<const autofill::DetailInput*>::iterator start = it;
+    while (it != detailInputs_.end() &&
+           (*it)->length != autofill::DetailInput::LONG) {
+      ++it;
+    }
+    std::reverse(start, it);
+  }
+
   base::scoped_nsobject<LayoutView> view([[LayoutView alloc] init]);
   [view setLayoutManager:
       scoped_ptr<SimpleGridLayout>(new SimpleGridLayout(view))];
@@ -625,11 +635,11 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
       NSString* tooltipText =
           base::SysUTF16ToNSString(delegate_->TooltipForField(input.type));
       if ([tooltipText length] > 0) {
-        DCHECK(!tooltipController_);
-        DCHECK(!tooltipField_);
+        if (!tooltipController_) {
           tooltipController_.reset(
               [[AutofillTooltipController alloc]
                    initWithArrowLocation:info_bubble::kTopRight]);
+        }
         tooltipField_ = field.get();
         NSImage* icon =
             ui::ResourceBundle::GetSharedInstance().GetNativeImageNamed(
@@ -658,8 +668,15 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
     }
   }
 
+  if (inputs_) {
+    [[self view] replaceSubview:inputs_ with:view];
+    id delegate = [[view_ window] windowController];
+    if ([delegate respondsToSelector:@selector(requestRelayout)])
+      [delegate performSelector:@selector(requestRelayout)];
+  }
+
+  inputs_ = view;
   [self updateFieldIcons];
-  return view.autorelease();
 }
 
 - (void)updateFieldIcons {
@@ -682,7 +699,6 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
 }
 
 - (void)updateEditability {
-
   base::scoped_nsobject<NSMutableArray> controls([[NSMutableArray alloc] init]);
   [self addInputsToArray:controls];
   for (NSControl<AutofillInputField>* control in controls.get()) {
@@ -716,7 +732,7 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
   NSControl<AutofillInputField>* field = [inputs_ viewWithTag:type];
   if (field) {
     [[field window] makeFirstResponder:field];
-    [self textfieldEditedOrActivated:field edited:NO];
+    [self fieldEditedOrActivated:field edited:NO];
   }
 }
 

chrome/browser/ui/views/autofill/autofill_dialog_views.h

@@ -463,9 +463,9 @@ class AutofillDialogViews : public AutofillDialogView,
     SectionContainer* container;
     // The view that allows manual input.
     views::View* manual_input;
-    // The textfields in |manual_input|, tracked by their DetailInput.
+    // The textfields in |manual_input|, tracked by their ServerFieldType.
     TextfieldMap textfields;
-    // The comboboxes in |manual_input|, tracked by their DetailInput.
+    // The comboboxes in |manual_input|, tracked by their ServerFieldType.
     ComboboxMap comboboxes;
     // The view that holds the text of the suggested data. This will be
     // visible IFF |manual_input| is not visible.
@@ -507,10 +507,8 @@ class AutofillDialogViews : public AutofillDialogView,
   // a given section.
   views::View* CreateInputsContainer(DialogSection section);
 
-  // Creates a grid of textfield views for the given section, and stores them
-  // in the appropriate DetailsGroup. The top level View in the hierarchy is
-  // returned.
-  views::View* InitInputsView(DialogSection section);
+  // Creates a grid of inputs for the given section.
+  void InitInputsView(DialogSection section);
 
   // Changes the function of the whole dialog. Currently this can show a loading
   // shield, an embedded sign in web view, or the more typical detail input mode
@@ -533,6 +531,9 @@ class AutofillDialogViews : public AutofillDialogView,
   // Returns NULL if no DetailsGroup was found.
   DetailsGroup* GroupForView(views::View* view);
 
+  // Erases all views in |group| from |validity_map_|.
+  void EraseInvalidViewsInGroup(const DetailsGroup* group);
+
   // Explicitly focuses the initially focusable view.
   void FocusInitialView();
 
@@ -563,11 +564,13 @@ class AutofillDialogViews : public AutofillDialogView,
   // ones and returns true if all were valid.
   bool ValidateForm();
 
-  // When an input textfield is edited (its contents change) or activated
-  // (clicked while focused), this function will inform the delegate that it's
-  // time to show a suggestion popup and possibly reset the validity state of
-  // the input.
-  void TextfieldEditedOrActivated(views::Textfield* textfield, bool was_edit);
+  // When an input is edited (its contents change) or activated (clicked while
+  // focused), this function will inform the delegate to take the appropriate
+  // action (textfields may show a suggestion popup, comboboxes may rebuild the
+  // section inputs). May also reset the validity state of the input.
+  void InputEditedOrActivated(ServerFieldType type,
+                              const gfx::Rect& bounds,
+                              bool was_edit);
 
   // Updates the views in the button strip.
   void UpdateButtonStripExtraView();
@@ -577,11 +580,17 @@ class AutofillDialogViews : public AutofillDialogView,
   void DoContentsPreferredSizeChanged();
 
   // Gets the textfield view that is shown for the given |type| or NULL.
-  views::Textfield* TextfieldForType(ServerFieldType type);
+  DecoratedTextfield* TextfieldForType(ServerFieldType type);
+
+  // Returns the associated ServerFieldType for |textfield|.
+  ServerFieldType TypeForTextfield(const views::Textfield* textfield);
 
   // Gets the combobox view that is shown for the given |type|, or NULL.
   views::Combobox* ComboboxForType(ServerFieldType type);
 
+  // Returns the associated ServerFieldType for |combobox|.
+  ServerFieldType TypeForCombobox(const views::Combobox* combobox) const;
+
   // Called when the details container changes in size or position.
   void DetailsContainerBoundsChanged();
 

chrome/chrome_tests_unit.gypi

@@ -1432,6 +1432,7 @@
         'browser/ui/autofill/autofill_dialog_models_unittest.cc',
         'browser/ui/autofill/autofill_dialog_types_unittest.cc',
         'browser/ui/autofill/autofill_popup_controller_unittest.cc',
+        'browser/ui/autofill/country_combobox_model_unittest.cc',
         'browser/ui/autofill/data_model_wrapper_unittest.cc',
         'browser/ui/autofill/generated_credit_card_bubble_controller_unittest.cc',
         'browser/ui/autofill/mock_autofill_dialog_view_delegate.cc',
@@ -2553,6 +2554,7 @@
             'browser/tab_contents/language_state_unittest.cc',
             'browser/tab_contents/render_view_context_menu_unittest.cc',
             'browser/ui/autofill/autofill_dialog_controller_unittest.cc',
+            'browser/ui/autofill/country_combobox_model_unittest.cc',
             'browser/ui/browser_instant_controller_unittest.cc',
             'browser/ui/bookmarks/bookmark_bubble_sign_in_delegate_unittest.cc',
             'browser/ui/bookmarks/bookmark_context_menu_controller_unittest.cc',