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

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

 // Copyright 2014 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 "chrome/browser/ui/views/passwords/manage_passwords_bubble_view.h"
 
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/passwords/manage_passwords_test.h"
 #include "chrome/browser/ui/passwords/passwords_model_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
 #include "chrome/browser/ui/views/passwords/manage_passwords_icon_views.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/render_view_host.h"
 #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;
 using testing::Field;
 using testing::_;
 
 namespace {
 
 const char kDisplayDispositionMetric[] = "PasswordBubble.DisplayDisposition";
 
@@ skipping 17 matching lines @@
 bool IsBubbleShowing() {
   return ManagePasswordsBubbleView::manage_password_bubble() &&
       ManagePasswordsBubbleView::manage_password_bubble()->
           GetWidget()->IsVisible();
 }
 
 }  // namespace
 
 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());
   const ManagePasswordsBubbleView* bubble =
       ManagePasswordsBubbleView::manage_password_bubble();
   EXPECT_TRUE(bubble->initially_focused_view());
   EXPECT_FALSE(bubble->GetFocusManager()->GetFocusedView());
   ManagePasswordsBubbleView::CloseCurrentBubble();
   EXPECT_FALSE(IsBubbleShowing());
 
   // And, just for grins, ensure that we can re-open the bubble.
   ManagePasswordsBubbleView::ShowBubble(
       browser()->tab_strip_model()->GetActiveWebContents(),
       ManagePasswordsBubbleView::USER_GESTURE);
   EXPECT_TRUE(IsBubbleShowing());
   bubble = ManagePasswordsBubbleView::manage_password_bubble();
   EXPECT_TRUE(bubble->initially_focused_view());
   EXPECT_EQ(bubble->initially_focused_view(),
             bubble->GetFocusManager()->GetFocusedView());
   ManagePasswordsBubbleView::CloseCurrentBubble();
   EXPECT_FALSE(IsBubbleShowing());
 }
 
 // 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());
   ExecuteManagePasswordsCommand();
   EXPECT_TRUE(IsBubbleShowing());
   const ManagePasswordsBubbleView* bubble =
       ManagePasswordsBubbleView::manage_password_bubble();
   EXPECT_TRUE(bubble->initially_focused_view());
   EXPECT_EQ(bubble->initially_focused_view(),
             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();
   EXPECT_TRUE(IsBubbleShowing());
   ManagePasswordsBubbleView::CloseCurrentBubble();
   EXPECT_FALSE(IsBubbleShowing());
 }
 
 IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest,
                        CommandExecutionInManagingState) {
@@ skipping 42 matching lines @@
             samples->GetCount(
                 metrics_util::MANUAL_MANAGE_PASSWORDS));
 }
 
 IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest,
                        CommandExecutionInPendingState) {
   // Open once with pending password: automagical!
   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());
 
   std::unique_ptr<base::HistogramSamples> samples(
       GetSamples(kDisplayDispositionMetric));
   EXPECT_EQ(
       1,
       samples->GetCount(
           metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING));
@@ skipping 128 matching lines @@
 }
 
 IN_PROC_BROWSER_TEST_F(ManagePasswordsBubbleViewTest, AutoSigninNoFocus) {
   test_form()->origin = GURL("https://example.com");
   test_form()->display_name = base::ASCIIToUTF16("Peter");
   test_form()->username_value = base::ASCIIToUTF16("pet12@gmail.com");
   std::vector<std::unique_ptr<autofill::PasswordForm>> local_credentials;
   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
 }