Convert mustash use surface references.

services/ui/ws/frame_generator.cc

Index: services/ui/ws/frame_generator.cc
diff --git a/services/ui/ws/frame_generator.cc b/services/ui/ws/frame_generator.cc
index d9333340ad44ef18123c913388d8d969e8a5c36b..8d9b353445db017c9be40bb4fb3ceb3c37a3f4fe 100644
--- a/services/ui/ws/frame_generator.cc
+++ b/services/ui/ws/frame_generator.cc
@@ -10,7 +10,6 @@
 #include "cc/quads/render_pass_draw_quad.h"
 #include "cc/quads/shared_quad_state.h"
 #include "cc/quads/surface_draw_quad.h"
-#include "cc/surfaces/surface_id.h"
 #include "gpu/ipc/client/gpu_channel_host.h"
 #include "services/ui/surfaces/display_compositor.h"
 #include "services/ui/surfaces/surfaces_context_provider.h"
@@ -22,22 +21,44 @@
 namespace ui {
 
 namespace ws {
+namespace {
+
+// Walks up window tree and finds the SurfaceId for the ServerWindow embedding
+// |window|. If |window| is a display root, then we return the surface
+// manager top level root.
+cc::SurfaceId FindParentSurfaceId(ServerWindow* window) {
+  if (!window->parent())
+    return cc::SurfaceManager::kRootSurfaceId;
+
+  ServerWindow* current = window;
+  while (current->parent()) {
+    current = current->parent();
+    if (current->compositor_frame_sink_manager() &&
+        current->compositor_frame_sink_manager()->HasCompositorFrameSinkOfType(
+            mojom::CompositorFrameSinkType::DEFAULT)) {
+      return current->compositor_frame_sink_manager()->GetLatestSurfaceId(
+          mojom::CompositorFrameSinkType::DEFAULT);
+    }
+  }
+
+  NOTREACHED();
+  return cc::SurfaceId();
+}
+
+}  // namespace
 
 FrameGenerator::FrameGenerator(FrameGeneratorDelegate* delegate,
                                ServerWindow* root_window)
     : delegate_(delegate),
-      frame_sink_id_(
-          WindowIdToTransportId(root_window->id()),
-          static_cast<uint32_t>(mojom::CompositorFrameSinkType::DEFAULT)),
       root_window_(root_window),
       binding_(this),
       weak_factory_(this) {
   DCHECK(delegate_);
-  surface_sequence_generator_.set_frame_sink_id(frame_sink_id_);
 }
 
 FrameGenerator::~FrameGenerator() {
-  ReleaseAllSurfaceReferences();
+  RemoveDeadSurfaceReferences();
+  RemoveAllSurfaceReferences();
   // Invalidate WeakPtrs now to avoid callbacks back into the
   // FrameGenerator during destruction of |compositor_frame_sink_|.
   weak_factory_.InvalidateWeakPtrs();
@@ -86,6 +107,48 @@ void FrameGenerator::OnAcceleratedWidgetAvailable(
   }
 }
 
+void FrameGenerator::AddSurfaceReference(const cc::SurfaceId& surface_id,
+                                         ServerWindow* window) {
+  DCHECK(!surface_id.is_null());
+
+  auto it = active_references_.find(surface_id.frame_sink_id());
+  if (it == active_references_.end()) {
+    cc::SurfaceId parent_id = FindParentSurfaceId(window);
+
+    // Add new reference from parent to surface, plus add reference to local
+    // cache.
+    GetDisplayCompositor()->AddSurfaceReference(parent_id, surface_id);
+    active_references_[surface_id.frame_sink_id()] =
+        SurfaceReference({parent_id, surface_id});
+
+    // Observe |window| so that we can remove references when it's destroyed.
+    Add(window);
+    return;
+  }
+
+  SurfaceReference& ref = it->second;
+  DCHECK(surface_id.frame_sink_id() == ref.child_id.frame_sink_id());
+
+  // Check if we're holding a reference to the surface and return early.
+  if (surface_id.local_frame_id() == ref.child_id.local_frame_id())
+    return;
+
+  // Add a reference from parent to new surface first.
+  GetDisplayCompositor()->AddSurfaceReference(ref.parent_id, surface_id);
+
+  // If the display root surface has changed, update all references to embedded
+  // surfaces.
+  if (!window->parent())
+    AddNewParentReferences(ref.child_id, surface_id);
+
+  // Move the existing reference to list of references to remove after we submit
+  // the next CompositorFrame and update local reference cache to be the new
+  // reference. If this is the display root surface then removing this reference
+  // will recursively remove any references it held.
+  dead_references_.push_back(ref);
+  ref.child_id = surface_id;
+}
+
 void FrameGenerator::DidReceiveCompositorFrameAck() {}
 
 void FrameGenerator::OnBeginFrame(const cc::BeginFrameArgs& begin_frame_arags) {
@@ -94,8 +157,15 @@ void FrameGenerator::OnBeginFrame(const cc::BeginFrameArgs& begin_frame_arags) {
 
   // TODO(fsamuel): We should add a trace for generating a top level frame.
   cc::CompositorFrame frame(GenerateCompositorFrame(root_window_->bounds()));
-  if (compositor_frame_sink_)
+  if (compositor_frame_sink_) {
     compositor_frame_sink_->SubmitCompositorFrame(std::move(frame));
+
+    // Remove dead references after we submit a frame. This has to happen after
+    // the frame is submitted otherwise we could end up deleting a surface that
+    // is still embedded in the last frame.
+    // TODO(kylechar): This should be synchronized with SubmitCompositorFrame().
+    RemoveDeadSurfaceReferences();
+  }
 }
 
 void FrameGenerator::ReclaimResources(
@@ -186,8 +256,6 @@ void FrameGenerator::DrawWindowTree(
                 combined_opacity, SkXfermode::kSrcOver_Mode,
                 0 /* sorting-context_id */);
     auto* quad = pass->CreateAndAppendDrawQuad<cc::SurfaceDrawQuad>();
-    AddOrUpdateSurfaceReference(mojom::CompositorFrameSinkType::DEFAULT,
-                                window);
     quad->SetAll(sqs, bounds_at_origin /* rect */,
                  gfx::Rect() /* opaque_rect */,
                  bounds_at_origin /* visible_rect */, true /* needs_blending*/,
@@ -211,8 +279,6 @@ void FrameGenerator::DrawWindowTree(
                 0 /* sorting-context_id */);
 
     auto* quad = pass->CreateAndAppendDrawQuad<cc::SurfaceDrawQuad>();
-    AddOrUpdateSurfaceReference(mojom::CompositorFrameSinkType::UNDERLAY,
-                                window);
     quad->SetAll(sqs, bounds_at_origin /* rect */,
                  gfx::Rect() /* opaque_rect */,
                  bounds_at_origin /* visible_rect */, true /* needs_blending*/,
@@ -220,63 +286,48 @@ void FrameGenerator::DrawWindowTree(
   }
 }
 
-void FrameGenerator::AddOrUpdateSurfaceReference(
-    mojom::CompositorFrameSinkType type,
-    ServerWindow* window) {
-  cc::SurfaceId surface_id =
-      window->compositor_frame_sink_manager()->GetLatestSurfaceId(type);
-  if (surface_id.is_null())
-    return;
-  auto it = dependencies_.find(surface_id.frame_sink_id());
-  if (it == dependencies_.end()) {
-    SurfaceDependency dependency = {
-        surface_id.local_frame_id(),
-        surface_sequence_generator_.CreateSurfaceSequence()};
-    dependencies_[surface_id.frame_sink_id()] = dependency;
-    GetDisplayCompositor()->AddSurfaceReference(surface_id,
-                                                dependency.sequence);
-    // Observe |window_surface|'s window so that we can release references when
-    // the window is destroyed.
-    Add(window);
-    return;
+void FrameGenerator::AddNewParentReferences(
+    const cc::SurfaceId& old_surface_id,
+    const cc::SurfaceId& new_surface_id) {
+  DCHECK(old_surface_id.frame_sink_id() == new_surface_id.frame_sink_id());
+
+  DisplayCompositor* display_compositor = GetDisplayCompositor();
+  for (auto& map_entry : active_references_) {
+    SurfaceReference& ref = map_entry.second;
+    if (ref.parent_id == old_surface_id) {
+      display_compositor->AddSurfaceReference(new_surface_id, ref.child_id);
+      ref.parent_id = new_surface_id;
+    }
   }
+}
 
-  // We are already holding a reference to this surface so there's no work to do
-  // here.
-  if (surface_id.local_frame_id() == it->second.local_frame_id)
+void FrameGenerator::RemoveDeadSurfaceReferences() {
+  if (dead_references_.empty())
     return;
 
-  // If we have have an existing reference to a surface from the given
-  // FrameSink, then we should release the reference, and then add this new
-  // reference. This results in a delete and lookup in the map but simplifies
-  // the code.
-  ReleaseFrameSinkReference(surface_id.frame_sink_id());
-
-  // This recursion will always terminate. This line is being called because
-  // there was a stale surface reference. The stale reference has been released
-  // in the previous line and cleared from the dependencies_ map. Thus, in the
-  // recursive call, we'll enter the second if blcok because the FrameSinkId
-  // is no longer referenced in the map.
-  AddOrUpdateSurfaceReference(type, window);
+  DisplayCompositor* display_compositor = GetDisplayCompositor();
+  for (auto& ref : dead_references_) {

[Fady Samuel, 2016/11/10 13:53:02]

nit: no braces necessary.

[kylechar, 2016/11/14 19:05:38]

Acknowledged.

+    display_compositor->RemoveSurfaceReference(ref.parent_id, ref.child_id);
+  }
+  dead_references_.clear();
 }
 
-void FrameGenerator::ReleaseFrameSinkReference(
+void FrameGenerator::RemoveFrameSinkReference(
     const cc::FrameSinkId& frame_sink_id) {
-  auto it = dependencies_.find(frame_sink_id);
-  if (it == dependencies_.end())
+  auto it = active_references_.find(frame_sink_id);
+  if (it == active_references_.end())
     return;
-  std::vector<uint32_t> sequences;
-  sequences.push_back(it->second.sequence.sequence);
-  GetDisplayCompositor()->ReturnSurfaceReferences(frame_sink_id, sequences);
-  dependencies_.erase(it);
+  dead_references_.push_back(it->second);
+  active_references_.erase(it);
 }
 
-void FrameGenerator::ReleaseAllSurfaceReferences() {
-  std::vector<uint32_t> sequences;
-  for (auto& dependency : dependencies_)
-    sequences.push_back(dependency.second.sequence.sequence);
-  GetDisplayCompositor()->ReturnSurfaceReferences(frame_sink_id_, sequences);
-  dependencies_.clear();
+void FrameGenerator::RemoveAllSurfaceReferences() {
+  DisplayCompositor* display_compositor = GetDisplayCompositor();
+  for (auto& map_entry : active_references_) {
+    const SurfaceReference& ref = map_entry.second;
+    display_compositor->RemoveSurfaceReference(ref.parent_id, ref.child_id);
+  }
+  active_references_.clear();
 }
 
 ui::DisplayCompositor* FrameGenerator::GetDisplayCompositor() {
@@ -296,13 +347,13 @@ void FrameGenerator::OnWindowDestroying(ServerWindow* window) {
       window->compositor_frame_sink_manager()->GetLatestSurfaceId(
           mojom::CompositorFrameSinkType::DEFAULT);
   if (!default_surface_id.is_null())
-    ReleaseFrameSinkReference(default_surface_id.frame_sink_id());
+    RemoveFrameSinkReference(default_surface_id.frame_sink_id());
 
   cc::SurfaceId underlay_surface_id =
       window->compositor_frame_sink_manager()->GetLatestSurfaceId(
           mojom::CompositorFrameSinkType::UNDERLAY);
   if (!underlay_surface_id.is_null())
-    ReleaseFrameSinkReference(underlay_surface_id.frame_sink_id());
+    RemoveFrameSinkReference(underlay_surface_id.frame_sink_id());
 }
 
 }  // namespace ws