Move the UI related code from AutofillExternalDelegate to AutofillManagerDelegate

chrome/browser/autofill/autofill_external_delegate_browsertest.cc

Index: chrome/browser/autofill/autofill_external_delegate_browsertest.cc
diff --git a/chrome/browser/autofill/autofill_external_delegate_browsertest.cc b/chrome/browser/autofill/autofill_external_delegate_browsertest.cc
index 0c2863a4cf54d235836cd5693e5d36f51d6dbe0a..e503e704de4cb43a5b1236839f813a6af066f60a 100644
--- a/chrome/browser/autofill/autofill_external_delegate_browsertest.cc
+++ b/chrome/browser/autofill/autofill_external_delegate_browsertest.cc
@@ -5,13 +5,13 @@
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/autofill/autofill_manager.h"
 #include "chrome/browser/autofill/test_autofill_external_delegate.h"
-#include "chrome/browser/ui/autofill/autofill_popup_controller_impl.h"
-#include "chrome/browser/ui/autofill/autofill_popup_view.h"
+#include "chrome/browser/autofill/test_autofill_manager_delegate.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_tabstrip.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/testing_pref_service_syncable.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -19,51 +19,52 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/test_utils.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/rect.h"
 
 namespace {
 
-class MockAutofillExternalDelegate : public AutofillExternalDelegate {
+class MockAutofillManagerDelegate
+    : public autofill::TestAutofillManagerDelegate {
  public:
-  explicit MockAutofillExternalDelegate(content::WebContents* web_contents)
-      : AutofillExternalDelegate(
-            web_contents,
-            AutofillManager::FromWebContents(web_contents)),
-        popup_hidden_(true) {}
-  ~MockAutofillExternalDelegate() {}
+  virtual PrefService* GetPrefs() { return &prefs_; }
 
-  virtual void DidSelectSuggestion(int unique_id) OVERRIDE {}
-
-  virtual void ClearPreviewedForm() OVERRIDE {}
-
-  AutofillPopupControllerImpl* GetController() {
-    return controller();
-  }
-
-  virtual void ApplyAutofillSuggestions(
-      const std::vector<string16>& autofill_values,
-      const std::vector<string16>& autofill_labels,
-      const std::vector<string16>& autofill_icons,
-      const std::vector<int>& autofill_unique_ids)  OVERRIDE {
-    popup_hidden_ = false;
-
-    AutofillExternalDelegate::ApplyAutofillSuggestions(autofill_values,
-                                                       autofill_labels,
-                                                       autofill_icons,
-                                                       autofill_unique_ids);
+  PrefRegistrySyncable* GetPrefRegistry() {
+    return prefs_.registry();
   }
 
-  virtual void HideAutofillPopup() OVERRIDE {
-    popup_hidden_ = true;
+  MOCK_METHOD6(ShowAutofillPopup,
+               void(const gfx::RectF& element_bounds,
+                    const std::vector<string16>& values,
+                    const std::vector<string16>& labels,
+                    const std::vector<string16>& icons,
+                    const std::vector<int>& identifiers,
+                    AutofillPopupDelegate* delegate));
 
-    AutofillExternalDelegate::HideAutofillPopup();
-  }
+  MOCK_METHOD0(HideAutofillPopup, void());
 
-  bool popup_hidden() const { return popup_hidden_; }
+ private:
+  TestingPrefServiceSyncable prefs_;
+};
 
+// Subclass AutofillManager so we can create AutofillManager instance.
+class TestAutofillManager : public AutofillManager {
+ public:
+  TestAutofillManager(content::WebContents* web_contents,
+                      autofill::AutofillManagerDelegate* delegate)
+      : AutofillManager(web_contents, delegate) {}
+  virtual ~TestAutofillManager() {}

[Ilya Sherman, 2013/02/26 22:55:51]

nit: Please leave a blank line below this one.

[kaiwang, 2013/02/27 00:00:10]

Done.

  private:
-  bool popup_hidden_;
+  DISALLOW_COPY_AND_ASSIGN(TestAutofillManager);
+};
+
+class TestAutofillExternalDelegate : public AutofillExternalDelegate {
+ public:
+  TestAutofillExternalDelegate(content::WebContents* web_contents,
+                               AutofillManager* autofill_manager)
+      : AutofillExternalDelegate(web_contents, autofill_manager) {}
+  ~TestAutofillExternalDelegate() {}
 };
 
 }  // namespace
@@ -80,8 +81,13 @@ class AutofillExternalDelegateBrowserTest
     ASSERT_TRUE(web_contents_ != NULL);
     Observe(web_contents_);
 
+    AutofillManager::RegisterUserPrefs(manager_delegate_.GetPrefRegistry());
+
+    autofill_manager_.reset(
+        new TestAutofillManager(web_contents_, &manager_delegate_));
     autofill_external_delegate_.reset(
-        new MockAutofillExternalDelegate(web_contents_));
+        new TestAutofillExternalDelegate(web_contents_,
+                                         autofill_manager_.get()));
   }
 
   // Normally the WebContents will automatically delete the delegate, but here
