[reland] Bring up BrowserActionInteractiveTest on Mac.

chrome/browser/extensions/api/extension_action/browser_action_interactive_test.cc

 // Copyright 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 "build/build_config.h"
 #include "chrome/browser/extensions/browser_action_test_util.h"
 #include "chrome/browser/extensions/extension_action.h"
 #include "chrome/browser/extensions/extension_action_manager.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
@@ skipping 10 matching lines @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/notification_types.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
 #include "extensions/common/permissions/permissions_data.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "extensions/test/result_catcher.h"
+#include "ui/base/ui_features.h"
 
 #if defined(OS_WIN)
 #include "ui/views/win/hwnd_util.h"
 #endif
 
 namespace extensions {
 namespace {
 
 // chrome.browserAction API tests that interact with the UI in such a way that
 // they cannot be run concurrently (i.e. openPopup API tests that require the
 // window be focused/active).
 class BrowserActionInteractiveTest : public ExtensionApiTest {
  public:
   BrowserActionInteractiveTest() {}
   ~BrowserActionInteractiveTest() override {}
 
+  // BrowserTestBase:
+  void SetUpOnMainThread() override {
+    ExtensionApiTest::SetUpOnMainThread();
+    EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+  }
+
  protected:
   // Function to control whether to run popup tests for the current platform.
   // These tests require RunExtensionSubtest to work as expected and the browser
   // window to able to be made active automatically. Returns false for platforms
   // where these conditions are not met.
   bool ShouldRunPopupTest() {
     // TODO(justinlin): http://crbug.com/177163
 #if defined(OS_WIN) && !defined(NDEBUG)
     return false;
-#elif defined(OS_MACOSX)
-    // TODO(justinlin): Browser window do not become active on Mac even when
-    // Activate() is called on them. Enable when/if it's possible to fix.
-    return false;
 #else
     return true;
 #endif
   }
 
+  void EnsurePopupActive() {
+    BrowserActionTestUtil test_util(browser());
+    EXPECT_TRUE(test_util.HasPopup());
+    EXPECT_TRUE(test_util.WaitForPopup());
+    EXPECT_TRUE(test_util.HasPopup());
+  }
+
   // Open an extension popup via the chrome.browserAction.openPopup API.
-  void OpenExtensionPopupViaAPI() {
+  void OpenPopupViaAPI() {
     // Setup the notification observer to wait for the popup to finish loading.
     content::WindowedNotificationObserver frame_observer(
         content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
         content::NotificationService::AllSources());
     // Show first popup in first window and expect it to have loaded.
     ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
                                     "open_popup_succeeds.html")) << message_;
     frame_observer.Wait();
+    EnsurePopupActive();
+  }
+
+  // Open an extension popup by clicking the browser action button.
+  void OpenPopupViaToolbar() {
+    content::WindowedNotificationObserver frame_observer(
+        content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
+        content::NotificationService::AllSources());
+    BrowserActionTestUtil(browser()).Press(0);
+    frame_observer.Wait();
+    EnsurePopupActive();
+  }
+
+  // Trigger a focus loss to close the popup.
+  void ClosePopupViaFocusLoss() {
     EXPECT_TRUE(BrowserActionTestUtil(browser()).HasPopup());
+    content::WindowedNotificationObserver observer(
+        extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+        content::NotificationService::AllSources());
+
+#if defined(OS_MACOSX)
+    // ClickOnView() in an inactive window is not robust on Mac. The click does
+    // not guarantee window activation on trybots. So activate the browser
+    // explicitly. This works because bubbles on Mac are always toplevel.

[Devlin, 2017/05/31 14:39:00]

nit: I'd add something like "activating the browser window causes the bubble
window to lose focus" (feel free to wordsmith) somewhere.

[tapted, 2017/06/01 05:23:37]

Done.

+    EXPECT_TRUE(ui_test_utils::BringBrowserWindowToFront(browser()));
+#else
+    // Elsewhere, click on the omnibox. Note that with aura, the browser may be
+    // "active" the entire time when the popup is not a toplevel window. It's
+    // aura::Window::Focus() that determines where key events go in this case.
+    ui_test_utils::ClickOnView(browser(), VIEW_ID_OMNIBOX);
+#endif
+
+    // The window disappears immediately.
+    EXPECT_FALSE(BrowserActionTestUtil(browser()).HasPopup());
+
+    // Wait for the notification to achieve a consistent state and verify that
+    // the popup was properly torn down.
+    observer.Wait();
+    base::RunLoop().RunUntilIdle();
   }
 };
 
 // Tests opening a popup using the chrome.browserAction.openPopup API. This test
 // opens a popup in the starting window, closes the popup, creates a new window
 // and opens a popup in the new window. Both popups should succeed in opening.
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, TestOpenPopup) {
   if (!ShouldRunPopupTest())
     return;
 
   BrowserActionTestUtil browserActionBar(browser());
   // Setup extension message listener to wait for javascript to finish running.
   ExtensionTestMessageListener listener("ready", true);
   {
-    OpenExtensionPopupViaAPI();
+    OpenPopupViaAPI();
     EXPECT_TRUE(browserActionBar.HasPopup());
     browserActionBar.HidePopup();
   }
 
   EXPECT_TRUE(listener.WaitUntilSatisfied());
   Browser* new_browser = NULL;
   {
     content::WindowedNotificationObserver frame_observer(
         content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
         content::NotificationService::AllSources());
@@ skipping 97 matching lines @@
       "browser_action/popup")));
   const Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(extension) << message_;
 
   ExtensionTestMessageListener listener("ready", true);
   // Load the test extension which will do nothing except notifyPass() to
   // return control here.
   ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
                                   "open_popup_fails.html")) << message_;
   EXPECT_TRUE(listener.WaitUntilSatisfied());
