Changes ownership of WindowTreeHostConnection

Changes ownership of WindowTreeHostConnection Previously WindowTreeHostConnection owned then WindowTreeHost. This change makes the WindowTreeHosts owned by the ConnectionManager. WindowTreeHost in turn owns the WindowTreeHostConnection. I'm doing this as multiple windows managers per display implies WindowTreeHost won't always have a WindowTreeHostConnection. I also nuked WindowTreeHostDelegate and folded into WindowTreeHostConnection. BUG=590955 TEST=covered by tests [email protected] Review URL: https://codereview.chromium.org/1750143002 Cr-Commit-Position: refs/heads/master@{#378393}
huningxin · Mar 1, 2016 · 04ce21a · 04ce21a
1 parent a7e8a5f
commit 04ce21a
Show file tree

Hide file tree

Showing 10 changed files with 156 additions and 245 deletions.
diff --git a/components/mus/mus_app.cc b/components/mus/mus_app.cc
@@ -232,10 +232,11 @@ void MandolineUIServicesApp::CreateWindowTreeHost(
   ws::WindowTreeHostImpl* host_impl = new ws::WindowTreeHostImpl(
       connection_manager_.get(), connector_, gpu_state_, surfaces_state_);
 
-  // WindowTreeHostConnection manages its own lifetime.
-  host_impl->Init(new ws::WindowTreeHostConnectionImpl(
-      std::move(host), make_scoped_ptr(host_impl), std::move(tree_client),
-      connection_manager_.get()));
+  scoped_ptr<ws::WindowTreeHostConnectionImpl> host_connection(
+      new ws::WindowTreeHostConnectionImpl(std::move(host), host_impl,
+                                           std::move(tree_client),
+                                           connection_manager_.get()));
+  host_impl->Init(std::move(host_connection));
 }
 
 }  // namespace mus