@@ -90,32 +96,39 @@ class AutofillExternalDelegateBrowserTest
       OVERRIDE {
     DCHECK_EQ(web_contents_, web_contents);
     autofill_external_delegate_.reset();
+    autofill_manager_.reset();
   }
 
  protected:
   content::WebContents* web_contents_;
-  scoped_ptr<MockAutofillExternalDelegate> autofill_external_delegate_;
+
+  testing::NiceMock<MockAutofillManagerDelegate> manager_delegate_;
+  scoped_ptr<TestAutofillManager> autofill_manager_;
+  scoped_ptr<TestAutofillExternalDelegate> autofill_external_delegate_;
 };
 
 IN_PROC_BROWSER_TEST_F(AutofillExternalDelegateBrowserTest,
                        SwitchTabAndHideAutofillPopup) {
   autofill::GenerateTestAutofillPopup(autofill_external_delegate_.get());
 
+  // Both content::NOTIFICATION_NAV_ENTRY_COMMITTED and
+  // content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED will be triggered.
+  // So HideAutofillPopup will be called twice.
+  EXPECT_CALL(manager_delegate_, HideAutofillPopup()).Times(2);
+
   content::WindowedNotificationObserver observer(
       content::NOTIFICATION_WEB_CONTENTS_VISIBILITY_CHANGED,
       content::Source<content::WebContents>(web_contents_));
   chrome::AddSelectedTabWithURL(browser(), GURL(chrome::kAboutBlankURL),
                                 content::PAGE_TRANSITION_AUTO_TOPLEVEL);
   observer.Wait();
-
-  EXPECT_TRUE(autofill_external_delegate_->popup_hidden());
 }
 
 IN_PROC_BROWSER_TEST_F(AutofillExternalDelegateBrowserTest,
                        TestPageNavigationHidingAutofillPopup) {
   autofill::GenerateTestAutofillPopup(autofill_external_delegate_.get());
 
-  EXPECT_FALSE(autofill_external_delegate_->popup_hidden());
+  EXPECT_CALL(manager_delegate_, HideAutofillPopup()).Times(2);

[Ilya Sherman, 2013/02/26 22:55:51]

nit: Please include the comment from lines 114-116 here as well.

[kaiwang, 2013/02/27 00:00:10]

Done.

 
   content::WindowedNotificationObserver observer(
       content::NOTIFICATION_NAV_ENTRY_COMMITTED,
@@ -128,17 +141,4 @@ IN_PROC_BROWSER_TEST_F(AutofillExternalDelegateBrowserTest,
       GURL(chrome::kChromeUIAboutURL), content::Referrer(),
       CURRENT_TAB, content::PAGE_TRANSITION_TYPED, false));
   observer.Wait();
-
-  EXPECT_TRUE(autofill_external_delegate_->popup_hidden());
-}
-
-// Tests that closing the widget does not leak any resources.  This test is
-// only really meaningful when run on the memory bots.
-IN_PROC_BROWSER_TEST_F(AutofillExternalDelegateBrowserTest,
-                       CloseWidgetAndNoLeaking) {
-  autofill::GenerateTestAutofillPopup(autofill_external_delegate_.get());
-
-  // Delete the view from under the delegate to ensure that the
-  // delegate and the controller can handle the popup getting deleted elsewhere.
-  autofill_external_delegate_->GetController()->view()->Hide();
 }