Make AppWindowWaiter a common test support

Changes: (1) move AppWindowWaiter from chromeos namespace to apps namespace (2) apply the new test support BUG=672312 TEST=automated tests. Review-Url: https://codereview.chromium.org/2576353002 Cr-Commit-Position: refs/heads/master@{#439406}
marcosholgado · Dec 19, 2016 · dcac6d9 · dcac6d9
1 parent 3583cfe
commit dcac6d9
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 75 deletions.
diff --git a/apps/BUILD.gn b/apps/BUILD.gn
@@ -71,3 +71,15 @@ static_library("apps") {
   # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
   configs += [ "//build/config/compiler:no_size_t_to_int_warning" ]
 }
+
+static_library("test_support") {
+  testonly = true
+  sources = [
+    "test/app_window_waiter.cc",
+    "test/app_window_waiter.h",
+  ]
+
+  public_deps = [
+    "//content/public/browser",
+  ]
+}
diff --git a/.../chromeos/login/test/app_window_waiter.cc → apps/test/app_window_waiter.cc b/.../chromeos/login/test/app_window_waiter.cc → apps/test/app_window_waiter.cc
@@ -2,11 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/chromeos/login/test/app_window_waiter.h"
+#include "apps/test/app_window_waiter.h"
 
 #include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/native_app_window.h"
 
-namespace chromeos {
+namespace apps {
 
 AppWindowWaiter::AppWindowWaiter(extensions::AppWindowRegistry* registry,
                                  const std::string& app_id)
@@ -42,6 +43,18 @@ extensions::AppWindow* AppWindowWaiter::WaitForShown() {
   return window_;
 }
 
+extensions::AppWindow* AppWindowWaiter::WaitForActivated() {
+  window_ = registry_->GetCurrentAppWindowForApp(app_id_);
+  if (window_ && window_->GetBaseWindow()->IsActive())
+    return window_;
+
+  wait_type_ = WAIT_FOR_ACTIVATED;
+  run_loop_.reset(new base::RunLoop);
+  run_loop_->Run();
+
+  return window_;
+}
+
 void AppWindowWaiter::OnAppWindowAdded(extensions::AppWindow* app_window) {
   if (wait_type_ != WAIT_FOR_ADDED || !run_loop_ || !run_loop_->running())
     return;
@@ -63,4 +76,14 @@ void AppWindowWaiter::OnAppWindowShown(extensions::AppWindow* app_window,
   }
 }
 
-}  // namespace chromeos
+void AppWindowWaiter::OnAppWindowActivated(extensions::AppWindow* app_window) {
+  if (wait_type_ != WAIT_FOR_ACTIVATED || !run_loop_ || !run_loop_->running())
+    return;
+
+  if (app_window->extension_id() == app_id_) {
+    window_ = app_window;
+    run_loop_->Quit();
+  }
+}
+
+}  // namespace apps
diff --git a/...r/chromeos/login/test/app_window_waiter.h → apps/test/app_window_waiter.h b/...r/chromeos/login/test/app_window_waiter.h → apps/test/app_window_waiter.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_TEST_APP_WINDOW_WAITER_H_
-#define CHROME_BROWSER_CHROMEOS_LOGIN_TEST_APP_WINDOW_WAITER_H_
+#ifndef APPS_TEST_APP_WINDOW_WAITER_H_
+#define APPS_TEST_APP_WINDOW_WAITER_H_
 
 #include <memory>
 #include <string>
@@ -17,10 +17,11 @@ namespace extensions {
 class AppWindow;
 }
 
