ShellTestBaseAura for AppShell unit tests

ShellTestBaseAura augments extensions::ExtensionsTest and also sets up
the Aura environment. app_shell_unittests test classes can derive from
ShellTestBaseAura to get this set-up for free, instead of deriving from
either ExtensionsTest (which doesn't create a
ShellExtensionsBrowserClient) or AuraTestBase (which sets up its own
Screen and RootWindow).

In particular, deriving ShellDesktopControllerAuraTest from AuraTestBase
doesn't make sense because the ShellDesktopControllerAura creates
its own screen and RootWindow.

This also provides ShellTestHelperAura, which can be used as a class
member to get shell environment set-up without deriving a test from
ExtensionsTest.

Change-Id: I4565c985c77444ebc7325fd7d5295d028f85f774
Reviewed-on: https://chromium-review.googlesource.com/591641
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491934}

extensions/browser/api/runtime/restart_after_delay_api_unittest.cc

@@ -98,14 +98,15 @@ class RestartAfterDelayApiTest : public ApiUnitTest {
   ~RestartAfterDelayApiTest() override {}
 
   void SetUp() override {
-    ApiUnitTest::SetUp();
-
     // Use our ExtensionsBrowserClient that returns our RuntimeAPIDelegate.
-    test_browser_client_.reset(
-        new DelayedRestartExtensionsBrowserClient(browser_context()));
-    test_browser_client_->set_extension_system_factory(
-        extensions_browser_client()->extension_system_factory());
-    ExtensionsBrowserClient::Set(test_browser_client_.get());
+    std::unique_ptr<DelayedRestartExtensionsBrowserClient> test_browser_client =
+        base::MakeUnique<DelayedRestartExtensionsBrowserClient>(
+            browser_context());
+
+    // ExtensionsTest takes ownership of the ExtensionsBrowserClient.
+    SetExtensionsBrowserClient(std::move(test_browser_client));
+
+    ApiUnitTest::SetUp();
 
     // The RuntimeAPI should only be accessed (i.e. constructed) after the above
     // ExtensionsBrowserClient has been setup.
@@ -116,11 +117,17 @@ class RestartAfterDelayApiTest : public ApiUnitTest {
     runtime_api->AllowNonKioskAppsInRestartAfterDelayForTesting();
 
     RuntimeAPI::RegisterPrefs(
-        test_browser_client_->testing_pref_service()->registry());
+        static_cast<DelayedRestartExtensionsBrowserClient*>(
+            extensions_browser_client())
+            ->testing_pref_service()
+            ->registry());
   }
 
   base::TimeTicks WaitForSuccessfulRestart() {
-    return test_browser_client_->api_delegate()->WaitForSuccessfulRestart();
+    return static_cast<DelayedRestartExtensionsBrowserClient*>(
+               extensions_browser_client())
+        ->api_delegate()
+        ->WaitForSuccessfulRestart();
   }
 
   bool IsDelayedRestartTimerRunning() {
@@ -166,8 +173,6 @@ class RestartAfterDelayApiTest : public ApiUnitTest {
     return function->GetError();
   }
 
-  std::unique_ptr<DelayedRestartExtensionsBrowserClient> test_browser_client_;
-
   DISALLOW_COPY_AND_ASSIGN(RestartAfterDelayApiTest);
 };
 

extensions/browser/extensions_test.cc

@@ -51,13 +51,24 @@ ExtensionsTest::~ExtensionsTest() {
   content::SetUtilityClientForTesting(nullptr);
 }
 
+void ExtensionsTest::SetExtensionsBrowserClient(
+    std::unique_ptr<TestExtensionsBrowserClient> extensions_browser_client) {
+  DCHECK(extensions_browser_client);
+  DCHECK(!extensions_browser_client_);
+  extensions_browser_client_ = std::move(extensions_browser_client);
+}
+
 void ExtensionsTest::SetUp() {
   content_browser_client_ = base::MakeUnique<TestContentBrowserClient>();
   content_utility_client_ = base::MakeUnique<TestContentUtilityClient>();
   browser_context_ = base::MakeUnique<content::TestBrowserContext>();
   incognito_context_ = CreateTestIncognitoContext();
-  extensions_browser_client_ =
-      base::MakeUnique<TestExtensionsBrowserClient>(browser_context_.get());
+
+  if (!extensions_browser_client_) {
+    extensions_browser_client_ =
+        base::MakeUnique<TestExtensionsBrowserClient>();
+  }
+  extensions_browser_client_->SetMainContext(browser_context_.get());
 
   BrowserContextDependencyManager::GetInstance()->MarkBrowserContextLive(
       browser_context_.get());

extensions/browser/extensions_test.h

@@ -43,6 +43,11 @@ class ExtensionsTest : public testing::Test {
       std::unique_ptr<content::TestBrowserThreadBundle> thread_bundle);
   ~ExtensionsTest() override;
 
+  // Allows setting a custom TestExtensionsBrowserClient. Must only be called
+  // before SetUp().
+  void SetExtensionsBrowserClient(
+      std::unique_ptr<TestExtensionsBrowserClient> extensions_browser_client);
+
   // Returned as a BrowserContext since most users don't need methods from
   // TestBrowserContext.
   content::BrowserContext* browser_context() { return browser_context_.get(); }

extensions/browser/test_extensions_browser_client.cc

@@ -22,16 +22,19 @@ namespace extensions {
 
 TestExtensionsBrowserClient::TestExtensionsBrowserClient(
     BrowserContext* main_context)
-    : main_context_(main_context),
+    : main_context_(nullptr),
       incognito_context_(nullptr),
       lock_screen_context_(nullptr),
       process_manager_delegate_(nullptr),
       extension_system_factory_(nullptr),
       extension_cache_(new NullExtensionCache) {
-  DCHECK(main_context_);
-  DCHECK(!main_context_->IsOffTheRecord());
+  if (main_context)
+    SetMainContext(main_context);
 }
 
+TestExtensionsBrowserClient::TestExtensionsBrowserClient()
+    : TestExtensionsBrowserClient(nullptr) {}
+
 TestExtensionsBrowserClient::~TestExtensionsBrowserClient() {}
 
 void TestExtensionsBrowserClient::SetUpdateClientFactory(
@@ -39,6 +42,14 @@ void TestExtensionsBrowserClient::SetUpdateClientFactory(
   update_client_factory_ = factory;
 }
 
+void TestExtensionsBrowserClient::SetMainContext(
+    content::BrowserContext* main_context) {
+  DCHECK(!main_context_);
+  DCHECK(main_context);
+  DCHECK(!main_context->IsOffTheRecord());
+  main_context_ = main_context;
+}
+
 void TestExtensionsBrowserClient::SetIncognitoContext(BrowserContext* context) {
   // If a context is provided it must be off-the-record.
   DCHECK(!context || context->IsOffTheRecord());

extensions/browser/test_extensions_browser_client.h

@@ -27,8 +27,10 @@ class KioskDelegate;
 // this class should call ExtensionsBrowserClient::Set() with its instance.
 class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
  public:
-  // |main_context| is required and must not be an incognito context.
+  // If provided, |main_context| must not be an incognito context.
   explicit TestExtensionsBrowserClient(content::BrowserContext* main_context);
+  // Alternate constructor allowing |main_context_| to be set later.
+  TestExtensionsBrowserClient();
   ~TestExtensionsBrowserClient() override;
 
   void set_process_manager_delegate(ProcessManagerDelegate* delegate) {
@@ -49,6 +51,10 @@ class TestExtensionsBrowserClient : public ExtensionsBrowserClient {
   void SetUpdateClientFactory(
       const base::Callback<update_client::UpdateClient*(void)>& factory);
 
+  // Sets the main browser context. Only call if a BrowserContext was not
+  // already provided. |main_context| must not be an incognito context.
+  void SetMainContext(content::BrowserContext* main_context);
+
   // Associates an incognito context with |main_context_|.
   void SetIncognitoContext(content::BrowserContext* incognito_context);
 

extensions/shell/BUILD.gn

@@ -278,6 +278,8 @@ test("app_shell_unittests") {
     "browser/shell_oauth2_token_service_unittest.cc",
     "browser/shell_prefs_unittest.cc",
     "common/shell_content_client_unittest.cc",
+    "test/shell_test_extensions_browser_client.cc",
+    "test/shell_test_extensions_browser_client.h",
   ]
 
   data = [
@@ -295,6 +297,8 @@ test("app_shell_unittests") {
     "//content/test:test_support",
     "//extensions:shell_and_test_pak",
     "//extensions:test_support",
+    "//extensions/browser",
+    "//extensions/common",
     "//testing/gtest",
   ]
 
@@ -308,8 +312,16 @@ test("app_shell_unittests") {
       "browser/shell_desktop_controller_aura_unittest.cc",
       "browser/shell_native_app_window_aura_unittest.cc",
       "browser/shell_screen_unittest.cc",
+      "test/shell_test_base_aura.cc",
+      "test/shell_test_base_aura.h",
+      "test/shell_test_helper_aura.cc",
+      "test/shell_test_helper_aura.h",
+    ]
+    deps += [
+      "//ui/aura:test_support",
+      "//ui/base:test_support",
+      "//ui/compositor:test_support",
     ]
-    deps += [ "//ui/aura:test_support" ]
   }
 
   if (is_chromeos) {

extensions/shell/browser/shell_desktop_controller_aura.h

@@ -70,6 +70,8 @@ class ShellDesktopControllerAura
   ShellDesktopControllerAura();
   ~ShellDesktopControllerAura() override;
 
+  aura::WindowTreeHost* host() { return host_.get(); }
+
   // DesktopController:
   gfx::Size GetWindowSize() override;
   AppWindow* CreateAppWindow(content::BrowserContext* context,
@@ -105,8 +107,6 @@ class ShellDesktopControllerAura
   virtual void InitWindowManager();
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(ShellDesktopControllerAuraTest, InputEvents);
-
   // Creates the window that hosts the app.
   void CreateRootWindow();
 

extensions/shell/browser/shell_desktop_controller_aura_unittest.cc

@@ -9,7 +9,17 @@
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/time/time.h"
-#include "ui/aura/test/aura_test_base.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/web_contents.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/test_app_window_contents.h"
+#include "extensions/common/test_util.h"
+#include "extensions/shell/browser/shell_app_delegate.h"
+#include "extensions/shell/test/shell_test_base_aura.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/aura/window.h"
+#include "ui/aura/window_event_dispatcher.h"
+#include "ui/aura/window_tree_host.h"
 #include "ui/base/ime/dummy_text_input_client.h"
 #include "ui/base/ime/input_method.h"
 #include "ui/base/ime/input_method_factory.h"
@@ -17,6 +27,7 @@
 #include "ui/events/event.h"
 #include "ui/events/event_dispatcher.h"
 #include "ui/events/keycodes/keyboard_codes.h"
+#include "ui/gfx/geometry/rect.h"
 
 #if defined(OS_CHROMEOS)
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -25,9 +36,7 @@
 
 namespace extensions {
 
-// TODO(michaelpg): Why do we use AuraTestBase when ShellDesktopControllerAura
-// already creates a screen and root window host itself?
-class ShellDesktopControllerAuraTest : public aura::test::AuraTestBase {
+class ShellDesktopControllerAuraTest : public ShellTestBaseAura {
  public:
   ShellDesktopControllerAuraTest()
 #if defined(OS_CHROMEOS)
@@ -44,7 +53,8 @@ class ShellDesktopControllerAuraTest : public aura::test::AuraTestBase {
     power_manager_client_ = new chromeos::FakePowerManagerClient();
     dbus_setter->SetPowerManagerClient(base::WrapUnique(power_manager_client_));
 #endif
-    aura::test::AuraTestBase::SetUp();
+
+    ShellTestBaseAura::SetUp();
 
     // The input method will be used for the next CreateInputMethod call,
     // causing the host to take ownership.
@@ -55,7 +65,7 @@ class ShellDesktopControllerAuraTest : public aura::test::AuraTestBase {
 
   void TearDown() override {
     controller_.reset();
-    aura::test::AuraTestBase::TearDown();
+    ShellTestBaseAura::TearDown();
 #if defined(OS_CHROMEOS)
     chromeos::DBusThreadManager::Shutdown();
 #endif
@@ -91,7 +101,7 @@ TEST_F(ShellDesktopControllerAuraTest, PowerButton) {
 // Tests that basic input events are handled and forwarded to the host.
 // TODO(michaelpg): Test other types of input.
 TEST_F(ShellDesktopControllerAuraTest, InputEvents) {
-  ui::InputMethod* input_method = controller_->host_->GetInputMethod();
+  ui::InputMethod* input_method = controller_->host()->GetInputMethod();
   ASSERT_TRUE(input_method);
 
   // Set up a focused text input to receive the keypress event.
@@ -102,8 +112,8 @@ TEST_F(ShellDesktopControllerAuraTest, InputEvents) {
   // Dispatch a keypress on the window tree host to verify it is processed.
   ui::KeyEvent key_press(base::char16(97), ui::VKEY_A, ui::EF_NONE);
   ui::EventDispatchDetails details =
-      controller_->host_->dispatcher()->DispatchEvent(
-          controller_->host_->window(), &key_press);
+      controller_->host()->dispatcher()->DispatchEvent(
+          controller_->host()->window(), &key_press);
   EXPECT_FALSE(details.dispatcher_destroyed);
   EXPECT_FALSE(details.target_destroyed);
   EXPECT_TRUE(key_press.handled());
@@ -113,4 +123,35 @@ TEST_F(ShellDesktopControllerAuraTest, InputEvents) {
   input_method->DetachTextInputClient(&client);
 }
 
+// Tests the basic window layout.
+TEST_F(ShellDesktopControllerAuraTest, FillLayout) {
+  controller_->host()->SetBoundsInPixels(gfx::Rect(0, 0, 500, 700));
+
+  scoped_refptr<Extension> extension = test_util::CreateEmptyExtension();
+  AppWindow* app_window =
+      controller_->CreateAppWindow(browser_context(), extension.get());
+
+  content::WebContents* web_contents = content::WebContents::Create(
+      content::WebContents::CreateParams(browser_context()));
+  TestAppWindowContents* app_window_contents =
+      new TestAppWindowContents(web_contents);
+  DCHECK_EQ(web_contents, app_window_contents->GetWebContents());
+
+  // Init the ShellExtensionsWebContentsObserver.
+  app_window->app_delegate()->InitWebContents(web_contents);
+  EXPECT_TRUE(web_contents->GetMainFrame());
+
+  app_window->Init(GURL(std::string()), app_window_contents,
+                   web_contents->GetMainFrame(), AppWindow::CreateParams());
+
+  aura::Window* root_window = controller_->host()->window();
+  EXPECT_EQ(1u, root_window->children().size());
+
+  // Test that reshaping the host window also resizes the child window.
+  controller_->host()->SetBoundsInPixels(gfx::Rect(0, 0, 400, 300));
+
+  // TODO(michaelpg): Check the sizes of the window and its child after fixing
+  // crbug.com/750282.
+}
+
 }  // namespace extensions

extensions/shell/test/DEPS

@@ -5,3 +5,10 @@ include_rules = [
   # Testing utilities can access anything in extensions/shell.
   "+extensions/shell",
 ]
+
+specific_include_rules = {
+  ".*_aura.*": [
+    "+ui/aura",
+    "+ui/compositor",
+  ],
+}

extensions/shell/test/shell_test_base_aura.cc

+// Copyright 2017 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 "extensions/shell/test/shell_test_base_aura.h"
+
+#include "base/memory/ptr_util.h"
+#include "content/public/test/test_browser_thread_bundle.h"
+#include "extensions/shell/test/shell_test_extensions_browser_client.h"
+#include "extensions/shell/test/shell_test_helper_aura.h"
+
+namespace extensions {
+
+ShellTestBaseAura::ShellTestBaseAura()
+    : ExtensionsTest(base::MakeUnique<content::TestBrowserThreadBundle>()) {}
+
+ShellTestBaseAura::~ShellTestBaseAura() {}
+
+void ShellTestBaseAura::SetUp() {
+  helper_ = base::MakeUnique<ShellTestHelperAura>();
+  helper_->SetUp();
+  std::unique_ptr<TestExtensionsBrowserClient> extensions_browser_client =
+      base::MakeUnique<ShellTestExtensionsBrowserClient>();
+  SetExtensionsBrowserClient(std::move(extensions_browser_client));
+  ExtensionsTest::SetUp();
+}
+
+void ShellTestBaseAura::TearDown() {
+  ExtensionsTest::TearDown();
+  helper_->TearDown();
+}
+
+}  // namespace extensions

extensions/shell/test/shell_test_base_aura.h

+// Copyright 2017 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.
+
+#ifndef EXTENSIONS_SHELL_TEST_SHELL_TEST_BASE_AURA_H_
+#define EXTENSIONS_SHELL_TEST_SHELL_TEST_BASE_AURA_H_
+
+#include <memory>
+
+#include "base/macros.h"
+
+#include "extensions/browser/extensions_test.h"
+
+namespace extensions {
+class ShellTestHelperAura;
+
+class ShellTestBaseAura : public ExtensionsTest {
+ public:
+  ShellTestBaseAura();
+  ~ShellTestBaseAura() override;
+
+  // ExtensionsTest:
+  void SetUp() override;
+  void TearDown() override;
+
+ private:
+  std::unique_ptr<ShellTestHelperAura> helper_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShellTestBaseAura);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_SHELL_TEST_SHELL_TEST_BASE_AURA_H_

extensions/shell/test/shell_test_extensions_browser_client.cc

+// Copyright 2017 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 "extensions/shell/test/shell_test_extensions_browser_client.h"
+
+#include "build/build_config.h"
+#include "extensions/shell/browser/shell_extension_web_contents_observer.h"
+
+namespace extensions {
+
+ShellTestExtensionsBrowserClient::ShellTestExtensionsBrowserClient(
+    content::BrowserContext* main_context)
+    : TestExtensionsBrowserClient(main_context) {}
+
+ShellTestExtensionsBrowserClient::ShellTestExtensionsBrowserClient() {}
+
+ShellTestExtensionsBrowserClient::~ShellTestExtensionsBrowserClient() {}
+
+ExtensionWebContentsObserver*
+ShellTestExtensionsBrowserClient::GetExtensionWebContentsObserver(
+    content::WebContents* web_contents) {
+  return ShellExtensionWebContentsObserver::FromWebContents(web_contents);
+}
+
+}  // namespace extensions

extensions/shell/test/shell_test_extensions_browser_client.h

+// Copyright 2017 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.
+
+#ifndef EXTENSIONS_SHELL_TEST_SHELL_TEST_EXTENSIONS_BROWSER_CLIENT_H_
+#define EXTENSIONS_SHELL_TEST_SHELL_TEST_EXTENSIONS_BROWSER_CLIENT_H_
+
+#include "base/macros.h"
+#include "extensions/browser/test_extensions_browser_client.h"
+
+namespace extensions {
+
+// A TestExtensionsBrowserClient for AppShell tests.
+class ShellTestExtensionsBrowserClient : public TestExtensionsBrowserClient {
+ public:
+  // If provided, |main_context| must not be an incognito context.
+  explicit ShellTestExtensionsBrowserClient(
+      content::BrowserContext* main_context);
+  ShellTestExtensionsBrowserClient();
+  ~ShellTestExtensionsBrowserClient() override;
+
+  // ExtensionsBrowserClient overrides:
+  ExtensionWebContentsObserver* GetExtensionWebContentsObserver(
+      content::WebContents* web_contents) override;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ShellTestExtensionsBrowserClient);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_SHELL_TEST_SHELL_TEST_EXTENSIONS_BROWSER_CLIENT_H_

extensions/shell/test/shell_test_helper_aura.cc

+// Copyright 2017 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 "extensions/shell/test/shell_test_helper_aura.h"
+
+#include "ui/aura/test/aura_test_helper.h"
+#include "ui/compositor/compositor.h"
+#include "ui/compositor/test/context_factories_for_test.h"
+
+namespace extensions {
+
+ShellTestHelperAura::ShellTestHelperAura() {}
+
+ShellTestHelperAura::~ShellTestHelperAura() {}
+
+void ShellTestHelperAura::SetUp() {
+  // The ContextFactory must exist before any Compositors are created.
+  ui::ContextFactory* context_factory = nullptr;
+  ui::ContextFactoryPrivate* context_factory_private = nullptr;
+  ui::InitializeContextFactoryForTests(/*enable_pixel_output=*/false,
+                                       &context_factory,
+                                       &context_factory_private);
+
+  // AuraTestHelper sets up the rest of the Aura initialization.
+  helper_.reset(new aura::test::AuraTestHelper());
+  helper_->SetUp(context_factory, context_factory_private);
+}
+
+void ShellTestHelperAura::TearDown() {
+  helper_->RunAllPendingInMessageLoop();
+  helper_->TearDown();
+  ui::TerminateContextFactoryForTests();
+}
+
+}  // namespace extensions

extensions/shell/test/shell_test_helper_aura.h

+// Copyright 2017 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.
+
+#ifndef EXTENSIONS_SHELL_BROWSER_TEST_SHELL_TEST_HELPER_AURA_H_
+#define EXTENSIONS_SHELL_BROWSER_TEST_SHELL_TEST_HELPER_AURA_H_
+
+#include <memory>
+
+#include "base/macros.h"
+
+namespace aura {
+namespace test {
+class AuraTestHelper;
+}
+}  // namespace aura
+
+namespace extensions {
+
+// A helper class that does common Aura initialization required for the shell.
+class ShellTestHelperAura {
+ public:
+  ShellTestHelperAura();
+  ~ShellTestHelperAura();
+
+  // Initializes common test dependencies.
+  void SetUp();
+
+  // Cleans up.
+  void TearDown();
+
+ private:
+  std::unique_ptr<aura::test::AuraTestHelper> helper_;
+
+  DISALLOW_COPY_AND_ASSIGN(ShellTestHelperAura);
+};
+
+}  // namespace extensions
+
+#endif  // EXTENSIONS_SHELL_BROWSER_TEST_SHELL_TEST_HELPER_AURA_H_