Change AutofillDialogControllerImpl::input_showing_popup_ -> popup_input_type_

so that rebuilding detail inputs on Autofill popup acceptance doesn't crash.
This also changes the public AutofillDialogView API to accept ServerFieldTypes
intead of DetailInputs in FillSection() and HitTestInput().

NOTE: |input.type| was the only thing used by all callers anyways, so rather than
dance around resetting a potentially dead pointer, I'd rather change to server
types.

BUG=331544
R=estade@chromium.org,groby@chromium.org

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

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

13 files changed, 51 insertions, 60 deletions

diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
index 1cd3bf0..f4ec293 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_browsertest.cc
@@ -166,7 +166,7 @@ class TestAutofillDialogController : public AutofillDialogControllerImpl {
 
   // Increase visibility for testing.
   using AutofillDialogControllerImpl::view;
-  using AutofillDialogControllerImpl::input_showing_popup;
+  using AutofillDialogControllerImpl::popup_input_type;
 
   MOCK_METHOD0(LoadRiskFingerprintData, void());
 
@@ -563,7 +563,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest, FillInputFromAutofill) {
                                value.substr(0, value.size() / 2));
   view->ActivateInput(triggering_input);
 
-  ASSERT_EQ(&triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(triggering_input.type, controller()->popup_input_type());
   controller()->DidAcceptSuggestion(base::string16(), 0);
 
   // All inputs should be filled.
@@ -590,7 +590,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest, FillInputFromAutofill) {
   view->SetTextContentsOfInput(triggering_input,
                                value.substr(0, value.size() / 2));
   view->ActivateInput(triggering_input);
-  ASSERT_EQ(&triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(triggering_input.type, controller()->popup_input_type());
   controller()->DidAcceptSuggestion(base::string16(), 0);
 
   for (size_t i = 0; i < inputs.size(); ++i) {
@@ -616,7 +616,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest,
                                value.substr(0, value.size() / 2));
   view->ActivateInput(triggering_input);
 
-  ASSERT_EQ(&triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(triggering_input.type, controller()->popup_input_type());
   controller()->DidAcceptSuggestion(base::string16(), 0);
 
   // All inputs should be filled.
@@ -649,7 +649,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest,
   view->SetTextContentsOfInput(triggering_input,
                                value.substr(0, value.size() / 2));
   view->ActivateInput(triggering_input);
-  ASSERT_EQ(&triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(triggering_input.type, controller()->popup_input_type());
   controller()->DidAcceptSuggestion(base::string16(), 0);
 
   for (size_t i = 0; i < inputs.size(); ++i) {
@@ -681,7 +681,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest,
   TestableAutofillDialogView* view = controller()->GetTestableView();
   view->ActivateInput(triggering_input);
 
-  ASSERT_EQ(&triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(triggering_input.type, controller()->popup_input_type());
 
   // Choose the variant suggestion.
   controller()->DidAcceptSuggestion(base::string16(), 1);
@@ -729,7 +729,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest,
                                value.substr(0, value.size() / 2));
   view->ActivateInput(triggering_input);
 
-  ASSERT_EQ(&triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(triggering_input.type, controller()->popup_input_type());
   controller()->DidAcceptSuggestion(base::string16(), 0);
 
   // All inputs should be filled.
@@ -745,7 +745,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest,
   view->SetTextContentsOfInput(triggering_input,
                                value.substr(0, value.size() / 2));
   view->ActivateInput(triggering_input);
-  ASSERT_EQ(&triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(triggering_input.type, controller()->popup_input_type());
   controller()->DidAcceptSuggestion(base::string16(), 0);
 
   AutofillCreditCardWrapper wrapper2(&card2);
@@ -773,7 +773,7 @@ IN_PROC_BROWSER_TEST_F(AutofillDialogControllerTest,
                                value.substr(0, value.size() / 2));
   view->ActivateInput(billing_triggering_input);
 
-  ASSERT_EQ(&billing_triggering_input, controller()->input_showing_popup());
+  ASSERT_EQ(billing_triggering_input.type, controller()->popup_input_type());
   controller()->DidAcceptSuggestion(base::string16(), 0);
 
   for (size_t i = 0; i < inputs.size(); ++i) {
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
index ee13efc..5e368da 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.cc
@@ -1897,18 +1897,8 @@ void AutofillDialogControllerImpl::UserEditedOrActivatedInput(
     return;
   }
 
-  // |input_showing_popup_| must be set before calling |Show()|.
-  const DetailInputs& inputs = RequestedFieldsForSection(section);
-  for (DetailInputs::const_iterator iter = inputs.begin();
-       iter != inputs.end(); ++iter) {
-    if (iter->type == type) {
-      input_showing_popup_ = &(*iter);
-      break;
-    }
-  }
-
-  if (!input_showing_popup_)
-    return;
+  // |popup_input_type_| must be set before calling |Show()|.
+  popup_input_type_ = type;
 
   // TODO(estade): do we need separators and control rows like 'Clear
   // Form'?
@@ -1937,7 +1927,7 @@ void AutofillDialogControllerImpl::FocusMoved() {
 }
 
 bool AutofillDialogControllerImpl::ShouldShowErrorBubble() const {
-  return !input_showing_popup_;
+  return popup_input_type_ == UNKNOWN_TYPE;
 }
 
 void AutofillDialogControllerImpl::ViewClosed() {
@@ -2142,8 +2132,10 @@ void AutofillDialogControllerImpl::OnPopupHidden() {}
 
 bool AutofillDialogControllerImpl::ShouldRepostEvent(
     const ui::MouseEvent& event) {
-  // If the event would be reposted inside |input_showing_popup_|, just ignore.
-  return !view_->HitTestInput(*input_showing_popup_, event.location());
+  DCHECK_NE(UNKNOWN_TYPE, popup_input_type_);
+  // If the event would be reposted inside the input showing an Autofill popup,
+  // just ignore.
+  return !view_->HitTestInput(popup_input_type_, event.location());
 }
 
 void AutofillDialogControllerImpl::DidSelectSuggestion(int identifier) {
@@ -2157,20 +2149,20 @@ void AutofillDialogControllerImpl::DidAcceptSuggestion(
   const PersonalDataManager::GUIDPair& pair = popup_guids_[identifier];
 
   scoped_ptr<DataModelWrapper> wrapper;
-  if (common::IsCreditCardType(input_showing_popup_->type)) {
+  if (common::IsCreditCardType(popup_input_type_)) {
     wrapper.reset(new AutofillCreditCardWrapper(
         GetManager()->GetCreditCardByGUID(pair.first)));
   } else {
     wrapper.reset(new AutofillProfileWrapper(
         GetManager()->GetProfileByGUID(pair.first),
-        AutofillType(input_showing_popup_->type),
+        AutofillType(popup_input_type_),
         pair.second));
   }
 
   for (size_t i = SECTION_MIN; i <= SECTION_MAX; ++i) {
     DialogSection section = static_cast<DialogSection>(i);
     wrapper->FillInputs(MutableRequestedFieldsForSection(section));
-    view_->FillSection(section, *input_showing_popup_);
+    view_->FillSection(section, popup_input_type_);
   }
 
   GetMetricLogger().LogDialogPopupEvent(
@@ -2536,7 +2528,7 @@ AutofillDialogControllerImpl::AutofillDialogControllerImpl(
       suggested_cc_billing_(this),
       suggested_shipping_(this),
       cares_about_shipping_(true),
-      input_showing_popup_(NULL),
+      popup_input_type_(UNKNOWN_TYPE),
       weak_ptr_factory_(this),
       waiting_for_explicit_sign_in_response_(false),
       has_accepted_legal_documents_(false),
@@ -3003,7 +2995,7 @@ DetailInputs* AutofillDialogControllerImpl::MutableRequestedFieldsForSection(
 void AutofillDialogControllerImpl::HidePopup() {
   if (popup_controller_.get())
     popup_controller_->Hide();
-  input_showing_popup_ = NULL;
+  popup_input_type_ = UNKNOWN_TYPE;
 }
 
 void AutofillDialogControllerImpl::SetEditingExistingData(
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
index ab279d15..14fbef0 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_impl.h
@@ -241,8 +241,8 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   // Exposed for testing.
   AutofillDialogView* view() { return view_.get(); }
   virtual AutofillDialogView* CreateView();
-  const DetailInput* input_showing_popup() const {
-    return input_showing_popup_;
+  ServerFieldType popup_input_type() const {
+    return popup_input_type_;
   }
 
   // Returns the PersonalDataManager for |profile_|.
@@ -672,9 +672,8 @@ class AutofillDialogControllerImpl : public AutofillDialogViewDelegate,
   // they're manually filling the dialog).
   base::WeakPtr<AutofillPopupControllerImpl> popup_controller_;
 
-  // The input for which |popup_controller_| is currently showing a popup
-  // (if any).
-  const DetailInput* input_showing_popup_;
+  // The type of the visible Autofill popup input (or UNKNOWN_TYPE if none).
+  ServerFieldType popup_input_type_;
 
   scoped_ptr<AutofillDialogView> view_;
 
diff --git a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
index 9d3192e..3940620 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
+++ b/chrome/browser/ui/autofill/autofill_dialog_controller_unittest.cc
@@ -178,7 +178,7 @@ class TestAutofillDialogView : public AutofillDialogView {
   }
 
   virtual void FillSection(DialogSection section,
-                           const DetailInput& originating_input) OVERRIDE {};
+                           ServerFieldType originating_type) OVERRIDE {}
   virtual void GetUserInput(DialogSection section, FieldValueMap* output)
       OVERRIDE {
     *output = outputs_[section];
@@ -188,7 +188,7 @@ class TestAutofillDialogView : public AutofillDialogView {
   }
 
   virtual base::string16 GetCvc() OVERRIDE { return base::string16(); }
-  virtual bool HitTestInput(const DetailInput& input,
+  virtual bool HitTestInput(ServerFieldType type,
                             const gfx::Point& screen_point) OVERRIDE {
     return false;
   }
diff --git a/chrome/browser/ui/autofill/autofill_dialog_view.h b/chrome/browser/ui/autofill/autofill_dialog_view.h
index 39ab3be..d074769 100644
--- a/chrome/browser/ui/autofill/autofill_dialog_view.h
+++ b/chrome/browser/ui/autofill/autofill_dialog_view.h
@@ -71,7 +71,7 @@ class AutofillDialogView {
   // Fills the given section with Autofill data that was triggered by a user
   // interaction with |originating_input|.
   virtual void FillSection(DialogSection section,
-                           const DetailInput& originating_input) = 0;
+                           ServerFieldType originating_type) = 0;
 
   // Fills |output| with data the user manually input.
   virtual void GetUserInput(DialogSection section, FieldValueMap* output) = 0;
@@ -81,8 +81,8 @@ class AutofillDialogView {
   // relevant.
   virtual base::string16 GetCvc() = 0;
 
-  // Whether or not |point| is within |input|'s bounds.
-  virtual bool HitTestInput(const DetailInput& input,
+  // Whether or not |point| is within the bounds of an input of |type|.
+  virtual bool HitTestInput(ServerFieldType type,
                             const gfx::Point& screen_point) = 0;
 
   // Returns true if new or edited autofill details should be saved.
diff --git a/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.h b/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.h
index 6f1442e..c44e88e 100644
--- a/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.h
+++ b/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.h
@@ -47,11 +47,11 @@ class AutofillDialogCocoa : public AutofillDialogView,
   virtual void UpdateSection(DialogSection section) OVERRIDE;
   virtual void UpdateErrorBubble() OVERRIDE;
   virtual void FillSection(DialogSection section,
-                           const DetailInput& originating_input) OVERRIDE;
+                           ServerFieldType originating_type) OVERRIDE;
   virtual void GetUserInput(DialogSection section,
                             FieldValueMap* output) OVERRIDE;
   virtual base::string16 GetCvc() OVERRIDE;
-  virtual bool HitTestInput(const DetailInput& input,
+  virtual bool HitTestInput(ServerFieldType type,
                             const gfx::Point& screen_point) OVERRIDE;
   virtual bool SaveDetailsLocally() OVERRIDE;
   virtual const content::NavigationController* ShowSignIn() OVERRIDE;
diff --git a/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm b/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm
index 582c031..cf1e7c4 100644
--- a/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm
+++ b/chrome/browser/ui/cocoa/autofill/autofill_dialog_cocoa.mm
@@ -95,8 +95,8 @@ void AutofillDialogCocoa::UpdateSection(DialogSection section) {
 }
 
 void AutofillDialogCocoa::FillSection(DialogSection section,
-                                      const DetailInput& originating_input) {
-  [sheet_delegate_ fillSection:section forInput:originating_input];
+                                      ServerFieldType originating_type) {
+  [sheet_delegate_ fillSection:section forType:originating_type];
 }
 
 void AutofillDialogCocoa::GetUserInput(DialogSection section,
@@ -108,7 +108,7 @@ base::string16 AutofillDialogCocoa::GetCvc() {
   return base::SysNSStringToUTF16([sheet_delegate_ getCvc]);
 }
 
-bool AutofillDialogCocoa::HitTestInput(const DetailInput& input,
+bool AutofillDialogCocoa::HitTestInput(ServerFieldType type,
                                        const gfx::Point& screen_point) {
   // TODO(dbeam): implement.
   return false;
diff --git a/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.h b/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.h
index d3d9004..58a829e 100644
--- a/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.h
+++ b/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.h
@@ -38,7 +38,7 @@ class AutofillDialogCocoa;
 - (void)updateSection:(autofill::DialogSection)section;
 - (void)updateForErrors;
 - (void)fillSection:(autofill::DialogSection)section
-           forInput:(const autofill::DetailInput&)input;
+           forType:(const autofill::ServerFieldType)type;
 - (void)getInputs:(autofill::FieldValueMap*)outputs
        forSection:(autofill::DialogSection)section;
 - (NSString*)getCvc;
diff --git a/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.mm b/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.mm
index 1a32035..716bb9a 100644
--- a/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.mm
+++ b/chrome/browser/ui/cocoa/autofill/autofill_dialog_window_controller.mm
@@ -398,8 +398,8 @@ const CGFloat kMinimumContentsHeight = 101;
 }
 
 - (void)fillSection:(autofill::DialogSection)section
-           forInput:(const autofill::DetailInput&)input {
-  [[mainContainer_ sectionForId:section] fillForInput:input];
+            forType:(autofill::ServerFieldType)type {
+  [[mainContainer_ sectionForId:section] fillForType:type];
   [mainContainer_ updateSaveInChrome];
 }
 
diff --git a/chrome/browser/ui/cocoa/autofill/autofill_section_container.h b/chrome/browser/ui/cocoa/autofill/autofill_section_container.h
index a81bbf2..f524993 100644
--- a/chrome/browser/ui/cocoa/autofill/autofill_section_container.h
+++ b/chrome/browser/ui/cocoa/autofill/autofill_section_container.h
@@ -92,8 +92,8 @@ class AutofillDialogViewDelegate;
 - (void)update;
 
 // Fills the section with Autofill data that was triggered by a user
-// interaction with the originating |input|.
-- (void)fillForInput:(const autofill::DetailInput&)input;
+// interaction with the originating |type|.
+- (void)fillForType:(const autofill::ServerFieldType)type;
 
 // Validate this section. Validation rules depend on |validationType|.
 - (BOOL)validateFor:(autofill::ValidationType)validationType;
diff --git a/chrome/browser/ui/cocoa/autofill/autofill_section_container.mm b/chrome/browser/ui/cocoa/autofill/autofill_section_container.mm
index 31d0bb8..2afcb64 100644
--- a/chrome/browser/ui/cocoa/autofill/autofill_section_container.mm
+++ b/chrome/browser/ui/cocoa/autofill/autofill_section_container.mm
@@ -345,13 +345,13 @@ bool ShouldOverwriteComboboxes(autofill::DialogSection section,
   [self updateAndClobber:YES];
 }
 
-- (void)fillForInput:(const autofill::DetailInput&)input {
+- (void)fillForType:(const autofill::ServerFieldType)type {
   // Make sure to overwrite the originating input if it is a text field.
   AutofillTextField* field =
-      base::mac::ObjCCast<AutofillTextField>([inputs_ viewWithTag:input.type]);
+      base::mac::ObjCCast<AutofillTextField>([inputs_ viewWithTag:type]);
   [field setFieldValue:@""];
 
-  if (ShouldOverwriteComboboxes(section_, input.type)) {
+  if (ShouldOverwriteComboboxes(section_, type)) {
     for (NSControl* control in [inputs_ subviews]) {
       AutofillPopUpButton* popup =
           base::mac::ObjCCast<AutofillPopUpButton>(control);
diff --git a/chrome/browser/ui/views/autofill/autofill_dialog_views.cc b/chrome/browser/ui/views/autofill/autofill_dialog_views.cc
index 50571b7..787694a 100644
--- a/chrome/browser/ui/views/autofill/autofill_dialog_views.cc
+++ b/chrome/browser/ui/views/autofill/autofill_dialog_views.cc
@@ -1315,11 +1315,11 @@ void AutofillDialogViews::UpdateErrorBubble() {
 }
 
 void AutofillDialogViews::FillSection(DialogSection section,
-                                      const DetailInput& originating_input) {
+                                      ServerFieldType originating_type) {
   DetailsGroup* group = GroupForSection(section);
   // Make sure to overwrite the originating input.
   TextfieldMap::iterator text_mapping =
-      group->textfields.find(originating_input.type);
+      group->textfields.find(originating_type);
   if (text_mapping != group->textfields.end())
     text_mapping->second->SetText(base::string16());
 
@@ -1327,7 +1327,7 @@ void AutofillDialogViews::FillSection(DialogSection section,
   // CC comboboxes (even if they already have something in them). If the
   // Autofill data comes from an AutofillProfile, leave the comboboxes alone.
   if (section == GetCreditCardSection() &&
-      AutofillType(originating_input.type).group() == CREDIT_CARD) {
+      AutofillType(originating_type).group() == CREDIT_CARD) {
     for (ComboboxMap::const_iterator it = group->comboboxes.begin();
          it != group->comboboxes.end(); ++it) {
       if (AutofillType(it->first).group() == CREDIT_CARD)
@@ -1357,11 +1357,11 @@ base::string16 AutofillDialogViews::GetCvc() {
       decorated_textfield()->text();
 }
 
-bool AutofillDialogViews::HitTestInput(const DetailInput& input,
+bool AutofillDialogViews::HitTestInput(ServerFieldType type,
                                        const gfx::Point& screen_point) {
-  views::View* view = TextfieldForType(input.type);
+  views::View* view = TextfieldForType(type);
   if (!view)
-    view = ComboboxForType(input.type);
+    view = ComboboxForType(type);
 
   if (view) {
     gfx::Point target_point(screen_point);
diff --git a/chrome/browser/ui/views/autofill/autofill_dialog_views.h b/chrome/browser/ui/views/autofill/autofill_dialog_views.h
index 1e5fb15..daf1a29 100644
--- a/chrome/browser/ui/views/autofill/autofill_dialog_views.h
+++ b/chrome/browser/ui/views/autofill/autofill_dialog_views.h
@@ -87,11 +87,11 @@ class AutofillDialogViews : public AutofillDialogView,
   virtual void UpdateSection(DialogSection section) OVERRIDE;
   virtual void UpdateErrorBubble() OVERRIDE;
   virtual void FillSection(DialogSection section,
-                           const DetailInput& originating_input) OVERRIDE;
+                           ServerFieldType originating_type) OVERRIDE;
   virtual void GetUserInput(DialogSection section,
                             FieldValueMap* output) OVERRIDE;
   virtual base::string16 GetCvc() OVERRIDE;
-  virtual bool HitTestInput(const DetailInput& input,
+  virtual bool HitTestInput(ServerFieldType type,
                             const gfx::Point& screen_point) OVERRIDE;
   virtual bool SaveDetailsLocally() OVERRIDE;
   virtual const content::NavigationController* ShowSignIn() OVERRIDE;