MacViews: Allows the toolkit-views Manage Passwords Dialog to be used

chrome/browser/ui/views/passwords/manage_passwords_bubble_view_interactive_uitest.cc

Index: chrome/browser/ui/views/passwords/manage_passwords_bubble_view_interactive_uitest.cc
diff --git a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_interactive_uitest.cc b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_interactive_uitest.cc
index 1579d11594006ac20772438262888c2444e7c8bc..afff97b46d8a0f5602df14115f8a7b44515899d7 100644
--- a/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_interactive_uitest.cc
+++ b/chrome/browser/ui/views/passwords/manage_passwords_bubble_view_interactive_uitest.cc
@@ -26,6 +26,8 @@
 #include "content/public/common/content_features.h"
 #include "net/url_request/test_url_fetcher_factory.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "ui/base/ui_base_switches.h"
+#include "ui/base/ui_features.h"
 #include "ui/views/test/widget_test.h"
 
 using testing::Eq;
@@ -63,9 +65,24 @@ bool IsBubbleShowing() {
 
 namespace metrics_util = password_manager::metrics_util;
 
-using ManagePasswordsBubbleViewTest = ManagePasswordsTest;
+class ManagePasswordsBubbleViewTest : public ManagePasswordsTest {
+ public:
+  ManagePasswordsBubbleViewTest() {}
+  ~ManagePasswordsBubbleViewTest() override {}
+
+  // content::BrowserTestBase:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+#if defined(OS_MACOSX)
+    command_line->AppendSwitch(switches::kExtendMdToSecondaryUi);
+#endif
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ManagePasswordsBubbleViewTest);
+};
 
 IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, BasicOpenAndClose) {
+  ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));

[tapted, 2017/04/19 03:31:07]

ah, I need to try again on that CL to do this by default - the bots got angry at
me

[varkha, 2017/04/19 08:34:17]

Acknowledged.

   EXPECT_FALSE(IsBubbleShowing());
   SetupPendingPassword();
   EXPECT_TRUE(IsBubbleShowing());
@@ -92,6 +109,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, BasicOpenAndClose) {
 // Same as 'BasicOpenAndClose', but use the command rather than the static
 // method directly.
 IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandControlsBubble) {
+  ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
   // The command only works if the icon is visible, so get into management mode.
   SetupManagingPasswords();
   EXPECT_FALSE(IsBubbleShowing());
@@ -104,6 +122,7 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, CommandControlsBubble) {
             bubble->GetFocusManager()->GetFocusedView());
   ManagePasswordsBubbleView::CloseCurrentBubble();
   EXPECT_FALSE(IsBubbleShowing());
+  content::RunAllPendingInMessageLoop();

[tapted, 2017/04/19 03:31:06]

It's not obvious why adding this was necessary. comment? (e.g. did we block
showing the bubble on mac while it is still closing? - does adding an IsVisible
check back help?)

[varkha, 2017/04/19 08:34:17]

Done. Added a comment.

 
   // And, just for grins, ensure that we can re-open the bubble.
   ExecuteManagePasswordsCommand();
@@ -166,6 +185,8 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest,
   SetupPendingPassword();
   EXPECT_TRUE(IsBubbleShowing());
   ManagePasswordsBubbleView::CloseCurrentBubble();
+  content::RunAllPendingInMessageLoop();

[tapted, 2017/04/19 03:31:06]

(same comment)

[varkha, 2017/04/19 08:34:17]

Done. Added a comment.

+
   // This opening should be measured as manual.
   ExecuteManagePasswordsCommand();
   EXPECT_TRUE(IsBubbleShowing());
@@ -314,27 +335,28 @@ IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, AutoSigninNoFocus) {
   local_credentials.push_back(
       base::MakeUnique<autofill::PasswordForm>(*test_form()));
 
-  // Open another window with focus.
+  // Open another window with focus and wait for it to become active.
   Browser* focused_window = CreateBrowser(browser()->profile());
   ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(focused_window));
   content::RunAllPendingInMessageLoop();
 
-  gfx::NativeWindow window = browser()->window()->GetNativeWindow();
-  views::Widget* widget = views::Widget::GetWidgetForNativeWindow(window);
-  ASSERT_NE(nullptr, widget);
+  ui_test_utils::BrowserActivationWaiter focused_waiter(focused_window);
+  focused_waiter.WaitForActivation();

[tapted, 2017/04/19 03:31:07]

BringBrowserWindowToFront does exactly this. I don't think this is necessary

[varkha, 2017/04/19 08:34:17]

Done.

 
-  views::test::WidgetActivationWaiter inactive_waiter(widget, false);

[tapted, 2017/04/19 03:31:06]

Note I don't think this |inactive_waiter| was ever doing anything since at this
point |focused_window|->window() is guaranteed to have focus by the
BringBrowserWindowToFront call above. So here, |widget->IsActive()| == false
means the WidgetActivationWaiter bails out early and .Wait() will no-op.

[varkha, 2017/04/19 08:34:17]

Acknowledged.

-  inactive_waiter.Wait();
   ManagePasswordsBubbleView::set_auto_signin_toast_timeout(0);
   SetupAutoSignin(std::move(local_credentials));
-  content::RunAllPendingInMessageLoop();
   EXPECT_TRUE(IsBubbleShowing());
 
+// Auto-close of the auto-sign-in dialog is not implemented for a Cocoa
+// browser.
+#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
   // Bring the first window back. The toast closes by timeout.
   focused_window->window()->Close();
   browser()->window()->Activate();
   content::RunAllPendingInMessageLoop();
-  views::test::WidgetActivationWaiter active_waiter(widget, true);
-  active_waiter.Wait();
+  ui_test_utils::BrowserActivationWaiter waiter(browser());
+  waiter.WaitForActivation();

[tapted, 2017/04/19 03:31:06]

I think the 5 lines above can just be replaced by 

ASSERT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));

[varkha, 2017/04/19 08:34:17]

Done.

+
   EXPECT_FALSE(IsBubbleShowing());

[tapted, 2017/04/19 03:31:06]

Then the comment above the #ifdef can move down here, and we can have

// Auto-close of the auto-sign-in dialog is not implemented for a Cocoa
// browser.
#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
EXPECT_TRUE(IsBubbleShowing());
#else
EXPECT_FALSE(IsBubbleShowing());
#endif

[varkha, 2017/04/19 08:34:17]

Done.

+#endif
 }