-
-  content::WindowedNotificationObserver frame_observer(
-      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
-      content::NotificationService::AllSources());
-  // Open popup in the first extension.
-  BrowserActionTestUtil(browser()).Press(0);
-  frame_observer.Wait();
-  EXPECT_TRUE(BrowserActionTestUtil(browser()).HasPopup());
-
+  OpenPopupViaToolbar();
   ResultCatcher catcher;
   // Return control to javascript to validate that opening a popup fails now.
   listener.Reply("show another");
   ASSERT_TRUE(catcher.GetNextResult()) << message_;
 }
 
 // Test that openPopup does not grant tab permissions like for browser action
 // clicks if the activeTab permission is set.
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
                        TestOpenPopupDoesNotGrantTabPermissions) {
   if (!ShouldRunPopupTest())
     return;
 
-  OpenExtensionPopupViaAPI();
+  OpenPopupViaAPI();
   ExtensionService* service = extensions::ExtensionSystem::Get(
       browser()->profile())->extension_service();
   ASSERT_FALSE(
       service->GetExtensionById(last_loaded_extension_id(), false)
           ->permissions_data()
           ->HasAPIPermissionForTab(
               SessionTabHelper::IdForTab(
                   browser()->tab_strip_model()->GetActiveWebContents()),
               APIPermission::kTab));
 }
 
 // Test that the extension popup is closed when the browser window is clicked.
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, BrowserClickClosesPopup1) {

[Devlin, 2017/05/31 14:39:00]

should we rename this test and update the comment?

[tapted, 2017/06/01 05:23:37]

Yup - Done.

   if (!ShouldRunPopupTest())
     return;
-
-  // Open an extension popup via the chrome.browserAction.openPopup API.
-  content::WindowedNotificationObserver frame_observer(
-      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
-      content::NotificationService::AllSources());
-  ASSERT_TRUE(RunExtensionSubtest("browser_action/open_popup",
-                                  "open_popup_succeeds.html")) << message_;
-  frame_observer.Wait();
-  EXPECT_TRUE(BrowserActionTestUtil(browser()).HasPopup());
-
-  // Click on the omnibox to close the extension popup.
-  ui_test_utils::ClickOnView(browser(), VIEW_ID_OMNIBOX);
-  EXPECT_FALSE(BrowserActionTestUtil(browser()).HasPopup());
+  OpenPopupViaAPI();
+  ClosePopupViaFocusLoss();
 }
 
 // Test that the extension popup is closed when the browser window is clicked.
 #if defined(OS_WIN)
 // Flaky on Windows: http://crbug.com/639130
 #define MAYBE_BrowserClickClosesPopup2 DISABLED_BrowserClickClosesPopup2
 #else
 #define MAYBE_BrowserClickClosesPopup2 BrowserClickClosesPopup2
 #endif
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
                        MAYBE_BrowserClickClosesPopup2) {

[Devlin, 2017/05/31 14:39:00]

ditto

[tapted, 2017/06/01 05:23:37]

Done.

   if (!ShouldRunPopupTest())
     return;
 
   // Load a first extension that can open a popup.
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(
       "browser_action/popup")));
   const Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(extension) << message_;
