cbui: port PermissionBubbleInteractiveUITest to Win/Linux

This change moves these tests from Mac-only to all Views platforms,
although they are currently disabled on Windows & Linux because of the
linked bug. To do this, the tests are rewritten to use Views APIs rather
than directly using NSWindow, and PermissionRequestManagerTestApi is
modified to deal in Widgets rather than NativeWindows.

This change also removes PermissionBubbleViewsInteractiveUITest, which
was an incomplete copy of PermissionBubbleInteractiveUITest dating to
the initial MacViews secondary UI work, and also has never run on any
non-Mac platform.

Bug: 866878
Change-Id: Id0cbf5e2e1ec54afed57fd74e2c4380c1003de40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2038157Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Bret Sepulveda <bsep@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738574}

chrome/browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_interactive_uitest.mm → chrome/browser/ui/views/permission_bubble/permission_bubble_interactive_uitest.cc

@@ -2,15 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#import <Cocoa/Cocoa.h>
-
 #include <memory>
 
-#include "base/command_line.h"
-#import "base/mac/scoped_nsobject.h"
 #include "base/run_loop.h"
-#include "base/test/scoped_feature_list.h"
-#include "chrome/browser/chrome_notification_types.h"
+#include "build/build_config.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_window.h"
@@ -20,42 +15,37 @@
 #include "chrome/test/base/ui_test_utils.h"
 #include "chrome/test/permissions/permission_request_manager_test_api.h"
 #include "ui/base/test/ui_controls.h"