diff --git a/components/mus/ws/BUILD.gn b/components/mus/ws/BUILD.gn
@@ -56,7 +56,6 @@ source_set("lib") {
     "window_tree_factory.h",
     "window_tree_host_connection.cc",
     "window_tree_host_connection.h",
-    "window_tree_host_delegate.h",
     "window_tree_host_impl.cc",
     "window_tree_host_impl.h",
     "window_tree_impl.cc",

diff --git a/components/mus/ws/connection_manager.cc b/components/mus/ws/connection_manager.cc
@@ -38,25 +38,25 @@ ConnectionManager::ConnectionManager(
 ConnectionManager::~ConnectionManager() {
   in_destructor_ = true;
 
-  // Copy the HostConnectionMap because it will be mutated as the connections
-  // are closed.
-  HostConnectionMap host_connection_map(host_connection_map_);
-  for (auto& pair : host_connection_map)
-    pair.second->CloseConnection();
+  // DestroyHost() removes from |hosts_| and deletes the WindowTreeHostImpl.
+  while (!hosts_.empty())
+    DestroyHost(*hosts_.begin());
+  DCHECK(hosts_.empty());
 
   STLDeleteValues(&connection_map_);
   // All the connections should have been destroyed.
-  DCHECK(host_connection_map_.empty());
   DCHECK(connection_map_.empty());
 }
 
-void ConnectionManager::AddHost(WindowTreeHostConnection* host_connection) {
-  DCHECK_EQ(0u,
-            host_connection_map_.count(host_connection->window_tree_host()));
-  const bool is_first_connection = host_connection_map_.empty();
-  host_connection_map_[host_connection->window_tree_host()] = host_connection;
-  if (is_first_connection)
-    delegate_->OnFirstRootConnectionCreated();
+void ConnectionManager::AddHost(WindowTreeHostImpl* host) {
+  DCHECK_EQ(0u, pending_hosts_.count(host));
+  pending_hosts_.insert(host);
+}
+
+void ConnectionManager::DestroyHost(WindowTreeHostImpl* host) {
+  DCHECK(hosts_.count(host));
+  hosts_.erase(host);
+  delete host;
 }
 
 ServerWindow* ConnectionManager::CreateServerWindow(
@@ -121,31 +121,35 @@ ClientConnection* ConnectionManager::GetClientConnection(
   return connection_map_[window_tree->id()];
 }
 
-void ConnectionManager::OnHostConnectionClosed(
-    WindowTreeHostConnection* connection) {
-  auto it = host_connection_map_.find(connection->window_tree_host());
-  DCHECK(it != host_connection_map_.end());
+void ConnectionManager::OnHostConnectionClosed(WindowTreeHostImpl* host) {
+  if (pending_hosts_.count(host)) {
+    pending_hosts_.erase(host);
+    delete host;
+    return;
+  }
+  auto it = hosts_.find(host);
+  DCHECK(it != hosts_.end());
 
   // Get the ClientConnection by WindowTreeImpl ID.
   ConnectionMap::iterator service_connection_it =
-      connection_map_.find(it->first->GetWindowTree()->id());
+      connection_map_.find(host->GetWindowTree()->id());
   DCHECK(service_connection_it != connection_map_.end());
 
+  scoped_ptr<WindowTreeHostImpl> host_owner(*it);
+  hosts_.erase(it);
+
   // Tear down the associated WindowTree connection.
   // TODO(fsamuel): I don't think this is quite right, we should tear down all
   // connections within the root's viewport. We should probably employ an
   // observer pattern to do this. Each WindowTreeImpl should track its
   // parent's lifetime.
-  host_connection_map_.erase(it);
   OnConnectionError(service_connection_it->second);
 
-  for (auto& pair : connection_map_) {
-    pair.second->service()->OnWillDestroyWindowTreeHost(
-        connection->window_tree_host());
-  }
+  for (auto& pair : connection_map_)
+    pair.second->service()->OnWillDestroyWindowTreeHost(host);
 
   // If we have no more roots left, let the app know so it can terminate.
-  if (!host_connection_map_.size())
+  if (!hosts_.size() && !pending_hosts_.size())
     delegate_->OnNoMoreRootConnections();
 }
 
@@ -178,31 +182,42 @@ WindowTreeImpl* ConnectionManager::GetConnection(
 }
 
 ServerWindow* ConnectionManager::GetWindow(const WindowId& id) {
-  for (auto& pair : host_connection_map_) {
-    if (pair.first->root_window()->id() == id)
-      return pair.first->root_window();
+  for (WindowTreeHostImpl* host : hosts_) {
+    if (host->root_window()->id() == id)
+      return host->root_window();
   }
   WindowTreeImpl* service = GetConnection(id.connection_id);
   return service ? service->GetWindow(id) : nullptr;
 }
 
 bool ConnectionManager::IsWindowAttachedToRoot(
     const ServerWindow* window) const {
-  for (auto& pair : host_connection_map_) {
-    if (pair.first->IsWindowAttachedToRoot(window))
+  for (WindowTreeHostImpl* host : hosts_) {
+    if (host->IsWindowAttachedToRoot(window))
       return true;
   }
   return false;
 }
 
 void ConnectionManager::SchedulePaint(const ServerWindow* window,
                                       const gfx::Rect& bounds) {
-  for (auto& pair : host_connection_map_) {
-    if (pair.first->SchedulePaintIfInViewport(window, bounds))
+  for (WindowTreeHostImpl* host : hosts_) {
+    if (host->SchedulePaintIfInViewport(window, bounds))
       return;
   }
 }
 
+void ConnectionManager::OnWindowTreeHostDisplayAvailable(
+    WindowTreeHostImpl* host) {
+  DCHECK_NE(0u, pending_hosts_.count(host));
+  DCHECK_EQ(0u, hosts_.count(host));
+  const bool is_first_connection = hosts_.empty();
+  hosts_.insert(host);
+  pending_hosts_.erase(host);
+  if (is_first_connection)
+    delegate_->OnFirstRootConnectionCreated();
+}
+
 void ConnectionManager::OnConnectionMessagedClient(ConnectionSpecificId id) {
   if (current_operation_)
     current_operation_->MarkConnectionAsMessaged(id);
@@ -219,8 +234,8 @@ mojom::ViewportMetricsPtr ConnectionManager::GetViewportMetricsForWindow(
   if (host)
     return host->GetViewportMetrics().Clone();
 
-  if (!host_connection_map_.empty())
-    return host_connection_map_.begin()->first->GetViewportMetrics().Clone();
+  if (!hosts_.empty())
+    return (*hosts_.begin())->GetViewportMetrics().Clone();
 
   mojom::ViewportMetricsPtr metrics = mojom::ViewportMetrics::New();
   metrics->size_in_pixels = mojo::Size::New();
@@ -249,15 +264,16 @@ const WindowTreeHostImpl* ConnectionManager::GetWindowTreeHostByWindow(
     const ServerWindow* window) const {
   while (window && window->parent())
     window = window->parent();
-  for (auto& pair : host_connection_map_) {
-    if (window == pair.first->root_window())
-      return pair.first;
+  for (WindowTreeHostImpl* host : hosts_) {
+    if (window == host->root_window())
+      return host;
   }
   return nullptr;
 }
 
 WindowTreeHostImpl* ConnectionManager::GetActiveWindowTreeHost() {
-  return host_connection_map_.begin()->first;
+  // TODO(sky): this isn't active, but first. Make it active.
+  return hosts_.size() ? *hosts_.begin() : nullptr;
 }
 
 void ConnectionManager::AddDisplayManagerBinding(
@@ -472,11 +488,12 @@ void ConnectionManager::MaybeUpdateNativeCursor(ServerWindow* window) {
 
 void ConnectionManager::CallOnDisplays(
     mojom::DisplayManagerObserver* observer) {
-  mojo::Array<mojom::DisplayPtr> displays(host_connection_map_.size());
+  mojo::Array<mojom::DisplayPtr> displays(hosts_.size());
   {
     size_t i = 0;
-    for (auto& pair : host_connection_map_) {
-      displays[i] = DisplayForHost(pair.first);
+    // TODO(sky): need ordering!
+    for (WindowTreeHostImpl* host : hosts_) {
+      displays[i] = DisplayForHost(host);
       ++i;
     }
   }
@@ -497,9 +514,9 @@ void ConnectionManager::CallOnDisplayChanged(
 mojom::DisplayPtr ConnectionManager::DisplayForHost(WindowTreeHostImpl* host) {
   size_t i = 0;
   int next_x = 0;
-  for (auto& pair : host_connection_map_) {
+  for (WindowTreeHostImpl* host2 : hosts_) {
     const ServerWindow* root = host->root_window();
-    if (pair.first == host) {
+    if (host == host2) {
       mojom::DisplayPtr display = mojom::Display::New();
       display = mojom::Display::New();
       display->id = host->id();
@@ -544,9 +561,9 @@ const ServerWindow* ConnectionManager::GetRootWindow(
 }
 
 void ConnectionManager::ScheduleSurfaceDestruction(ServerWindow* window) {
-  for (auto& pair : host_connection_map_) {
-    if (pair.first->root_window()->Contains(window)) {
-      pair.first->ScheduleSurfaceDestruction(window);
+  for (WindowTreeHostImpl* host : hosts_) {
+    if (host->root_window()->Contains(window)) {
+      host->ScheduleSurfaceDestruction(window);
       break;
     }
   }

diff --git a/components/mus/ws/connection_manager.h b/components/mus/ws/connection_manager.h
@@ -53,8 +53,10 @@ class ConnectionManager : public ServerWindowDelegate,
                     const scoped_refptr<mus::SurfacesState>& surfaces_state);
   ~ConnectionManager() override;
 
-  // Adds a WindowTreeHost.
-  void AddHost(WindowTreeHostConnection* connection);
+  // Adds/removes a WindowTreeHost. ConnectionManager owns the
+  // WindowTreeHostImpls.
+  void AddHost(WindowTreeHostImpl* host);
+  void DestroyHost(WindowTreeHostImpl* host);
 
   // Creates a new ServerWindow. The return value is owned by the caller, but
   // must be destroyed before ConnectionManager.
@@ -73,9 +75,7 @@ class ConnectionManager : public ServerWindowDelegate,
 
   ClientConnection* GetClientConnection(WindowTreeImpl* window_tree);
 
-  // Invoked when a WindowTreeHostConnection encounters an error or the
-  // associated Display window is closed.
-  void OnHostConnectionClosed(WindowTreeHostConnection* connection);
+  void OnHostConnectionClosed(WindowTreeHostImpl* host);
 
   // See description of WindowTree::Embed() for details. This assumes
   // |transport_window_id| is valid.
@@ -108,6 +108,9 @@ class ConnectionManager : public ServerWindowDelegate,
   // Invoked when the WindowTreeHostImpl's display is closed.
   void OnDisplayClosed();
 
+  // Called when the AcceleratedWidget is available for |host|.
+  void OnWindowTreeHostDisplayAvailable(WindowTreeHostImpl* host);
+
   // Invoked when a connection messages a client about the change. This is used
   // to avoid sending ServerChangeIdAdvanced() unnecessarily.
   void OnConnectionMessagedClient(ConnectionSpecificId id);
@@ -134,9 +137,7 @@ class ConnectionManager : public ServerWindowDelegate,
 
   WindowTreeHostImpl* GetActiveWindowTreeHost();
 
-  bool has_tree_host_connections() const {
-    return !host_connection_map_.empty();
-  }
+  bool has_tree_host_connections() const { return !hosts_.empty(); }
 
   void AddDisplayManagerBinding(
       mojo::InterfaceRequest<mojom::DisplayManager> request);
@@ -303,8 +304,11 @@ class ConnectionManager : public ServerWindowDelegate,
   // Set of WindowTreeImpls.
   ConnectionMap connection_map_;
 
-  // Set of WindowTreeHostImpls.
-  HostConnectionMap host_connection_map_;
+  // WindowTreeHostImpls are initially added to |pending_hosts_|. When the
+  // display is initialized it is moved to |hosts_|.
+  // ConnectionManager owns the WindowTreeHostImpls.
+  std::set<WindowTreeHostImpl*> pending_hosts_;
+  std::set<WindowTreeHostImpl*> hosts_;
 
   // If non-null then we're processing a client operation. The Operation is
   // not owned by us (it's created on the stack by WindowTreeImpl).

diff --git a/components/mus/ws/window_tree_host_connection.cc b/components/mus/ws/window_tree_host_connection.cc
@@ -9,66 +9,26 @@
 #include "components/mus/ws/window_tree_impl.h"
 
 namespace mus {
-
 namespace ws {
 
-WindowTreeHostConnection::WindowTreeHostConnection(
-    scoped_ptr<WindowTreeHostImpl> host_impl,
-    ConnectionManager* manager)
-    : host_(std::move(host_impl)),
-      tree_(nullptr),
-      connection_manager_(manager),
-      connection_closed_(false) {}
-
-WindowTreeHostConnection::~WindowTreeHostConnection() {
-  // If this DCHECK fails then something has tried to delete this object without
-  // calling CloseConnection.
-  DCHECK(connection_closed_);
-}
-
-void WindowTreeHostConnection::CloseConnection() {
-  // A connection error will trigger the display to close and so we want to make
-  // sure we signal the ConnectionManager only once.
-  if (connection_closed_)
-    return;
-  // We have to shut down the focus system for this host before destroying the
-  // host, as destruction of the window tree will attempt to change focus.
-  host_->DestroyFocusController();
-  connection_manager()->OnHostConnectionClosed(this);
-  connection_closed_ = true;
-  delete this;
-}
-
-const WindowTreeImpl* WindowTreeHostConnection::GetWindowTree() const {
-  return tree_;
-}
-
-void WindowTreeHostConnection::OnDisplayInitialized() {}
-
-void WindowTreeHostConnection::OnDisplayClosed() {
-  CloseConnection();
-}
-
 WindowTreeHostConnectionImpl::WindowTreeHostConnectionImpl(
     mojo::InterfaceRequest<mojom::WindowTreeHost> request,
-    scoped_ptr<WindowTreeHostImpl> host_impl,
+    WindowTreeHostImpl* host_impl,
     mojom::WindowTreeClientPtr client,
     ConnectionManager* manager)
-    : WindowTreeHostConnection(std::move(host_impl), manager),
-      binding_(window_tree_host(), std::move(request)),
+    : connection_manager_(manager),
+      binding_(host_impl, std::move(request)),
       client_(std::move(client)) {}
 
 WindowTreeHostConnectionImpl::~WindowTreeHostConnectionImpl() {}
 
-void WindowTreeHostConnectionImpl::OnDisplayInitialized() {
-  connection_manager()->AddHost(this);
-  WindowTreeImpl* tree = connection_manager()->EmbedAtWindow(
-      window_tree_host()->root_window(),
-      mojom::WindowTree::kAccessPolicyEmbedRoot, std::move(client_));
+WindowTreeImpl* WindowTreeHostConnectionImpl::CreateWindowTree(
+    ServerWindow* root) {
+  WindowTreeImpl* tree = connection_manager_->EmbedAtWindow(
+      root, mojom::WindowTree::kAccessPolicyEmbedRoot, std::move(client_));
   tree->ConfigureWindowManager();
-  set_window_tree(tree);
+  return tree;
 }
 
 }  // namespace ws
-
 }  // namespace mus