Always Close the Autofill UI through the same path

chrome/browser/ui/autofill/autofill_popup_controller_browsertest.cc

+// Copyright (c) 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 "base/memory/scoped_ptr.h"
+#include "base/memory/weak_ptr.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/ui/browser.h"
+#include "chrome/browser/ui/fullscreen/fullscreen_controller.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+#include "content/public/browser/web_contents_view.h"
+#include "content/public/test/test_utils.h"
+#include "ui/gfx/rect.h"
+
+namespace {
+
+class TestAutofillPopupController : public AutofillPopupControllerImpl {
+ public:
+  explicit TestAutofillPopupController(
+      AutofillExternalDelegate* external_delegate,
+      gfx::NativeView container_view,
+      const gfx::RectF& element_bounds)
+      : AutofillPopupControllerImpl(external_delegate,
+                                    container_view,
+                                    element_bounds) {}
+  virtual ~TestAutofillPopupController() {}
+
+  base::WeakPtr<AutofillPopupControllerImpl> GetWeakPtr() {
+    return AutofillPopupControllerImpl::GetWeakPtr();
+  }
+
+ private:
+  virtual void ShowView() OVERRIDE {}
+};

[Ilya Sherman, 2013/02/22 00:29:04]

nit: DISALLOW_COPY_AND_ASSIGN

[csharp, 2013/02/22 15:41:31]

Done.

+
+class TestAutofillExternalDelegate :
+      public autofill::TestAutofillExternalDelegate {

[Ilya Sherman, 2013/02/22 00:29:04]

nit: Too much indentation?

[csharp, 2013/02/22 15:41:31]

Done.

+ public:
+  explicit TestAutofillExternalDelegate(content::WebContents* web_contents) :
+      autofill::TestAutofillExternalDelegate(web_contents, NULL),
+      popup_hidden_(false),
+      running_(false) {}
+
+  void set_controller(base::WeakPtr<AutofillPopupControllerImpl> controller) {
+    autofill::TestAutofillExternalDelegate::set_controller(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 {

[Ilya Sherman, 2013/02/22 00:29:04]

nit: Spurious space before OVERRIDE

[csharp, 2013/02/22 15:41:31]

Done.

+    popup_hidden_ = false;
+
+    AutofillExternalDelegate::ApplyAutofillSuggestions(autofill_values,
+                                                       autofill_labels,
+                                                       autofill_icons,
+                                                       autofill_unique_ids);
+  }
+
+  virtual void OnPopupHidden(content::KeyboardListener* listener) OVERRIDE {
+    popup_hidden_ = true;
+
+    AutofillExternalDelegate::OnPopupHidden(listener);
+
+    if (running_) {
+      message_loop_runner_->Quit();
+      running_ = false;
+    }
+  }
+
+  void WaitForPopupHidden() {
+    if (popup_hidden_)
+      return;
+
+    running_ = true;
+    message_loop_runner_ = new content::MessageLoopRunner;
+    message_loop_runner_->Run();
+  }
+
+  bool popup_hidden() const { return popup_hidden_; }
+
+ private:
+  bool popup_hidden_;
+
+  bool running_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+
+};
+
+}  // namespace
+
+class AutofillPopupControllerBrowserTest
+    : public InProcessBrowserTest,
+      public content::WebContentsObserver {
+ public:
+  AutofillPopupControllerBrowserTest() {}
+  virtual ~AutofillPopupControllerBrowserTest() {}
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    web_contents_ = browser()->tab_strip_model()->GetActiveWebContents();
+    ASSERT_TRUE(web_contents_ != NULL);
+    Observe(web_contents_);
+
+    autofill_external_delegate_.reset(
+        new TestAutofillExternalDelegate(web_contents_));
+
+    gfx::NativeView view = web_contents() ?
+        web_contents()->GetView()->GetContentNativeView() : NULL;
+
+    TestAutofillPopupController* controller =
+        new TestAutofillPopupController(autofill_external_delegate_.get(),
+                                        view,
+                                        gfx::Rect());
+    autofill_external_delegate_->set_controller(
+        controller->GetWeakPtr());
+  }
+
+  // Normally the WebContents will automatically delete the delegate, but here
+  // the delegate is owned by this test, so we have to manually destroy.

[Ilya Sherman, 2013/02/22 00:29:04]

Why not just do this in a method like TearDown()?

[csharp, 2013/02/22 15:41:31]

Hmm, I don't think we actually need this code at all, removed (since the
scoped_ptr will get properly deleted by itself)

+  virtual void WebContentsDestroyed(content::WebContents* web_contents)
+      OVERRIDE {
+    DCHECK_EQ(web_contents_, web_contents);

[Ilya Sherman, 2013/02/22 00:29:04]

Tests should use ASSERT_EQ, not DCHECK_EQ

+    autofill_external_delegate_.reset();
+  }
+
+ protected:
+  content::WebContents* web_contents_;
+  scoped_ptr<TestAutofillExternalDelegate>
+      autofill_external_delegate_;
+};
+
+IN_PROC_BROWSER_TEST_F(AutofillPopupControllerBrowserTest,
+                       HidePopupOnWindowConfiguration) {
+  autofill::GenerateTestAutofillPopup(autofill_external_delegate_.get());
+
+  EXPECT_FALSE(autofill_external_delegate_->popup_hidden());
+
+  browser()->fullscreen_controller()->ToggleFullscreenMode();
+
+  autofill_external_delegate_->WaitForPopupHidden();
+  EXPECT_TRUE(autofill_external_delegate_->popup_hidden());
+}