-
-  // Open an extension popup by clicking the browser action button.
-  content::WindowedNotificationObserver frame_observer(
-      content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
-      content::NotificationService::AllSources());
-  BrowserActionTestUtil(browser()).Press(0);
-  frame_observer.Wait();
-  EXPECT_TRUE(BrowserActionTestUtil(browser()).HasPopup());
-
-  // Click on the omnibox to close the extension popup.
-  ui_test_utils::ClickOnView(browser(), VIEW_ID_OMNIBOX);
-  EXPECT_FALSE(BrowserActionTestUtil(browser()).HasPopup());
+  OpenPopupViaToolbar();
+  ClosePopupViaFocusLoss();
 }
 
 // Test that the extension popup is closed on browser tab switches.
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, TabSwitchClosesPopup) {
   if (!ShouldRunPopupTest())
     return;
 
   // Add a second tab to the browser and open an extension popup.
   chrome::NewTab(browser());
   ASSERT_EQ(2, browser()->tab_strip_model()->count());
   EXPECT_EQ(browser()->tab_strip_model()->GetWebContentsAt(1),
             browser()->tab_strip_model()->GetActiveWebContents());
-  OpenExtensionPopupViaAPI();
+  OpenPopupViaAPI();
 
   content::WindowedNotificationObserver observer(
       extensions::NOTIFICATION_EXTENSION_HOST_DESTROYED,
       content::NotificationService::AllSources());
   // Change active tabs, the extension popup should close.
   browser()->tab_strip_model()->ActivateTabAt(0, true);
   observer.Wait();
 
   EXPECT_FALSE(BrowserActionTestUtil(browser()).HasPopup());
 }
 
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
                        DeleteBrowserActionWithPopupOpen) {
   if (!ShouldRunPopupTest())
     return;
 
   // First, we open a popup.
-  OpenExtensionPopupViaAPI();
+  OpenPopupViaAPI();
   BrowserActionTestUtil browser_action_test_util(browser());
   EXPECT_TRUE(browser_action_test_util.HasPopup());
 
   // Then, find the extension that created it.
   content::WebContents* active_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   ASSERT_TRUE(active_web_contents);
   GURL url = active_web_contents->GetLastCommittedURL();
   const Extension* extension = ExtensionRegistry::Get(browser()->profile())->
       enabled_extensions().GetExtensionOrAppByURL(url);
   ASSERT_TRUE(extension);
 
   // Finally, uninstall the extension, which causes the view to be deleted and
   // the popup to go away. This should not crash.
   UninstallExtension(extension->id());
   EXPECT_FALSE(browser_action_test_util.HasPopup());
 }
 
-#if defined(TOOLKIT_VIEWS)
+// BrowserActionTestUtil::InspectPopup() is not implemented for a Cocoa browser.
+#if !defined(OS_MACOSX) || BUILDFLAG(MAC_VIEWS_BROWSER)
+#define MAYBE_CloseBrowserWithDevTools CloseBrowserWithDevTools
+#else
+#define MAYBE_CloseBrowserWithDevTools DISABLED_CloseBrowserWithDevTools
+#endif
 // Test closing the browser while inspecting an extension popup with dev tools.
-IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest, CloseBrowserWithDevTools) {
+IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
+                       MAYBE_CloseBrowserWithDevTools) {
   if (!ShouldRunPopupTest())
     return;
 
   // Load a first extension that can open a popup.
   ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(
       "browser_action/popup")));
   const Extension* extension = GetSingleLoadedExtension();
   ASSERT_TRUE(extension) << message_;
 
   // Open an extension popup by clicking the browser action button.
   content::WindowedNotificationObserver frame_observer(
       content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME,
       content::NotificationService::AllSources());
   BrowserActionTestUtil(browser()).InspectPopup(0);
   frame_observer.Wait();
   EXPECT_TRUE(BrowserActionTestUtil(browser()).HasPopup());
 
   // Close the browser window, this should not cause a crash.
   chrome::CloseWindow(browser());
 }
-#endif  // TOOLKIT_VIEWS
 
 #if defined(OS_WIN)
 // Test that forcibly closing the browser and popup HWND does not cause a crash.
 // http://crbug.com/400646
 IN_PROC_BROWSER_TEST_F(BrowserActionInteractiveTest,
                        DISABLED_DestroyHWNDDoesNotCrash) {
   if (!ShouldRunPopupTest())
     return;
 
-  OpenExtensionPopupViaAPI();
+  OpenPopupViaAPI();
   BrowserActionTestUtil test_util(browser());
   const gfx::NativeView view = test_util.GetPopupNativeView();
   EXPECT_NE(static_cast<gfx::NativeView>(NULL), view);
   const HWND hwnd = views::HWNDForNativeView(view);
   EXPECT_EQ(hwnd,
             views::HWNDForNativeView(browser()->window()->GetNativeWindow()));
   EXPECT_EQ(TRUE, ::IsWindow(hwnd));
 
   // Create a new browser window to prevent the message loop from terminating.
   browser()->OpenURL(content::OpenURLParams(GURL("about:"), content::Referrer(),
                                             WindowOpenDisposition::NEW_WINDOW,
                                             ui::PAGE_TRANSITION_TYPED, false));
 
   // Forcibly closing the browser HWND should not cause a crash.
   EXPECT_EQ(TRUE, ::CloseWindow(hwnd));
   EXPECT_EQ(TRUE, ::DestroyWindow(hwnd));
   EXPECT_EQ(FALSE, ::IsWindow(hwnd));
 }
 #endif  // OS_WIN
 
 }  // namespace
 }  // namespace extensions