cc: Register namespace hierarchy after ui::Compositor's SurfaceFactory available

In the future, we'd like to merge Surface client ID registration and
SurfaceFactory creation. SurfaceFactory is tied to an OutputSurface. Thus, we
cannot register a namespace hierarchy relationship until after we create an
OutputSurface for the ui::Compositor. However, we create a new OutputSurface
every time the GPU process is restarted. Thus, we need a mechanism to store
these relationships between clients across GPU restarts.

This is particularly important if the display compositor lives in the same
process GPU and these relationships would be lost. This CL stores the
relationship between DelegatedFrameHost and ui::Compositor in ui::Compositor.

Note that depending on the final shape of the display compositor host API,
we may wish to centralize all hierarchy relationship registration including
OOPIF.

BUG=629940
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_blink_rel

Review-Url: https://codereview.chromium.org/2175293003
Cr-Commit-Position: refs/heads/master@{#407770}

cc/trees/layer_tree_host.h

@@ -299,6 +299,8 @@ class CC_EXPORT LayerTreeHost : public MutatorHostClient {
 
   bool in_paint_layer_contents() const { return in_paint_layer_contents_; }
 
+  bool has_output_surface() const { return !!current_output_surface_; }
+
   // CreateUIResource creates a resource given a bitmap.  The bitmap is
   // generated via an interface function, which is called when initializing the
   // resource and when the resource has been lost (due to lost context).  The

content/browser/renderer_host/delegated_frame_host.cc

@@ -837,10 +837,7 @@ void DelegatedFrameHost::SetCompositor(ui::Compositor* compositor) {
   vsync_manager_ = compositor_->vsync_manager();
   vsync_manager_->AddObserver(this);
 
-  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
-  uint32_t parent = compositor->surface_id_allocator()->client_id();
-  factory->GetSurfaceManager()->RegisterSurfaceNamespaceHierarchy(
-      parent, id_allocator_->client_id());
+  compositor_->AddSurfaceClient(id_allocator_->client_id());
 }
 
 void DelegatedFrameHost::ResetCompositor() {
@@ -857,11 +854,7 @@ void DelegatedFrameHost::ResetCompositor() {
     vsync_manager_ = NULL;
   }
 
-  ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
-  uint32_t parent = compositor_->surface_id_allocator()->client_id();
-  factory->GetSurfaceManager()->UnregisterSurfaceNamespaceHierarchy(
-      parent, id_allocator_->client_id());
-
+  compositor_->RemoveSurfaceClient(id_allocator_->client_id());
   compositor_ = nullptr;
 }
 

ui/compositor/compositor.cc

@@ -245,15 +245,58 @@ Compositor::~Compositor() {
 
   context_factory_->RemoveCompositor(this);
   if (context_factory_->GetSurfaceManager()) {
+    for (auto& client : surface_clients_) {
+      if (client.second) {
+        context_factory_->GetSurfaceManager()
+            ->UnregisterSurfaceNamespaceHierarchy(client.second, client.first);
+      }
+    }
     context_factory_->GetSurfaceManager()->InvalidateSurfaceClientId(
         surface_id_allocator_->client_id());
   }
 }
 
+void Compositor::AddSurfaceClient(uint32_t client_id) {
+  // We don't give the client a parent until the ui::Compositor has an
+  // OutputSurface.
+  uint32_t parent_client_id = 0;
+  if (host_->has_output_surface()) {
+    parent_client_id = surface_id_allocator_->client_id();
+    context_factory_->GetSurfaceManager()->RegisterSurfaceNamespaceHierarchy(
+        parent_client_id, client_id);
+  }
+  surface_clients_[client_id] = parent_client_id;
+}
+
+void Compositor::RemoveSurfaceClient(uint32_t client_id) {
+  auto it = surface_clients_.find(client_id);
+  DCHECK(it != surface_clients_.end());
+  if (host_->has_output_surface()) {
+    context_factory_->GetSurfaceManager()->UnregisterSurfaceNamespaceHierarchy(
+        it->second, client_id);
+  }
+  surface_clients_.erase(it);
+}
+
 void Compositor::SetOutputSurface(
     std::unique_ptr<cc::OutputSurface> output_surface) {
   output_surface_requested_ = false;
   host_->SetOutputSurface(std::move(output_surface));
+  // ui::Compositor uses a SingleThreadProxy and so BindToClient will be called
+  // above and SurfaceManager will be made aware of the OutputSurface's client
+  // ID.
+  for (auto& client : surface_clients_) {
+    if (client.second == surface_id_allocator_->client_id())
+      continue;
+    // If a client already has a parent, then we unregister the existing parent.
+    if (client.second) {
+      context_factory_->GetSurfaceManager()
+          ->UnregisterSurfaceNamespaceHierarchy(client.second, client.first);
+    }
+    context_factory_->GetSurfaceManager()->RegisterSurfaceNamespaceHierarchy(
+        surface_id_allocator_->client_id(), client.first);
+    client.second = surface_id_allocator_->client_id();
+  }
 }
 
 void Compositor::ScheduleDraw() {
@@ -363,8 +406,14 @@ void Compositor::SetAcceleratedWidget(gfx::AcceleratedWidget widget) {
 
 gfx::AcceleratedWidget Compositor::ReleaseAcceleratedWidget() {
   DCHECK(!IsVisible());
-  if (!host_->output_surface_lost())
+  if (!host_->output_surface_lost()) {
     host_->ReleaseOutputSurface();
+    for (auto& client : surface_clients_) {
+      context_factory_->GetSurfaceManager()
+          ->UnregisterSurfaceNamespaceHierarchy(client.second, client.first);
+      client.second = 0;
+    }
+  }
   context_factory_->RemoveCompositor(this);
   widget_valid_ = false;
   gfx::AcceleratedWidget widget = widget_;

ui/compositor/compositor.h

@@ -198,6 +198,12 @@ class COMPOSITOR_EXPORT Compositor
 
   ui::ContextFactory* context_factory() { return context_factory_; }
 
+  void AddSurfaceClient(uint32_t client_id);
+  void RemoveSurfaceClient(uint32_t client_id);
+  const std::unordered_map<uint32_t, uint32_t>& SurfaceClientsForTesting() {
+    return surface_clients_;
+  }
+
   void SetOutputSurface(std::unique_ptr<cc::OutputSurface> surface);
 
   // Schedules a redraw of the layer tree associated with this compositor.
@@ -379,6 +385,7 @@ class COMPOSITOR_EXPORT Compositor
   base::ObserverList<CompositorAnimationObserver> animation_observer_list_;
 
   gfx::AcceleratedWidget widget_;
+  std::unordered_map<uint32_t, uint32_t> surface_clients_;
   bool widget_valid_;
   bool output_surface_requested_;
   std::unique_ptr<cc::SurfaceIdAllocator> surface_id_allocator_;

ui/compositor/compositor_unittest.cc

@@ -2,10 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <stdint.h>
+
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "cc/output/begin_frame_args.h"
+#include "cc/surfaces/surface_factory.h"
+#include "cc/surfaces/surface_factory_client.h"
+#include "cc/surfaces/surface_id_allocator.h"
+#include "cc/surfaces/surface_manager.h"
 #include "cc/test/begin_frame_args_test.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -20,6 +26,35 @@ using testing::_;
 namespace ui {
 namespace {
 
+class FakeCompositorFrameSink : public cc::SurfaceFactoryClient {
+ public:
+  FakeCompositorFrameSink(uint32_t client_id, cc::SurfaceManager* manager)
+      : client_id_(client_id),
+        manager_(manager),
+        source_(nullptr),
+        factory_(manager, this) {
+    manager_->RegisterSurfaceClientId(client_id_);
+    manager_->RegisterSurfaceFactoryClient(client_id_, this);
+  }
+
+  ~FakeCompositorFrameSink() override {
+    manager_->UnregisterSurfaceFactoryClient(client_id_);
+    manager_->InvalidateSurfaceClientId(client_id_);
+  }
+
+  void ReturnResources(const cc::ReturnedResourceArray& resources) override {}
+  void SetBeginFrameSource(cc::BeginFrameSource* begin_frame_source) override {
+    DCHECK(!source_ || !begin_frame_source);
+    source_ = begin_frame_source;
+  };
+
+ private:
+  const uint32_t client_id_;
+  cc::SurfaceManager* const manager_;
+  cc::BeginFrameSource* source_;
+  cc::SurfaceFactory factory_;
+};
+
 ACTION_P2(RemoveObserver, compositor, observer) {
   compositor->RemoveBeginFrameObserver(observer);
 }
@@ -94,6 +129,71 @@ TEST_F(CompositorTest, ReleaseWidgetWithOutputSurfaceNeverCreated) {
   compositor()->SetVisible(true);
 }
 
+TEST_F(CompositorTest, SurfaceClients) {
+  const uint32_t kClientId1 =
+      compositor()->surface_id_allocator()->client_id() + 1;
+  const uint32_t kClientId2 =
+      compositor()->surface_id_allocator()->client_id() + 2;
+  const uint32_t kClientId3 =
+      compositor()->surface_id_allocator()->client_id() + 3;
+  cc::SurfaceManager* manager =
+      compositor()->context_factory()->GetSurfaceManager();
+  FakeCompositorFrameSink client1(kClientId1, manager);
+  FakeCompositorFrameSink client2(kClientId2, manager);
+  FakeCompositorFrameSink client3(kClientId3, manager);
+  const uint32_t kNoClient = 0;
+
+  compositor()->AddSurfaceClient(kClientId1);
+  compositor()->AddSurfaceClient(kClientId2);
+  compositor()->AddSurfaceClient(kClientId3);
+
+  const std::unordered_map<uint32_t, uint32_t>& surface_clients =
+      compositor()->SurfaceClientsForTesting();
+  EXPECT_EQ(3u, surface_clients.size());
+  EXPECT_NE(surface_clients.end(), surface_clients.find(kClientId1));
+  EXPECT_NE(surface_clients.end(), surface_clients.find(kClientId2));
+  EXPECT_NE(surface_clients.end(), surface_clients.find(kClientId3));
+
+  // Verify that the clients haven't been assigned a parent compositor yet.
+  EXPECT_EQ(kNoClient, surface_clients.find(kClientId1)->second);
+  EXPECT_EQ(kNoClient, surface_clients.find(kClientId2)->second);
+  EXPECT_EQ(kNoClient, surface_clients.find(kClientId3)->second);
+
+  // This will trigger the creation of an OutputSurface and then
+  // assignment of a surface hierarchy.
+  std::unique_ptr<Layer> root_layer(new Layer(ui::LAYER_SOLID_COLOR));
+  root_layer->SetBounds(gfx::Rect(10, 10));
+  compositor()->SetRootLayer(root_layer.get());
+  compositor()->SetScaleAndSize(1.0f, gfx::Size(10, 10));
+  compositor()->SetVisible(true);
+  compositor()->ScheduleDraw();
+  DrawWaiterForTest::WaitForCompositingEnded(compositor());
+
+  // Verify that the clients have been assigned a parent compositor.
+  EXPECT_EQ(compositor()->surface_id_allocator()->client_id(),
+            surface_clients.find(kClientId1)->second);
+  EXPECT_EQ(compositor()->surface_id_allocator()->client_id(),
+            surface_clients.find(kClientId2)->second);
+  EXPECT_EQ(compositor()->surface_id_allocator()->client_id(),
+            surface_clients.find(kClientId3)->second);
+
+  // Remove a client while the parent compositor has an OutputSurface.
+  compositor()->RemoveSurfaceClient(kClientId3);
+  EXPECT_EQ(surface_clients.end(), surface_clients.find(kClientId3));
+
+  // This will remove parent compositor from the surface hierarchy.
+  compositor()->SetVisible(false);
+  compositor()->ReleaseAcceleratedWidget();
+  EXPECT_EQ(kNoClient, surface_clients.find(kClientId1)->second);
+  EXPECT_EQ(kNoClient, surface_clients.find(kClientId2)->second);
+
+  // Remove a client after the OutputSurface has been dropped.
+  compositor()->RemoveSurfaceClient(kClientId2);
+  EXPECT_EQ(surface_clients.end(), surface_clients.find(kClientId2));
+
+  // The remaining client will be cleared during destruction.
+}
+
 #if defined(OS_WIN)
 // TODO(crbug.com/608436): Flaky on windows trybots
 #define MAYBE_CreateAndReleaseOutputSurface \