Implement temporary reference assignment with DisplayCompositor.

services/ui/ws/window_server.cc

Index: services/ui/ws/window_server.cc
diff --git a/services/ui/ws/window_server.cc b/services/ui/ws/window_server.cc
index 3b0446dea15217be18930c59287a6f6f4925ca74..7c7479b43ea0abb7e736984465226e55e54b2424 100644
--- a/services/ui/ws/window_server.cc
+++ b/services/ui/ws/window_server.cc
@@ -606,6 +606,29 @@ bool WindowServer::IsUserInHighContrastMode(const UserId& user) const {
   return (iter == high_contrast_mode_.end()) ? false : iter->second;
 }
 
+ServerWindow* WindowServer::FindCompositorFrameSinkParent(
+    ServerWindow* window) {
+  if (!window->parent())
+    return nullptr;
+
+  // TODO(kylechar): Add a bool to ServerWindow to indicate if the window is
+  // backed by a CompositorFrameSink (eg. it will submit CompositorFrames).
+
+  // If the parent of |window| is part of a WindowTree, the WindowTree root that

[sky, 2017/02/28 20:42:54]

Can you describe why does this code need to check the parent?

[kylechar, 2017/02/28 20:54:05]

A new cc::Surface that corresponds to |window| was just created. The WS needs to
find the FrameSinkId of the CompositorFrameSink that will embed that new surface
so it can tell the GPU. This uses the ServerWindow hierarchy to find that parent
(and the correspond FrameSinkId).

Assuming that all CFSs had been created then we could walk up window->parent()
until we found the ServerWindow that had a
ServerWindowCompositorFrameSinkManager. Unfortunately that happens
asynchronously and the embedded client could submit a surface before the parent
client has created a CompositorFrameSink. So walking up the parents won't work.
The TODO above alludes to that and would make this easier.

This assumes that the root of each WindowTree has a cc::Surface and that the
surface hierarchy for each client is only ever 1 level deep. This is true right
now (AFAIK) but probably needs the bool value to work in all future cases.

[kylechar, 2017/02/28 20:54:52]

Want the comments for this method expanded to explain?

+  // contains |window| should be backed by a CompositorFrameSink.
+  WindowTree* window_tree = GetTreeWithId(window->parent()->id().client_id);
+  if (window_tree) {
+    for (const ServerWindow* root : window_tree->roots()) {
+      if (root->Contains(window))
+        // TODO(kylechar): Walk up parents until one with the backed by
+        // CompositorFrameSink bool is found instead.
+        return const_cast<ServerWindow*>(root);
+    }
+  }
+
+  return nullptr;
+}
+
 ServerWindow* WindowServer::GetRootWindow(const ServerWindow* window) {
   Display* display = display_manager_->GetDisplayContaining(window);
   return display ? display->root_window() : nullptr;
@@ -770,8 +793,6 @@ void WindowServer::OnTransientWindowRemoved(ServerWindow* window,
 }
 
 void WindowServer::OnGpuServiceInitialized() {
-  // TODO(kylechar): When gpu channel is removed, this can instead happen
-  // earlier, after GpuHost::OnInitialized().
   delegate_->StartDisplayInit();
 }
 
@@ -779,9 +800,14 @@ void WindowServer::OnSurfaceCreated(const cc::SurfaceInfo& surface_info) {
   WindowId window_id(
       WindowIdFromTransportId(surface_info.id().frame_sink_id().client_id()));
   ServerWindow* window = GetWindow(window_id);
+
   // If the window doesn't have a parent then we have nothing to propagate.
-  if (!window)
+  if (!window) {
+    display_compositor_->DropTemporaryReference(surface_info.id());
     return;
+  }
+
+  ServerWindow* compositing_parent = nullptr;
 
   // FrameGenerator will add an appropriate reference for the new surface.
   DCHECK(display_manager_->GetDisplayContaining(window));
@@ -789,6 +815,7 @@ void WindowServer::OnSurfaceCreated(const cc::SurfaceInfo& surface_info) {
   if (window == display->GetActiveRootWindow()) {
     display->platform_display()->GetFrameGenerator()->OnSurfaceCreated(
         surface_info);
+    compositing_parent = display->root_window();
   }
 
   // This is only used for testing to observe that a window has a
@@ -796,12 +823,31 @@ void WindowServer::OnSurfaceCreated(const cc::SurfaceInfo& surface_info) {
   if (!window_paint_callback_.is_null())
     window_paint_callback_.Run(window);
 
-  if (!window->parent())
+  if (!window->parent()) {
+    // If |window| is a display root then a surface reference has already been
+    // added by the DisplayCompositor. If not then nothing embeds |window|.
+    display_compositor_->DropTemporaryReference(surface_info.id());
     return;
+  }
 
   WindowTree* window_tree = GetTreeWithId(window->parent()->id().client_id);
   if (window_tree)
     window_tree->ProcessWindowSurfaceChanged(window, surface_info);
+
+  if (!compositing_parent)
+    compositing_parent = FindCompositorFrameSinkParent(window);
+
+  if (compositing_parent) {
+    // The parent ServerWindow backed by a CompositorFrameSink is expected to
+    // add surface reference to the new surface. Let the DisplayCompositor know
+    // who the parent is in case something happens to the parent before it can
+    // add a surface reference. This ensures there are no dangling temporary
+    // references.
+    compositing_parent->GetOrCreateCompositorFrameSinkManager()
+        ->ClaimTemporaryReference(surface_info.id());
+  } else {
+    display_compositor_->DropTemporaryReference(surface_info.id());
+  }
 }
 
 void WindowServer::OnActiveUserIdChanged(const UserId& previously_active_id,