aura: cleanup mus related test helper

AuraTestHelper now always create a WindowTreeHost. This resulted in being
able to remove some test code around configuring when to create the
WindowTreeHost. The only ick that was needed to support this is in AxRemoteHost.
AxRemoteHost assumed there was only one root, which isn't the case in test
code anymore. I'm hoping that once we make AxRemoteHost support more than one
root this code will go away.

There was also some mild trickery around window ids. In particular
WindowTreeClientPrivate has some functions that pass ids to WindowTreeClient.
These ids are used in creating windows, and must be unique. Because
AuraTestHelper now always triggers creating a WindowTreeHost, the id must be
increased. I've hopefully done things so tests shouldn't have to worry about
this.

BUG=866634
TEST=covered by tests

Change-Id: Ia3d2cd757555478f3cce8929574bc8df8acb3af2
Reviewed-on: https://chromium-review.googlesource.com/1153773
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579531}

ui/aura/mus/drag_drop_controller_mus_unittest.cc

@@ -23,15 +23,13 @@
 namespace aura {
 namespace {
 
-class DragDropControllerMusTest : public test::AuraTestBase {
+class DragDropControllerMusTest : public test::AuraMusClientTestBase {
  public:
   DragDropControllerMusTest() = default;
 
-  // test::AuraMusClientTestBase
+  // test::AuraMusClientTestBase:
   void SetUp() override {
-    ConfigureEnvMode(Env::Mode::MUS);
-    SetCreateHostForPrimaryDisplay(true);
-    AuraTestBase::SetUp();
+    AuraMusClientTestBase::SetUp();
     controller_ = std::make_unique<DragDropControllerMus>(&controller_host_,
                                                           window_tree());
     window_ =
@@ -41,7 +39,7 @@ class DragDropControllerMusTest : public test::AuraTestBase {
   void TearDown() override {
     window_.reset();
     controller_.reset();
-    AuraTestBase::TearDown();
+    AuraMusClientTestBase::TearDown();
   }
 
  protected:

ui/aura/test/aura_mus_test_base.cc

@@ -12,13 +12,7 @@ AuraMusClientTestBase::AuraMusClientTestBase() = default;
 AuraMusClientTestBase::~AuraMusClientTestBase() = default;
 
 void AuraMusClientTestBase::SetUp() {
-  // Run AuraTestBase::SetUp() first because it puts an InProcessContextFactory
-  // in env.
   ConfigureEnvMode(Env::Mode::MUS);
-  // Don't create a WindowTreeHost by default.
-  // TODO(sky): update tests so that this isn't necessary (make the test code
-  // always create a host for the primary display). https://crbug.com/866634
-  SetCreateHostForPrimaryDisplay(false);
   AuraTestBase::SetUp();
 }
 

ui/aura/test/aura_test_base.cc

@@ -94,8 +94,6 @@ void AuraTestBase::SetUp() {
   helper_ = std::make_unique<AuraTestHelper>();
   if (env_mode_ == Env::Mode::MUS)
     helper_->EnableMusWithTestWindowTree(window_tree_client_delegate_);
-  helper_->set_create_host_for_primary_display(
-      create_host_for_primary_display_);
   helper_->SetUp(context_factory, context_factory_private);
 }
 
@@ -158,11 +156,6 @@ ui::mojom::WindowTreeClient* AuraTestBase::window_tree_client() {
   return helper_->window_tree_client();
 }
 
-void AuraTestBase::SetCreateHostForPrimaryDisplay(bool value) {
-  DCHECK(!setup_called_);
-  create_host_for_primary_display_ = value;
-}
-
 void AuraTestBase::OnEmbed(
     std::unique_ptr<WindowTreeHostMus> window_tree_host) {}
 

ui/aura/test/aura_test_base.h

@@ -93,10 +93,6 @@ class AuraTestBase : public testing::Test, public WindowTreeClientDelegate {
     return observed_pointer_events_;
   }
 
-  // See AuraTestHelper for details on what this does. Must be called before
-  // SetUp().
-  void SetCreateHostForPrimaryDisplay(bool value);
-
   // WindowTreeClientDelegate:
   void OnEmbed(std::unique_ptr<WindowTreeHostMus> window_tree_host) override;
   void OnUnembed(Window* root) override;
@@ -120,7 +116,6 @@ class AuraTestBase : public testing::Test, public WindowTreeClientDelegate {
   std::unique_ptr<AuraTestHelper> helper_;
   std::unique_ptr<AuraTestContextFactory> mus_context_factory_;
   std::vector<std::unique_ptr<ui::PointerEvent>> observed_pointer_events_;
-  bool create_host_for_primary_display_ = true;
 
   DISALLOW_COPY_AND_ASSIGN(AuraTestBase);
 };

ui/aura/test/aura_test_helper.cc

@@ -20,6 +20,7 @@
 #include "ui/aura/test/mus/test_window_tree.h"
 #include "ui/aura/test/mus/test_window_tree_client_delegate.h"
 #include "ui/aura/test/mus/test_window_tree_client_setup.h"
+#include "ui/aura/test/mus/window_tree_client_private.h"
 #include "ui/aura/test/test_focus_client.h"
 #include "ui/aura/test/test_screen.h"
 #include "ui/aura/test/test_window_parenting_client.h"
@@ -146,30 +147,26 @@ void AuraTestHelper::SetUp(ui::ContextFactory* context_factory,
 
   ui::InitializeInputMethodForTesting();
 
-  if (mode_ != Mode::MUS_DONT_CREATE_WINDOW_TREE_CLIENT) {
-    display::Screen* screen = display::Screen::GetScreen();
-    gfx::Size host_size(screen ? screen->GetPrimaryDisplay().GetSizeInPixel()
-                               : gfx::Size(800, 600));
-    // This must be reset before creating TestScreen, which sets up the display
-    // scale factor for this test iteration.
-    display::Display::ResetForceDeviceScaleFactorForTesting();
-    test_screen_.reset(TestScreen::Create(host_size, window_tree_client_));
-    if (!screen)
-      display::Screen::SetScreenInstance(test_screen_.get());
-    if (env_mode == Env::Mode::LOCAL || create_host_for_primary_display_) {
-      host_.reset(test_screen_->CreateHostForPrimaryDisplay());
-      host_->window()->SetEventTargeter(
-          std::unique_ptr<ui::EventTargeter>(new WindowTargeter()));
-
-      client::SetFocusClient(root_window(), focus_client_.get());
-      client::SetCaptureClient(root_window(), capture_client());
-      parenting_client_.reset(new TestWindowParentingClient(root_window()));
-
-      root_window()->Show();
-      // Ensure width != height so tests won't confuse them.
-      host()->SetBoundsInPixels(gfx::Rect(host_size));
-    }
-  }
+  display::Screen* screen = display::Screen::GetScreen();
+  gfx::Size host_size(screen ? screen->GetPrimaryDisplay().GetSizeInPixel()
+                             : gfx::Size(800, 600));
+  // This must be reset before creating TestScreen, which sets up the display
+  // scale factor for this test iteration.
+  display::Display::ResetForceDeviceScaleFactorForTesting();
+  test_screen_.reset(TestScreen::Create(host_size, window_tree_client_));
+  if (!screen)
+    display::Screen::SetScreenInstance(test_screen_.get());
+  host_.reset(test_screen_->CreateHostForPrimaryDisplay());
+  host_->window()->SetEventTargeter(
+      std::unique_ptr<ui::EventTargeter>(new WindowTargeter()));
+
+  client::SetFocusClient(root_window(), focus_client_.get());
+  client::SetCaptureClient(root_window(), capture_client());
+  parenting_client_.reset(new TestWindowParentingClient(root_window()));
+
+  root_window()->Show();
+  // Ensure width != height so tests won't confuse them.
+  host()->SetBoundsInPixels(gfx::Rect(host_size));
 
   if (mode_ == Mode::MUS_CREATE_WINDOW_TREE_CLIENT) {
     window_tree_client_->focus_synchronizer()->SetActiveFocusClient(
@@ -185,24 +182,18 @@ void AuraTestHelper::TearDown() {
   teardown_called_ = true;
   parenting_client_.reset();
   env_window_tree_client_setter_.reset();
-  if (mode_ != Mode::MUS_DONT_CREATE_WINDOW_TREE_CLIENT && root_window()) {
-    client::SetFocusClient(root_window(), nullptr);
-    client::SetCaptureClient(root_window(), nullptr);
-    host_.reset();
-
-    if (display::Screen::GetScreen() == test_screen_.get())
-      display::Screen::SetScreenInstance(nullptr);
-    test_screen_.reset();
-
-    window_tree_client_setup_.reset();
-    focus_client_.reset();
-    capture_client_.reset();
-  } else {
-    if (display::Screen::GetScreen() == test_screen_.get())
-      display::Screen::SetScreenInstance(nullptr);
-    test_screen_.reset();
-    window_tree_client_setup_.reset();
-  }
+  client::SetFocusClient(root_window(), nullptr);
+  client::SetCaptureClient(root_window(), nullptr);
+  host_.reset();
+
+  if (display::Screen::GetScreen() == test_screen_.get())
+    display::Screen::SetScreenInstance(nullptr);
+  test_screen_.reset();
+
+  window_tree_client_setup_.reset();
+  focus_client_.reset();
+  capture_client_.reset();
+
   ui::GestureRecognizer::Reset();
   ui::ShutdownInputMethodForTesting();
 

ui/aura/test/aura_test_helper.h

@@ -67,10 +67,6 @@ class AuraTestHelper {
   // testing shutdown ordering.
   void DeleteWindowTreeClient();
 
-  void set_create_host_for_primary_display(bool value) {
-    create_host_for_primary_display_ = value;
-  }
-
   // Creates and initializes (shows and sizes) the RootWindow for use in tests.
   void SetUp(ui::ContextFactory* context_factory,
              ui::ContextFactoryPrivate* context_factory_private);
@@ -140,10 +136,6 @@ class AuraTestHelper {
 
   WindowTreeClient* window_tree_client_ = nullptr;
 
-  // Whether SetUp() should create a WindowTreeHost for the primary display. The
-  // value of this is *not* used if Mode is MUS.
-  bool create_host_for_primary_display_ = true;
-
   DISALLOW_COPY_AND_ASSIGN(AuraTestHelper);
 };
 

ui/aura/test/mus/window_tree_client_private.cc

@@ -104,7 +104,13 @@ bool WindowTreeClientPrivate::HasChangeInFlightOfType(ChangeType type) {
 ui::mojom::WindowDataPtr WindowTreeClientPrivate::CreateWindowDataForEmbed() {
   ui::mojom::WindowDataPtr root_data(ui::mojom::WindowData::New());
   root_data->parent_id = 0;
-  root_data->window_id = next_window_id_++;
+  // OnEmbed() is passed windows the client doesn't own. Use a |client_id| of 1
+  // to mirror what the server does for the client-id portion, and use the
+  // number of roots for the window id. The important part is the combination is
+  // unique to the client.
+  const ui::Id client_id = 1;
+  root_data->window_id =
+      (client_id << 32) | tree_client_impl_->GetRoots().size();
   root_data->visible = true;
   return root_data;
 }

ui/aura/test/mus/window_tree_client_private.h

@@ -73,7 +73,6 @@ class WindowTreeClientPrivate {
   ui::mojom::WindowDataPtr CreateWindowDataForEmbed();
 
   WindowTreeClient* tree_client_impl_;
-  uint16_t next_window_id_ = 1u;
 
   DISALLOW_COPY_AND_ASSIGN(WindowTreeClientPrivate);
 };

ui/views/mus/ax_remote_host.cc

@@ -152,17 +152,17 @@ void AXRemoteHost::Enable() {
 
   std::set<aura::Window*> roots =
       MusClient::Get()->window_tree_client()->GetRoots();
-  if (roots.empty()) {
-    // Client hasn't opened any widgets yet.
-    return;
+  for (aura::Window* root : roots) {
+    Widget* widget = Widget::GetWidgetForNativeWindow(root);
+    // Typically it's only tests that create Windows (the WindowTreeHost
+    // backing the root Window) in such a way that there is no associated
+    // widget.
+    if (widget) {
+      // TODO(jamescook): Support multiple roots.
+      DCHECK(!widget_);
+      StartMonitoringWidget(widget);
+    }
   }
-
-  // TODO(jamescook): Support multiple roots.
-  aura::Window* root_window = *roots.begin();
-  DCHECK(root_window);
-  Widget* root_widget = Widget::GetWidgetForNativeWindow(root_window);
-  DCHECK(root_widget);
-  StartMonitoringWidget(root_widget);
 }
 
 void AXRemoteHost::Disable() {