Adds factory functions for creation of WindowTreeClient

These make construction slightly clearer in that only arguments that
are needed can be supplied.

BUG=none
TEST=covered by tests

Change-Id: Ie4029896ff24957b1f7264ef6fb64fba018625ab
Reviewed-on: https://chromium-review.googlesource.com/946882
Commit-Queue: Scott Violet <sky@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540762}

ash/window_manager_service.cc

@@ -139,12 +139,11 @@ void WindowManagerService::OnStart() {
   window_manager_ = std::make_unique<WindowManager>(
       context()->connector(), ash_config_, show_primary_host_on_connect_);
 
-  std::unique_ptr<aura::WindowTreeClient> window_tree_client =
-      std::make_unique<aura::WindowTreeClient>(
-          context()->connector(), window_manager_.get(), window_manager_.get());
   const bool automatically_create_display_roots = false;
-  window_tree_client->ConnectAsWindowManager(
-      automatically_create_display_roots);
+  std::unique_ptr<aura::WindowTreeClient> window_tree_client =
+      aura::WindowTreeClient::CreateForWindowManager(
+          context()->connector(), window_manager_.get(), window_manager_.get(),
+          automatically_create_display_roots);
 
   const bool init_network_handler = true;
   InitWindowManager(std::move(window_tree_client), init_network_handler);

ash/window_manager_unittest.cc

@@ -105,16 +105,17 @@ TEST_F(WindowManagerServiceTest, OpenWindow) {
 
   // Connect to mus and create a new top level window. The request goes to
   // |ash|, but is async.
-  aura::WindowTreeClient client(connector(), &window_tree_delegate, nullptr,
-                                nullptr, nullptr, false);
-  client.ConnectViaWindowTreeFactory();
-  aura::test::EnvWindowTreeClientSetter env_window_tree_client_setter(&client);
+  std::unique_ptr<aura::WindowTreeClient> client =
+      aura::WindowTreeClient::CreateForWindowTreeFactory(
+          connector(), &window_tree_delegate, false);
+  aura::test::EnvWindowTreeClientSetter env_window_tree_client_setter(
+      client.get());
   std::map<std::string, std::vector<uint8_t>> properties;
   properties[ui::mojom::WindowManager::kWindowType_InitProperty] =
       mojo::ConvertTo<std::vector<uint8_t>>(
           static_cast<int32_t>(ui::mojom::WindowType::WINDOW));
   aura::WindowTreeHostMus window_tree_host_mus(
-      aura::CreateInitParamsForTopLevel(&client, std::move(properties)));
+      aura::CreateInitParamsForTopLevel(client.get(), std::move(properties)));
   window_tree_host_mus.InitHost();
   aura::Window* child_window = new aura::Window(nullptr);
   child_window->Init(ui::LAYER_NOT_DRAWN);
@@ -124,12 +125,13 @@ TEST_F(WindowManagerServiceTest, OpenWindow) {
   // |child_window|. This blocks until it succeeds.
   ui::mojom::WindowTreeClientPtr tree_client;
   auto tree_client_request = MakeRequest(&tree_client);
-  client.Embed(child_window, std::move(tree_client), 0u, base::Bind(&OnEmbed));
-  aura::WindowTreeClient child_client(connector(), &window_tree_delegate,
-                                      nullptr, std::move(tree_client_request),
-                                      nullptr, false);
+  client->Embed(child_window, std::move(tree_client), 0u, base::Bind(&OnEmbed));
+  std::unique_ptr<aura::WindowTreeClient> child_client =
+      aura::WindowTreeClient::CreateForEmbedding(
+          connector(), &window_tree_delegate, std::move(tree_client_request),
+          false);
   window_tree_delegate.WaitForEmbed();
-  ASSERT_TRUE(!child_client.GetRoots().empty());
+  ASSERT_TRUE(!child_client->GetRoots().empty());
   window_tree_delegate.DestroyWindowTreeHost();
 }
 

chrome/browser/ui/ash/ash_shell_init.cc

@@ -63,13 +63,11 @@ std::unique_ptr<ash::WindowManager> CreateMusShell() {
   // When Ash runs in the same services as chrome content creates the
   // DiscardableSharedMemoryManager.
   const bool create_discardable_memory = false;
-  std::unique_ptr<aura::WindowTreeClient> window_tree_client =
-      std::make_unique<aura::WindowTreeClient>(
-          connector, window_manager.get(), window_manager.get(), nullptr,
-          nullptr, create_discardable_memory);
   const bool automatically_create_display_roots = false;
-  window_tree_client->ConnectAsWindowManager(
-      automatically_create_display_roots);
+  std::unique_ptr<aura::WindowTreeClient> window_tree_client =
+      aura::WindowTreeClient::CreateForWindowManager(
+          connector, window_manager.get(), window_manager.get(),
+          automatically_create_display_roots, create_discardable_memory);
   aura::Env::GetInstance()->SetWindowTreeClient(window_tree_client.get());
   window_manager->Init(std::move(window_tree_client),
                        std::make_unique<ChromeShellDelegate>());

mash/simple_wm/simple_wm.cc

@@ -373,10 +373,9 @@ void SimpleWM::OnStart() {
     context()->QuitNow();
     return;
   }
-  window_tree_client_ = std::make_unique<aura::WindowTreeClient>(
+  window_tree_client_ = aura::WindowTreeClient::CreateForWindowManager(
       context()->connector(), this, this);
   aura::Env::GetInstance()->SetWindowTreeClient(window_tree_client_.get());
-  window_tree_client_->ConnectAsWindowManager();
 }
 
 ////////////////////////////////////////////////////////////////////////////////

services/ui/demo/mus_demo_external.cc

@@ -31,7 +31,7 @@ class WindowTreeDataExternal : public WindowTreeData {
 
 int GetSquareSizeForWindow(int window_index) {
   return 50 * window_index + 400;
-};
+}
 
 }  // namespace
 
@@ -41,7 +41,8 @@ MusDemoExternal::~MusDemoExternal() {}
 
 std::unique_ptr<aura::WindowTreeClient>
 MusDemoExternal::CreateWindowTreeClient() {
-  return std::make_unique<aura::WindowTreeClient>(context()->connector(), this);
+  return aura::WindowTreeClient::CreateForWindowTreeHostFactory(
+      context()->connector(), this);
 }
 
 void MusDemoExternal::OnStartImpl() {
@@ -60,18 +61,6 @@ void MusDemoExternal::OnStartImpl() {
   // For now, a fake display is created in order to work around an assertion in
   // aura::GetDeviceScaleFactorFromDisplay().
   AddPrimaryDisplay(display::Display(0));
-
-  // The number of windows to open is specified by number_of_windows_. The
-  // windows are opened sequentially (the first one here and the others after
-  // each call to OnEmbed) to ensure that the WindowTreeHostMus passed to
-  // OnEmbed corresponds to the WindowTreeDataExternal::host_ created in
-  // OpenNewWindow.
-  OpenNewWindow();
-
-  // TODO(tonikitoo,msisov): For true external window mode, we want a slightly
-  // different API: it would connect to MUS, creating a unique WindowTree
-  // instance, capable of serving 0..n WindowTreeHost's.
-  window_tree_client()->ConnectViaWindowTreeHostFactory();
 }
 
 void MusDemoExternal::OpenNewWindow() {
@@ -85,6 +74,11 @@ void MusDemoExternal::OpenNewWindow() {
 
 void MusDemoExternal::OnEmbed(
     std::unique_ptr<aura::WindowTreeHostMus> window_tree_host) {
+  if (initialized_windows_count_ == 0) {
+    // The initial connection to the WindowService has been established.
+    OpenNewWindow();
+  }
+
   InitWindowTreeData(std::move(window_tree_host));
   initialized_windows_count_++;
 

services/ui/demo/mus_demo_internal.cc

@@ -24,12 +24,11 @@ MusDemoInternal::~MusDemoInternal() {}
 
 std::unique_ptr<aura::WindowTreeClient>
 MusDemoInternal::CreateWindowTreeClient() {
-  return std::make_unique<aura::WindowTreeClient>(context()->connector(), this,
-                                                  this);
+  return aura::WindowTreeClient::CreateForWindowManager(context()->connector(),
+                                                        this, this);
 }
 
 void MusDemoInternal::OnStartImpl() {
-  window_tree_client()->ConnectAsWindowManager();
   // The demo will actually start when the window server creates the display,
   // causing OnWmNewDisplay to be called.
 }

services/ui/test_wm/test_wm.cc

@@ -59,10 +59,9 @@ class TestWM : public service_manager::Service,
     screen_ = std::make_unique<display::ScreenBase>();
     display::Screen::SetScreenInstance(screen_.get());
     aura_env_ = aura::Env::CreateInstance(aura::Env::Mode::MUS);
-    window_tree_client_ = std::make_unique<aura::WindowTreeClient>(
+    window_tree_client_ = aura::WindowTreeClient::CreateForWindowManager(
         context()->connector(), this, this);
     aura_env_->SetWindowTreeClient(window_tree_client_.get());
-    window_tree_client_->ConnectAsWindowManager();
   }
   void OnBindInterface(const service_manager::BindSourceInfo& source_info,
                        const std::string& interface_name,

services/ui/ws/window_manager_client_unittest.cc

@@ -686,14 +686,14 @@ class EstablishConnectionViaFactoryDelegate
 TEST_F(WindowServerTest, EstablishConnectionViaFactory) {
   EstablishConnectionViaFactoryDelegate delegate(window_manager());
   set_window_manager_delegate(&delegate);
-  aura::WindowTreeClient second_client(connector(), this, nullptr, nullptr,
-                                       nullptr, false);
-  second_client.ConnectViaWindowTreeFactory();
+  std::unique_ptr<aura::WindowTreeClient> second_client =
+      aura::WindowTreeClient::CreateForWindowTreeFactory(connector(), this,
+                                                         false);
   aura::WindowTreeHostMus window_tree_host_in_second_client(
-      aura::CreateInitParamsForTopLevel(&second_client));
+      aura::CreateInitParamsForTopLevel(second_client.get()));
   window_tree_host_in_second_client.InitHost();
   window_tree_host_in_second_client.window()->Show();
-  ASSERT_TRUE(second_client.GetRoots().count(
+  ASSERT_TRUE(second_client->GetRoots().count(
                   window_tree_host_in_second_client.window()) > 0);
   // Wait for the window to appear in the wm.
   ASSERT_TRUE(delegate.QuitOnCreate());
@@ -716,17 +716,17 @@ TEST_F(WindowServerTest, OnWindowHierarchyChangedIncludesTransientParent) {
   // of the first window and then add it.
   EstablishConnectionViaFactoryDelegate delegate(window_manager());
   set_window_manager_delegate(&delegate);
-  aura::WindowTreeClient second_client(connector(), this, nullptr, nullptr,
-                                       nullptr, false);
-  second_client.ConnectViaWindowTreeFactory();
+  std::unique_ptr<aura::WindowTreeClient> second_client =
+      aura::WindowTreeClient::CreateForWindowTreeFactory(connector(), this,
+                                                         false);
   aura::WindowTreeHostMus window_tree_host_in_second_client(
-      aura::CreateInitParamsForTopLevel(&second_client));
+      aura::CreateInitParamsForTopLevel(second_client.get()));
   window_tree_host_in_second_client.InitHost();
   window_tree_host_in_second_client.window()->Show();
   aura::Window* second_client_child = NewVisibleWindow(
-      window_tree_host_in_second_client.window(), &second_client);
+      window_tree_host_in_second_client.window(), second_client.get());
   // Create the transient without a parent, set transient parent, then add.
-  aura::Window* transient = NewVisibleWindow(nullptr, &second_client);
+  aura::Window* transient = NewVisibleWindow(nullptr, second_client.get());
   aura::client::TransientWindowClient* transient_window_client =
       aura::client::GetTransientWindowClient();
   transient_window_client->AddTransientChild(second_client_child, transient);

services/ui/ws/window_server_test_base.cc

@@ -100,8 +100,7 @@ void WindowServerTestBase::SetUp() {
   env_ = aura::Env::CreateInstance(aura::Env::Mode::MUS);
   display::Screen::SetScreenInstance(&screen_);
   std::unique_ptr<aura::WindowTreeClient> window_manager_window_tree_client =
-      std::make_unique<aura::WindowTreeClient>(connector(), this, this);
-  window_manager_window_tree_client->ConnectAsWindowManager();
+      aura::WindowTreeClient::CreateForWindowManager(connector(), this, this);
   window_manager_ = window_manager_window_tree_client.get();
   window_tree_clients_.push_back(std::move(window_manager_window_tree_client));
 
@@ -290,11 +289,10 @@ void WindowServerTestBase::OnWmDeactivateWindow(aura::Window* window) {
 }
 
 void WindowServerTestBase::BindWindowTreeClientRequest(
-    mojom::WindowTreeClientRequest request) {
+    ui::mojom::WindowTreeClientRequest request) {
   const bool create_discardable_memory = false;
-  window_tree_clients_.push_back(std::make_unique<aura::WindowTreeClient>(
-      connector(), this, nullptr, std::move(request), nullptr,
-      create_discardable_memory));
+  window_tree_clients_.push_back(aura::WindowTreeClient::CreateForEmbedding(
+      connector(), this, std::move(request), create_discardable_memory));
 }
 
 bool WindowServerTestBase::DeleteWindowTreeHost(

ui/aura/mus/window_tree_client.cc

@@ -176,6 +176,80 @@ ui::Id GetServerIdForWindow(Window* window) {
 
 }  // namespace
 
+// static
+std::unique_ptr<WindowTreeClient> WindowTreeClient::CreateForWindowManager(
+    service_manager::Connector* connector,
+    WindowTreeClientDelegate* delegate,
+    WindowManagerDelegate* window_manager_delegate,
+    bool automatically_create_display_roots,
+    bool create_discardable_memory) {
+  std::unique_ptr<WindowTreeClient> wtc(
+      new WindowTreeClient(connector, delegate, window_manager_delegate,
+                           nullptr, nullptr, create_discardable_memory));
+
+  ui::mojom::WindowManagerWindowTreeFactoryPtr factory;
+  connector->BindInterface(ui::mojom::kServiceName, &factory);
+  ui::mojom::WindowTreePtr window_tree;
+  ui::mojom::WindowTreeClientPtr client;
+  wtc->binding_.Bind(MakeRequest(&client));
+  factory->CreateWindowTree(MakeRequest(&window_tree), std::move(client),
+                            automatically_create_display_roots);
+  wtc->SetWindowTree(std::move(window_tree));
+  return wtc;
+}
+
+// static
+std::unique_ptr<WindowTreeClient> WindowTreeClient::CreateForEmbedding(
+    service_manager::Connector* connector,
+    WindowTreeClientDelegate* delegate,
+    ui::mojom::WindowTreeClientRequest request,
+    bool create_discardable_memory) {
+  std::unique_ptr<WindowTreeClient> wtc(
+      new WindowTreeClient(connector, delegate, nullptr, std::move(request),
+                           nullptr, create_discardable_memory));
+  return wtc;
+}
+
+// static
+std::unique_ptr<WindowTreeClient> WindowTreeClient::CreateForWindowTreeFactory(
+    service_manager::Connector* connector,
+    WindowTreeClientDelegate* delegate,
+    bool create_discardable_memory,
+    scoped_refptr<base::SingleThreadTaskRunner> io_task_runner) {
+  std::unique_ptr<WindowTreeClient> wtc(
+      new WindowTreeClient(connector, delegate, nullptr, nullptr, nullptr,
+                           create_discardable_memory));
+  ui::mojom::WindowTreeFactoryPtr factory;
+  connector->BindInterface(ui::mojom::kServiceName, &factory);
+  ui::mojom::WindowTreePtr window_tree;
+  ui::mojom::WindowTreeClientPtr client;
+  wtc->binding_.Bind(MakeRequest(&client));
+  factory->CreateWindowTree(MakeRequest(&window_tree), std::move(client));
+  wtc->SetWindowTree(std::move(window_tree));
+  return wtc;
+}
+
+// static
+std::unique_ptr<WindowTreeClient>
+WindowTreeClient::CreateForWindowTreeHostFactory(
+    service_manager::Connector* connector,
+    WindowTreeClientDelegate* delegate,
+    bool create_discardable_memory) {
+  std::unique_ptr<WindowTreeClient> wtc(
+      new WindowTreeClient(connector, delegate, nullptr, nullptr, nullptr,
+                           create_discardable_memory));
+  ui::mojom::WindowTreeHostFactoryPtr factory;
+  connector->BindInterface(ui::mojom::kServiceName, &factory);
+
+  ui::mojom::WindowTreeHostPtr window_tree_host;
+  ui::mojom::WindowTreeClientPtr client;
+  wtc->binding_.Bind(MakeRequest(&client));
+  factory->CreateWindowTreeHost(MakeRequest(&window_tree_host),
+                                std::move(client));
+  return wtc;
+}
+
+// TODO(sky): move to match location in header.
 WindowTreeClient::WindowTreeClient(
     service_manager::Connector* connector,
     WindowTreeClientDelegate* delegate,
@@ -282,42 +356,6 @@ WindowTreeClient::~WindowTreeClient() {
   CHECK(windows_.empty());
 }
 
-void WindowTreeClient::ConnectViaWindowTreeFactory() {
-  ui::mojom::WindowTreeFactoryPtr factory;
-  connector_->BindInterface(ui::mojom::kServiceName, &factory);
-  ui::mojom::WindowTreePtr window_tree;
-  ui::mojom::WindowTreeClientPtr client;
-  binding_.Bind(MakeRequest(&client));
-  factory->CreateWindowTree(MakeRequest(&window_tree), std::move(client));
-  SetWindowTree(std::move(window_tree));
-}
-
-void WindowTreeClient::ConnectAsWindowManager(
-    bool automatically_create_display_roots) {
-  DCHECK(window_manager_delegate_);
-
-  ui::mojom::WindowManagerWindowTreeFactoryPtr factory;
-  connector_->BindInterface(ui::mojom::kServiceName, &factory);
-  ui::mojom::WindowTreePtr window_tree;
-  ui::mojom::WindowTreeClientPtr client;
-  binding_.Bind(MakeRequest(&client));
-  factory->CreateWindowTree(MakeRequest(&window_tree), std::move(client),
-                            automatically_create_display_roots);
-  SetWindowTree(std::move(window_tree));
-}
-
-void WindowTreeClient::ConnectViaWindowTreeHostFactory() {
-  ui::mojom::WindowTreeHostFactoryPtr factory;
-  connector_->BindInterface(ui::mojom::kServiceName, &factory);
-
-  ui::mojom::WindowTreeHostPtr window_tree_host;
-  ui::mojom::WindowTreeClientPtr client;
-  binding_.Bind(MakeRequest(&client));
-  factory->CreateWindowTreeHost(MakeRequest(&window_tree_host),
-                                std::move(client));
-  WaitForInitialDisplays();
-}
-
 void WindowTreeClient::SetCanFocus(Window* window, bool can_focus) {
   DCHECK(tree_);
   DCHECK(window);