-namespace chromeos {
+namespace apps {
 
-// Helper class that monitors app windows to wait for a window to appear.
-// Use a new instance for each use, one instance will only work for one Wait.
+// Helper class that monitors app windows to wait for a window to
+// appear/activated. Use a new instance for each use, one instance will only
+// work for one Wait.
 class AppWindowWaiter : public extensions::AppWindowRegistry::Observer {
  public:
   AppWindowWaiter(extensions::AppWindowRegistry* registry,
@@ -33,16 +34,21 @@ class AppWindowWaiter : public extensions::AppWindowRegistry::Observer {
   // Waits for an AppWindow of the app to be shown.
   extensions::AppWindow* WaitForShown();
 
+  // Waits for an AppWindow of the app to be activated.
+  extensions::AppWindow* WaitForActivated();
+
   // AppWindowRegistry::Observer:
   void OnAppWindowAdded(extensions::AppWindow* app_window) override;
   void OnAppWindowShown(extensions::AppWindow* app_window,
                         bool was_hidden) override;
+  void OnAppWindowActivated(extensions::AppWindow* app_window) override;
 
  private:
   enum WaitType {
     WAIT_FOR_NONE,
     WAIT_FOR_ADDED,
     WAIT_FOR_SHOWN,
+    WAIT_FOR_ACTIVATED,
   };
 
   extensions::AppWindowRegistry* const registry_;
@@ -54,6 +60,6 @@ class AppWindowWaiter : public extensions::AppWindowRegistry::Observer {
   DISALLOW_COPY_AND_ASSIGN(AppWindowWaiter);
 };
 
-}  // namespace chromeos
+}  // namespace apps
 
-#endif  // CHROME_BROWSER_CHROMEOS_LOGIN_TEST_APP_WINDOW_WAITER_H_
+#endif  // APPS_TEST_APP_WINDOW_WAITER_H_
diff --git a/chrome/browser/chromeos/app_mode/kiosk_crash_restore_browsertest.cc b/chrome/browser/chromeos/app_mode/kiosk_crash_restore_browsertest.cc
@@ -5,14 +5,14 @@
 #include <memory>
 #include <string>
 
+#include "apps/test/app_window_waiter.h"
 #include "base/base64.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
 #include "chrome/browser/chromeos/app_mode/fake_cws.h"
 #include "chrome/browser/chromeos/app_mode/kiosk_app_manager.h"
-#include "chrome/browser/chromeos/login/test/app_window_waiter.h"
 #include "chrome/browser/chromeos/net/network_portal_detector_test_impl.h"
 #include "chrome/browser/chromeos/ownership/owner_settings_service_chromeos_factory.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
@@ -179,7 +179,7 @@ IN_PROC_BROWSER_TEST_F(KioskCrashRestoreTest, Basic) {
   extensions::AppWindowRegistry* const app_window_registry =
       extensions::AppWindowRegistry::Get(app_profile);
   extensions::AppWindow* const window =
-      AppWindowWaiter(app_window_registry, test_app_id()).Wait();
+      apps::AppWindowWaiter(app_window_registry, test_app_id()).Wait();
   ASSERT_TRUE(window);
 
   window->GetBaseWindow()->Close();

diff --git a/chrome/browser/chromeos/login/demo_mode/demo_app_launcher_browsertest.cc b/chrome/browser/chromeos/login/demo_mode/demo_app_launcher_browsertest.cc
@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "apps/test/app_window_waiter.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/login/demo_mode/demo_app_launcher.h"
-#include "chrome/browser/chromeos/login/test/app_window_waiter.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
@@ -47,8 +47,9 @@ Profile* WaitForProfile() {
 
 bool VerifyDemoAppLaunch() {
   Profile* profile = WaitForProfile();
-  return AppWindowWaiter(extensions::AppWindowRegistry::Get(profile),
-                         DemoAppLauncher::kDemoAppId).Wait() != NULL;
+  return apps::AppWindowWaiter(extensions::AppWindowRegistry::Get(profile),
+                               DemoAppLauncher::kDemoAppId)
+             .Wait() != NULL;
 }
 
 bool VerifyNetworksDisabled() {

diff --git a/chrome/browser/chromeos/login/kiosk_browsertest.cc b/chrome/browser/chromeos/login/kiosk_browsertest.cc
@@ -5,6 +5,7 @@
 #include <memory>
 #include <vector>
 
+#include "apps/test/app_window_waiter.h"
 #include "ash/common/wallpaper/wallpaper_controller.h"
 #include "ash/common/wallpaper/wallpaper_controller_observer.h"
 #include "ash/common/wm_shell.h"
@@ -25,7 +26,6 @@
 #include "chrome/browser/chromeos/file_manager/fake_disk_mount_manager.h"
 #include "chrome/browser/chromeos/login/app_launch_controller.h"
 #include "chrome/browser/chromeos/login/startup_utils.h"
-#include "chrome/browser/chromeos/login/test/app_window_waiter.h"
 #include "chrome/browser/chromeos/login/test/oobe_base_test.h"
 #include "chrome/browser/chromeos/login/test/oobe_screen_waiter.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
@@ -670,7 +670,7 @@ class KioskTest : public OobeBaseTest {
     extensions::AppWindowRegistry* app_window_registry =
         extensions::AppWindowRegistry::Get(app_profile);
     extensions::AppWindow* window =
-        AppWindowWaiter(app_window_registry, test_app_id_).Wait();
+        apps::AppWindowWaiter(app_window_registry, test_app_id_).Wait();
     EXPECT_TRUE(window);
 
     // Login screen should be gone or fading out.
@@ -860,7 +860,7 @@ IN_PROC_BROWSER_TEST_F(KioskTest, ZoomSupport) {
   extensions::AppWindowRegistry* app_window_registry =
       extensions::AppWindowRegistry::Get(app_profile);
   extensions::AppWindow* window =
-      AppWindowWaiter(app_window_registry, test_app_id()).Wait();
+      apps::AppWindowWaiter(app_window_registry, test_app_id()).Wait();
   ASSERT_TRUE(window);
 
   // Gets the original width of the app window.
@@ -2248,10 +2248,10 @@ IN_PROC_BROWSER_TEST_F(KioskEnterpriseTest, EnterpriseKioskApp) {
 
   // Wait for the window to appear.
   extensions::AppWindow* window =
-      AppWindowWaiter(
-          extensions::AppWindowRegistry::Get(
-              ProfileManager::GetPrimaryUserProfile()),
-          kTestEnterpriseKioskApp).Wait();
+      apps::AppWindowWaiter(extensions::AppWindowRegistry::Get(
+                                ProfileManager::GetPrimaryUserProfile()),
+                            kTestEnterpriseKioskApp)
+          .Wait();
   ASSERT_TRUE(window);
 
   // Check whether the app can retrieve an OAuth2 access token.

diff --git a/chrome/browser/chromeos/login/ui/login_feedback_browsertest.cc b/chrome/browser/chromeos/login/ui/login_feedback_browsertest.cc
@@ -6,11 +6,11 @@
 
 #include <memory>
 
+#include "apps/test/app_window_waiter.h"
 #include "ash/wm/window_util.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "chrome/browser/chromeos/login/login_manager_test.h"
-#include "chrome/browser/chromeos/login/test/app_window_waiter.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "extensions/browser/app_window/app_window.h"
@@ -38,8 +38,8 @@ IN_PROC_BROWSER_TEST_F(LoginFeedbackTest, Basic) {
   login_feedback->Request("Test feedback", run_loop.QuitClosure());
 
   extensions::AppWindow* feedback_window =
-      AppWindowWaiter(extensions::AppWindowRegistry::Get(profile),
-                      extension_misc::kFeedbackExtensionId)
+      apps::AppWindowWaiter(extensions::AppWindowRegistry::Get(profile),
+                            extension_misc::kFeedbackExtensionId)
           .WaitForShown();
   ASSERT_NE(nullptr, feedback_window);
   EXPECT_FALSE(feedback_window->is_hidden());

diff --git a/chrome/browser/extensions/api/tabs/tabs_test.cc b/chrome/browser/extensions/api/tabs/tabs_test.cc
@@ -9,6 +9,7 @@
 #include <memory>
 #include <string>
 
+#include "apps/test/app_window_waiter.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/pattern.h"
 #include "base/strings/string_split.h"
@@ -45,7 +46,6 @@
 #include "extensions/browser/api_test_utils.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_registry.h"
-#include "extensions/browser/app_window/native_app_window.h"
 #include "extensions/common/manifest_constants.h"
 #include "extensions/common/test_util.h"
 #include "extensions/test/extension_test_message_listener.h"
@@ -798,13 +798,13 @@ IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, UpdateDevToolsWindow) {
 // TODO(llandwerlin): Activating a browser window and waiting for the
 // action to happen requires views::Widget which is not available on
 // MacOSX. Deactivate for now.
+// TODO(warx): Move ExtensionWindowLastFocusedTest to interactive
+// uitest as it triggers native widget activation.
 #if !defined(OS_MACOSX)
 class ExtensionWindowLastFocusedTest : public ExtensionTabsTest {
  public:
   void SetUpOnMainThread() override;
 
-  void ActivateAppWindow(AppWindow* app_window);
-
   void ActivateBrowserWindow(Browser* browser);
 
   Browser* CreateBrowserWithEmptyTab(bool as_popup);
@@ -815,44 +815,6 @@ class ExtensionWindowLastFocusedTest : public ExtensionTabsTest {
                            const std::string& params);
 
  private:
-  // A helper class to wait for an AppWindow to become activated. On
-  // window system like X11, for a NativeWidget to be activated, we
-  // need to wait for the round trip communication with the X server.
-  class AppWindowActivatedWaiter : public AppWindowRegistry::Observer {
-   public:
-    AppWindowActivatedWaiter(AppWindow* app_window,
-                             content::BrowserContext* browser_context)
-        : app_window_(app_window),
-          browser_context_(browser_context),
-          waiting_(false) {
-      AppWindowRegistry::Get(browser_context_)->AddObserver(this);
-    }
-    ~AppWindowActivatedWaiter() override {
-      AppWindowRegistry::Get(browser_context_)->RemoveObserver(this);
-    }
-
-    void ActivateAndWait() {
-      app_window_->GetBaseWindow()->Activate();
-      if (!app_window_->GetBaseWindow()->IsActive()) {
-        waiting_ = true;
-        content::RunMessageLoop();
-      }
-    }
-
-    // AppWindowRegistry::Observer:
-    void OnAppWindowActivated(AppWindow* app_window) override {
-      if (app_window_ == app_window && waiting_) {
-        base::MessageLoopForUI::current()->QuitWhenIdle();
-        waiting_ = false;
-      }
-    }
-
-   private:
-    AppWindow* app_window_;
-    content::BrowserContext* browser_context_;
-    bool waiting_;
-  };
-
   // A helper class to wait for an views::Widget to become activated.
   class WidgetActivatedWaiter : public views::WidgetObserver {
    public:
@@ -892,11 +854,6 @@ void ExtensionWindowLastFocusedTest::SetUpOnMainThread() {
   extension_ = test_util::CreateEmptyExtension();
 }
 
-void ExtensionWindowLastFocusedTest::ActivateAppWindow(AppWindow* app_window) {
-  AppWindowActivatedWaiter waiter(app_window, browser()->profile());
-  waiter.ActivateAndWait();
-}
-
 void ExtensionWindowLastFocusedTest::ActivateBrowserWindow(Browser* browser) {
   BrowserView* view = BrowserView::GetBrowserViewForBrowser(browser);
   EXPECT_NE(nullptr, view);
@@ -993,7 +950,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionWindowLastFocusedTest,
       " \"minWidth\": 200, \"minHeight\": 200,"
       " \"maxWidth\": 400, \"maxHeight\": 400}}");
   {
-    ActivateAppWindow(app_window);
+    apps::AppWindowWaiter waiter(AppWindowRegistry::Get(browser()->profile()),
+                                 app_window->extension_id());
+    waiter.WaitForActivated();
 
     scoped_refptr<WindowsGetLastFocusedFunction> get_current_app_function =
         new WindowsGetLastFocusedFunction();
@@ -1065,7 +1024,9 @@ IN_PROC_BROWSER_TEST_F(ExtensionWindowLastFocusedTest,
       " \"minWidth\": 200, \"minHeight\": 200,"
       " \"maxWidth\": 400, \"maxHeight\": 400}}");
   {
-    ActivateAppWindow(app_window);
+    apps::AppWindowWaiter waiter(AppWindowRegistry::Get(browser()->profile()),
+                                 app_window->extension_id());
+    waiter.WaitForActivated();
 
     scoped_refptr<WindowsGetLastFocusedFunction> get_current_app_function =
         new WindowsGetLastFocusedFunction();

diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
@@ -187,6 +187,7 @@ static_library("test_support") {
 
   if (enable_extensions) {
     public_deps += [
+      "//apps:test_support",
       "//chrome/common/extensions/api",
       "//extensions:test_support",
     ]
@@ -248,6 +249,7 @@ static_library("test_support") {
 
   if (enable_extensions) {
     public_deps += [
+      "//apps:test_support",
       "//chrome/common/extensions/api",
       "//extensions:test_support",
     ]
@@ -2258,8 +2260,6 @@ test("browser_tests") {
         "../browser/chromeos/login/supervised/supervised_user_password_browsertest.cc",
         "../browser/chromeos/login/supervised/supervised_user_test_base.cc",
         "../browser/chromeos/login/supervised/supervised_user_test_base.h",
-        "../browser/chromeos/login/test/app_window_waiter.cc",
-        "../browser/chromeos/login/test/app_window_waiter.h",
         "../browser/chromeos/login/test/https_forwarder.cc",
         "../browser/chromeos/login/test/https_forwarder.h",
         "../browser/chromeos/login/test/oobe_base_test.cc",