diff options
Diffstat (limited to 'chrome/browser/ui/autofill')
3 files changed, 115 insertions, 20 deletions
diff --git a/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc b/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc index ae01eb0..2d74b87 100644 --- a/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc +++ b/chrome/browser/ui/autofill/autofill_popup_controller_impl.cc @@ -60,6 +60,27 @@ const DataResource kDataResources[] = { } // end namespace +// static +AutofillPopupControllerImpl* AutofillPopupControllerImpl::GetOrCreate( + AutofillPopupControllerImpl* previous, + AutofillPopupDelegate* delegate, + gfx::NativeView container_view, + const gfx::Rect& element_bounds) { + DCHECK(!previous || previous->delegate_ == delegate); + + if (previous && + previous->container_view() == container_view && + previous->element_bounds() == element_bounds) { + return previous; + } + + if (previous) + previous->Hide(); + + return new AutofillPopupControllerImpl( + delegate, container_view, element_bounds); +} + AutofillPopupControllerImpl::AutofillPopupControllerImpl( AutofillPopupDelegate* delegate, gfx::NativeView container_view, @@ -69,7 +90,8 @@ AutofillPopupControllerImpl::AutofillPopupControllerImpl( container_view_(container_view), element_bounds_(element_bounds), selected_line_(kNoSelection), - delete_icon_hovered_(false) { + delete_icon_hovered_(false), + is_hiding_(false) { #if !defined(OS_ANDROID) label_font_ = value_font_.DeriveFont(kLabelFontSizeDelta); #endif @@ -111,15 +133,8 @@ void AutofillPopupControllerImpl::Show( } void AutofillPopupControllerImpl::Hide() { - if (view_) - view_->Hide(); - else - delete this; -} - -void AutofillPopupControllerImpl::DelegateDestroyed() { delegate_ = NULL; - Hide(); + HideInternal(); } void AutofillPopupControllerImpl::ViewDestroyed() { @@ -316,7 +331,7 @@ bool AutofillPopupControllerImpl::HandleKeyPressEvent( SetSelectedLine(autofill_values().size() - 1); return true; case ui::VKEY_ESCAPE: - Hide(); + HideInternal(); return true; case ui::VKEY_DELETE: return (event.modifiers & content::NativeWebKeyboardEvent::ShiftKey) && @@ -328,6 +343,17 @@ bool AutofillPopupControllerImpl::HandleKeyPressEvent( } } +void AutofillPopupControllerImpl::HideInternal() { + if (is_hiding_) + return; + is_hiding_ = true; + + if (view_) + view_->Hide(); + else + delete this; +} + void AutofillPopupControllerImpl::SetSelectedLine(int selected_line) { if (selected_line_ == selected_line) return; @@ -422,7 +448,7 @@ bool AutofillPopupControllerImpl::RemoveSelectedLine() { delegate_->ClearPreviewedForm(); UpdateBoundsAndRedrawPopup(); } else { - Hide(); + HideInternal(); } return true; diff --git a/chrome/browser/ui/autofill/autofill_popup_controller_impl.h b/chrome/browser/ui/autofill/autofill_popup_controller_impl.h index 54cda77..4de5c2f 100644 --- a/chrome/browser/ui/autofill/autofill_popup_controller_impl.h +++ b/chrome/browser/ui/autofill/autofill_popup_controller_impl.h @@ -25,9 +25,14 @@ class KeyEvent; class AutofillPopupControllerImpl : public AutofillPopupController, public content::KeyboardListener { public: - AutofillPopupControllerImpl(AutofillPopupDelegate* delegate, - gfx::NativeView container_view, - const gfx::Rect& element_bounds); + // Creates a new |AutofillPopupControllerImpl|, or reuses |previous| if + // the construction arguments are the same. |previous| may be invalidated by + // this call. + static AutofillPopupControllerImpl* GetOrCreate( + AutofillPopupControllerImpl* previous, + AutofillPopupDelegate* delegate, + gfx::NativeView container_view, + const gfx::Rect& element_bounds); // Shows the popup, or updates the existing popup with the given values. void Show(const std::vector<string16>& autofill_values, @@ -35,15 +40,17 @@ class AutofillPopupControllerImpl : public AutofillPopupController, const std::vector<string16>& autofill_icons, const std::vector<int>& autofill_unique_ids); - // Hides the popup and destroys the controller. - void Hide(); - - // Called when |delegate_| is no longer valid. - void DelegateDestroyed(); + // Hides the popup and destroys the controller. This also invalidates + // |delegate_|. Virtual for testing. + virtual void Hide(); protected: FRIEND_TEST_ALL_PREFIXES(AutofillExternalDelegateBrowserTest, CloseWidgetAndNoLeaking); + + AutofillPopupControllerImpl(AutofillPopupDelegate* delegate, + gfx::NativeView container_view, + const gfx::Rect& element_bounds); virtual ~AutofillPopupControllerImpl(); // AutofillPopupController implementation. @@ -82,6 +89,10 @@ class AutofillPopupControllerImpl : public AutofillPopupController, virtual bool HandleKeyPressEvent( const content::NativeWebKeyboardEvent& event) OVERRIDE; + // Like Hide(), but doesn't invalidate |delegate_| (the delegate will still + // be informed of destruction). + void HideInternal(); + // Change which line is currently selected by the user. void SetSelectedLine(int selected_line); @@ -145,6 +156,9 @@ class AutofillPopupControllerImpl : public AutofillPopupController, // Used to indicate if the delete icon within a row is currently selected. bool delete_icon_hovered_; + + // True if |HideInternal| has already been called. + bool is_hiding_; }; #endif // CHROME_BROWSER_UI_AUTOFILL_AUTOFILL_POPUP_CONTROLLER_IMPL_H_ diff --git a/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc b/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc index f90fd19..efc5e4d 100644 --- a/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc +++ b/chrome/browser/ui/autofill/autofill_popup_controller_unittest.cc @@ -57,9 +57,13 @@ class TestAutofillPopupController : public AutofillPopupControllerImpl { bool RemoveSelectedLine() { return AutofillPopupControllerImpl::RemoveSelectedLine(); } + void DoHide() { + AutofillPopupControllerImpl::Hide(); + } MOCK_METHOD1(InvalidateRow, void(size_t)); MOCK_METHOD0(UpdateBoundsAndRedrawPopup, void()); + MOCK_METHOD0(Hide, void()); private: virtual void ShowView() OVERRIDE {} @@ -77,7 +81,7 @@ class AutofillPopupControllerUnitTest : public ::testing::Test { // This will make sure the controller and the view (if any) are both // cleaned up. if (autofill_popup_controller_) - autofill_popup_controller_->Hide(); + autofill_popup_controller_->DoHide(); } AutofillPopupController* popup_controller() { @@ -209,3 +213,54 @@ TEST_F(AutofillPopupControllerUnitTest, SkipSeparator) { autofill_popup_controller_->SelectPreviousLine(); EXPECT_EQ(0, autofill_popup_controller_->selected_line()); } + +TEST_F(AutofillPopupControllerUnitTest, GetOrCreate) { + MockAutofillExternalDelegate delegate; + + AutofillPopupControllerImpl* controller = + AutofillPopupControllerImpl::GetOrCreate( + NULL, + &delegate, + NULL, + gfx::Rect()); + EXPECT_TRUE(controller); + + // This should not inform |delegate| of its destruction. + EXPECT_CALL(delegate, ControllerDestroyed()).Times(0); + controller->Hide(); + + controller = + AutofillPopupControllerImpl::GetOrCreate( + NULL, + &delegate, + NULL, + gfx::Rect()); + EXPECT_TRUE(controller); + AutofillPopupControllerImpl* controller2 = + AutofillPopupControllerImpl::GetOrCreate( + controller, + &delegate, + NULL, + gfx::Rect()); + EXPECT_EQ(controller, controller2); + controller->Hide(); + + TestAutofillPopupController* test_controller = + new TestAutofillPopupController(&delegate); + EXPECT_CALL(*test_controller, Hide()); + + gfx::Rect bounds(0, 0, 1, 2); + AutofillPopupControllerImpl* controller3 = + AutofillPopupControllerImpl::GetOrCreate( + test_controller, + &delegate, + NULL, + bounds); + EXPECT_EQ( + bounds, + static_cast<AutofillPopupController*>(controller3)->element_bounds()); + controller3->Hide(); + + EXPECT_CALL(delegate, ControllerDestroyed()); + delete test_controller; +} |