-#import "ui/base/test/windowed_nsnotification_observer.h"
-#include "ui/base/ui_base_features.h"
-#import "ui/events/test/cocoa_test_event_utils.h"
+#include "ui/views/test/widget_test.h"
 
 class PermissionBubbleInteractiveUITest : public InProcessBrowserTest {
  public:
-  PermissionBubbleInteractiveUITest() {}
+  PermissionBubbleInteractiveUITest() = default;
+
+  void EnsureWindowActive(ui::BaseWindow* window, const char* message) {
+    EnsureWindowActive(
+        views::Widget::GetWidgetForNativeWindow(window->GetNativeWindow()),
+        message);
+  }
 
-  void EnsureWindowActive(NSWindow* window, const char* message) {
+  void EnsureWindowActive(views::Widget* widget, const char* message) {
     SCOPED_TRACE(message);
-    EXPECT_TRUE(window);
-
-    // Activation is asynchronous on Mac. If the window didn't become active,
-    // wait for it.
-    if (![window isKeyWindow]) {
-      base::scoped_nsobject<WindowedNSNotificationObserver> waiter(
-          [[WindowedNSNotificationObserver alloc]
-              initForNotification:NSWindowDidBecomeKeyNotification
-                           object:window]);
-      [waiter wait];
-    }
-    EXPECT_TRUE([window isKeyWindow]);
-
-    // Whether or not we had to wait, flush the run loop. This ensures any
-    // asynchronous close operations have completed.
-    base::RunLoop().RunUntilIdle();
+    EXPECT_TRUE(widget);
+
+    views::test::WidgetActivationWaiter waiter(widget, true);
+    waiter.Wait();
   }
 
-  // Send Cmd+keycode in the key window to NSApp.
+  // Send Ctrl/Cmd+keycode in the key window to the browser.
   void SendAccelerator(ui::KeyboardCode keycode, bool shift, bool alt) {
+#if defined(OS_MACOSX)
     bool control = false;
     bool command = true;
-    // Note that although this takes an NSWindow, it's just used to create the
-    // NSEvent* which will be dispatched to NSApp (i.e. not NSWindow).
-    ui_controls::SendKeyPress(
-        [NSApp keyWindow], keycode, control, shift, alt, command);
+#else
+    bool control = true;
+    bool command = false;
+#endif
+    ui_controls::SendKeyPress(browser()->window()->GetNativeWindow(), keycode,
+                              control, shift, alt, command);
   }
 
   void SetUpOnMainThread() override {
@@ -68,46 +58,53 @@ class PermissionBubbleInteractiveUITest : public InProcessBrowserTest {
 
     test_api_->AddSimpleRequest(ContentSettingsType::GEOLOCATION);
 
-    EXPECT_TRUE([browser()->window()->GetNativeWindow().GetNativeNSWindow()
-                     isKeyWindow]);
+    EXPECT_TRUE(browser()->window()->IsActive());
 
-    // The PermissionRequestManager displays prompts asynchronously.
+    // The permission prompt is shown asynchronously.
     base::RunLoop().RunUntilIdle();
-
-    // The bubble should steal key focus when shown.
-    EnsureWindowActive(test_api_->GetPromptWindow().GetNativeNSWindow(),
-                       "show permission bubble");
+    EnsureWindowActive(test_api_->GetPromptWindow(), "show permission bubble");
   }
 
  protected:
   std::unique_ptr<test::PermissionRequestManagerTestApi> test_api_;
-  base::test::ScopedFeatureList scoped_feature_list_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(PermissionBubbleInteractiveUITest);
 };
 
-// There is only one tab. Cmd+w will close it along with the browser window.
-IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest, CmdWClosesWindow) {
-  base::scoped_nsobject<NSWindow> browser_window(
-      browser()->window()->GetNativeWindow().GetNativeNSWindow(),
-      base::scoped_policy::RETAIN);
-  EXPECT_TRUE([browser_window isVisible]);
+#if defined(OS_WIN) || defined(OS_LINUX)
+// TODO(https://crbug.com/866878): Accelerators are broken when this bubble is
+// showing on non-Mac.
+#define MAYBE_CmdWClosesWindow DISABLED_CmdWClosesWindow
+#else
+#define MAYBE_CmdWClosesWindow CmdWClosesWindow
+#endif
+
+// There is only one tab. Ctrl/Cmd+w will close it along with the browser
+// window.
+IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest,
+                       MAYBE_CmdWClosesWindow) {
+  EXPECT_TRUE(browser()->window()->IsVisible());
 
   SendAccelerator(ui::VKEY_W, false, false);
 
   // The actual window close happens via a posted task.
-  EXPECT_TRUE([browser_window isVisible]);
+  EXPECT_TRUE(browser()->window()->IsVisible());
   ui_test_utils::WaitForBrowserToClose(browser());
-  EXPECT_FALSE([browser_window isVisible]);
+  EXPECT_FALSE(browser()->window()->IsVisible());
 }
 
-// Add a tab, ensure we can switch away and back using Cmd+Alt+Left/Right and
-// curly braces.
-IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest, SwitchTabs) {
-  NSWindow* browser_window =
-      browser()->window()->GetNativeWindow().GetNativeNSWindow();
-
+#if defined(OS_WIN) || defined(OS_LINUX)
+// TODO(https://crbug.com/866878): Accelerators are broken when this bubble is
+// showing on non-Mac.
+#define MAYBE_SwitchTabs DISABLED_SwitchTabs
+#else
+#define MAYBE_SwitchTabs SwitchTabs
+#endif
+
+// Add a tab, ensure we can switch away and back using Ctrl/Cmd+Alt+Left/Right
+// and curly braces.
+IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest, MAYBE_SwitchTabs) {
   EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
   EXPECT_TRUE(test_api_->GetPromptWindow());
 
@@ -119,8 +116,8 @@ IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest, SwitchTabs) {
   // test environment can't guarantee that macOS decides that the Browser window
   // is actually the "best" window to activate upon closing the current key
   // window. So activate it manually.
-  [browser_window makeKeyAndOrderFront:nil];
-  EnsureWindowActive(browser_window, "tab added");
+  browser()->window()->Activate();
+  EnsureWindowActive(browser()->window(), "tab added");
 
   // Prompt is hidden while its tab is not active.
   EXPECT_FALSE(test_api_->GetPromptWindow());
@@ -134,7 +131,7 @@ IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest, SwitchTabs) {
 
   // Note we don't need to makeKeyAndOrderFront: the permission window will take
   // focus when it is shown again.
-  EnsureWindowActive(test_api_->GetPromptWindow().GetNativeNSWindow(),
+  EnsureWindowActive(test_api_->GetPromptWindow(),
                      "switched to permission tab with arrow");
   EXPECT_TRUE(test_api_->GetPromptWindow());
 
@@ -142,8 +139,8 @@ IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest, SwitchTabs) {
   SendAccelerator(ui::VKEY_RIGHT, false, true);
   EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
 
-  [browser_window makeKeyAndOrderFront:nil];
-  EnsureWindowActive(browser_window, "switch away with arrow");
+  browser()->window()->Activate();
+  EnsureWindowActive(browser()->window(), "switch away with arrow");
   EXPECT_FALSE(test_api_->GetPromptWindow());
 
   // Also test switching tabs with curly braces. "VKEY_OEM_4" is
@@ -153,13 +150,13 @@ IN_PROC_BROWSER_TEST_F(PermissionBubbleInteractiveUITest, SwitchTabs) {
   chrome::FocusLocationBar(browser());
   SendAccelerator(ui::VKEY_OEM_4, true, false);
   EXPECT_EQ(0, browser()->tab_strip_model()->active_index());
-  EnsureWindowActive(test_api_->GetPromptWindow().GetNativeNSWindow(),
+  EnsureWindowActive(test_api_->GetPromptWindow(),
                      "switch to permission tab with curly brace");
   EXPECT_TRUE(test_api_->GetPromptWindow());
 
   SendAccelerator(ui::VKEY_OEM_4, true, false);
   EXPECT_EQ(1, browser()->tab_strip_model()->active_index());
-  [browser_window makeKeyAndOrderFront:nil];
-  EnsureWindowActive(browser_window, "switch away with curly brace");
+  browser()->window()->Activate();
+  EnsureWindowActive(browser()->window(), "switch away with curly brace");
   EXPECT_FALSE(test_api_->GetPromptWindow());
 }

chrome/browser/ui/views/permission_bubble/permission_bubble_views_interactive_uitest_mac.mm

-// Copyright 2018 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.
-
-#import <Cocoa/Cocoa.h>
-
-#include <memory>
-
-#import "base/mac/scoped_nsobject.h"
-#include "base/run_loop.h"
-#include "chrome/browser/chrome_notification_types.h"
-#include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/browser_commands.h"
-#include "chrome/browser/ui/browser_window.h"
-#include "chrome/test/base/in_process_browser_test.h"
-#include "chrome/test/base/interactive_test_utils.h"
-#include "chrome/test/base/ui_test_utils.h"
-#include "chrome/test/permissions/permission_request_manager_test_api.h"
-#include "ui/base/test/ui_controls.h"
-#import "ui/base/test/windowed_nsnotification_observer.h"
-#include "ui/base/ui_base_features.h"
-#import "ui/events/test/cocoa_test_event_utils.h"
-
-class PermissionBubbleViewsInteractiveUITest : public InProcessBrowserTest {
- public:
-  PermissionBubbleViewsInteractiveUITest() {}
-
-  void EnsureWindowActive(NSWindow* window, const char* message) {
-    SCOPED_TRACE(message);
-    EXPECT_TRUE(window);
-
-    // Activation is asynchronous on Mac. If the window didn't become active,
-    // wait for it.
-    if (![window isKeyWindow]) {
-      base::scoped_nsobject<WindowedNSNotificationObserver> waiter(
-          [[WindowedNSNotificationObserver alloc]
-              initForNotification:NSWindowDidBecomeKeyNotification
-                           object:window]);
-      [waiter wait];
-    }
-    EXPECT_TRUE([window isKeyWindow]);
-
-    // Whether or not we had to wait, flush the run loop. This ensures any
-    // asynchronous close operations have completed.
-    base::RunLoop().RunUntilIdle();
-  }
-
-  // Send Cmd+keycode in the key window to NSApp.
-  void SendAccelerator(ui::KeyboardCode keycode) {
-    bool shift = false;
-    bool alt = false;
-    bool control = false;
-    bool command = true;
-    // Note that although this takes an NSWindow, it's just used to create the
-    // NSEvent* which will be dispatched to NSApp (i.e. not NSWindow).
-    ui_controls::SendKeyPress([NSApp keyWindow], keycode, control, shift, alt,
-                              command);
-  }
-
-  void SetUpOnMainThread() override {
-    // Make the browser active (ensures the app can receive key events).
-    EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
-
-    test_api_ =
-        std::make_unique<test::PermissionRequestManagerTestApi>(browser());
-    EXPECT_TRUE(test_api_->manager());
-
-    test_api_->AddSimpleRequest(ContentSettingsType::GEOLOCATION);
-
-    EXPECT_TRUE([browser()->window()->GetNativeWindow().GetNativeNSWindow()
-                     isKeyWindow]);
-
-    // The PermissionRequestManager displays prompts asynchronously.
-    base::RunLoop().RunUntilIdle();
-
-    // The bubble should steal key focus when shown.
-    EnsureWindowActive(test_api_->GetPromptWindow().GetNativeNSWindow(),
-                       "show permission bubble");
-  }
-
- private:
-  std::unique_ptr<test::PermissionRequestManagerTestApi> test_api_;
-
-  DISALLOW_COPY_AND_ASSIGN(PermissionBubbleViewsInteractiveUITest);
-};
-
-// There is only one tab. Cmd+w will close it along with the browser window.
-IN_PROC_BROWSER_TEST_F(PermissionBubbleViewsInteractiveUITest,
-                       CmdWClosesWindow) {
-  base::scoped_nsobject<NSWindow> browser_window(
-      browser()->window()->GetNativeWindow().GetNativeNSWindow(),
-      base::scoped_policy::RETAIN);
-  EXPECT_TRUE([browser_window isVisible]);
-
-  SendAccelerator(ui::VKEY_W);
-
-  // The actual window close happens via a posted task.
-  EXPECT_TRUE([browser_window isVisible]);
-  ui_test_utils::WaitForBrowserToClose(browser());
-  EXPECT_FALSE([browser_window isVisible]);
-}

chrome/test/BUILD.gn

@@ -5705,7 +5705,6 @@ if (!is_android) {
       "../browser/ui/views/accessibility/navigation_accessibility_uitest_win.cc",
       "../browser/ui/views/content_test_utils.cc",
       "../browser/ui/views/content_test_utils.h",
-      "../browser/ui/views/permission_bubble/permission_bubble_views_interactive_uitest_mac.mm",
       "../browser/ui/views/tabs/tab_spinner_interactive_uitest.cc",
       "../browser/ui/views/web_apps/web_app_frame_toolbar_interactive_uitest.cc",
       "../browser/ui/views/web_dialog_view_browsertest.cc",
@@ -5859,6 +5858,7 @@ if (!is_android) {
         "../browser/ui/views/omnibox/omnibox_view_views_browsertest.cc",
         "../browser/ui/views/passwords/manage_passwords_icon_view_interactive_uitest.cc",
         "../browser/ui/views/passwords/password_bubble_interactive_uitest.cc",
+        "../browser/ui/views/permission_bubble/permission_bubble_interactive_uitest.cc",
         "../browser/ui/views/sad_tab_view_interactive_uitest.cc",
         "../browser/ui/views/status_icons/status_tray_state_changer_interactive_uitest_win.cc",
         "../browser/ui/views/tabs/tab_drag_controller_interactive_uitest.cc",
@@ -6000,15 +6000,6 @@ if (!is_android) {
         "//chrome",
         "//chrome:chrome_framework",
       ]
-
-      # The browser window can be views or Cocoa on Mac, but this is chosen at
-      # runtime. This block captures tests that only run with a Cocoa browser.
-      sources += [
-        # Note permission_bubble_cocoa_interactive_uitest tests both Cocoa and
-        # toolkit-views secondary UI. It should not be deleted with the Cocoa
-        # bubble (but it can be deleted with the Cocoa browser window).
-        "../browser/ui/cocoa/permission_bubble/permission_bubble_cocoa_interactive_uitest.mm",
-      ]
     }
 
     if (use_x11) {

chrome/test/permissions/permission_request_manager_test_api.cc

@@ -10,12 +10,9 @@
 #include "chrome/browser/permissions/permission_request_impl.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-
-#if defined(TOOLKIT_VIEWS)
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_bubble_view.h"
 #include "chrome/browser/ui/views/permission_bubble/permission_prompt_impl.h"
 #include "ui/views/widget/widget.h"
-#endif
 
 namespace test {
 namespace {
@@ -23,14 +20,14 @@ namespace {
 // Wraps a PermissionRequestImpl so that it can pass a closure to itself to the
 // PermissionRequestImpl constructor. Without this wrapper, there's no way to
 // handle all destruction paths.
-class TestPermisisonRequestOwner {
+class TestPermissionRequestOwner {
  public:
-  explicit TestPermisisonRequestOwner(ContentSettingsType type) {
+  explicit TestPermissionRequestOwner(ContentSettingsType type) {
     bool user_gesture = true;
     auto decided = [](ContentSetting) {};
     request_ = std::make_unique<PermissionRequestImpl>(
         GURL("https://example.com"), type, user_gesture, base::Bind(decided),
-        base::Bind(&TestPermisisonRequestOwner::DeleteThis,
+        base::Bind(&TestPermissionRequestOwner::DeleteThis,
                    base::Unretained(this)));
   }
 
@@ -41,7 +38,7 @@ class TestPermisisonRequestOwner {
 
   std::unique_ptr<PermissionRequestImpl> request_;
 
-  DISALLOW_COPY_AND_ASSIGN(TestPermisisonRequestOwner);
+  DISALLOW_COPY_AND_ASSIGN(TestPermissionRequestOwner);
 };
 
 }  // namespace
@@ -57,22 +54,17 @@ PermissionRequestManagerTestApi::PermissionRequestManagerTestApi(
 
 void PermissionRequestManagerTestApi::AddSimpleRequest(
     ContentSettingsType type) {
-  TestPermisisonRequestOwner* request_owner =
-      new TestPermisisonRequestOwner(type);
+  TestPermissionRequestOwner* request_owner =
+      new TestPermissionRequestOwner(type);
   manager_->AddRequest(request_owner->request());
 }
 
-gfx::NativeWindow PermissionRequestManagerTestApi::GetPromptWindow() {
-#if defined(TOOLKIT_VIEWS)
+views::Widget* PermissionRequestManagerTestApi::GetPromptWindow() {
   PermissionPromptImpl* prompt =
       static_cast<PermissionPromptImpl*>(manager_->view_.get());
   return prompt ? prompt->prompt_bubble_for_testing()
                       ->GetWidget()
-                      ->GetNativeWindow()
                 : nullptr;
-#else
-  NOTIMPLEMENTED();
-#endif
 }
 
 void PermissionRequestManagerTestApi::SimulateWebContentsDestroyed() {

chrome/test/permissions/permission_request_manager_test_api.h

@@ -11,6 +11,10 @@
 
 class Browser;
 
+namespace views {
+class Widget;
+}  // namespace views
+
 namespace test {
 
 class PermissionRequestManagerTestApi {
@@ -27,9 +31,9 @@ class PermissionRequestManagerTestApi {
   // NOTIFICATIONS, GEOLOCATON, or PLUGINS.
   void AddSimpleRequest(ContentSettingsType type);
 
-  // Return the NativeWindow for the permission prompt bubble, or nullptr if
+  // Return the Widget for the permission prompt bubble, or nullptr if
   // there is no prompt currently showing.
-  gfx::NativeWindow GetPromptWindow();
+  views::Widget* GetPromptWindow();
 
   void SimulateWebContentsDestroyed();