Add/remove surface references via MojoCompositorFrameSink.

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 0c869ea54129cd2505cd770cb33576e8f3e0eb8a..20fcd99216d3d6c33d53b9aadb7f753cafae9b3a 100644
--- a/services/ui/ws/frame_generator.cc
+++ b/services/ui/ws/frame_generator.cc
@@ -4,13 +4,14 @@
 
 #include "services/ui/ws/frame_generator.h"
 
+#include <utility>
+
 #include "base/containers/adapters.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/quads/render_pass.h"
 #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 "services/ui/ws/frame_generator_delegate.h"
 #include "services/ui/ws/server_window.h"
 #include "services/ui/ws/server_window_compositor_frame_sink_manager.h"
@@ -24,16 +25,12 @@ FrameGenerator::FrameGenerator(FrameGeneratorDelegate* delegate,
                                ServerWindow* root_window)
     : delegate_(delegate),
       root_window_(root_window),
-      top_level_root_surface_id_(
-          cc::FrameSinkId(0, 0),
-          cc::LocalFrameId(0, base::UnguessableToken::Create())),
       binding_(this),
       weak_factory_(this) {
   DCHECK(delegate_);
 }
 
 FrameGenerator::~FrameGenerator() {
-  RemoveDeadSurfaceReferences();
   RemoveAllSurfaceReferences();
   // Invalidate WeakPtrs now to avoid callbacks back into the
   // FrameGenerator during destruction of |compositor_frame_sink_|.
@@ -60,36 +57,34 @@ void FrameGenerator::OnSurfaceCreated(const cc::SurfaceId& surface_id,
                                       ServerWindow* window) {
   DCHECK(surface_id.is_valid());
 
-  // TODO(kylechar): Adding surface references should be synchronized with
-  // SubmitCompositorFrame().
-
   auto iter = active_references_.find(surface_id.frame_sink_id());
   if (iter == active_references_.end()) {
     AddFirstReference(surface_id, window);
     return;
   }
 
-  SurfaceReference& ref = iter->second;
-  DCHECK_EQ(surface_id.frame_sink_id(), ref.child_id.frame_sink_id());
+  cc::SurfaceReference& ref = iter->second;
 
   // This shouldn't be called multiple times for the same SurfaceId.
-  DCHECK_NE(surface_id.local_frame_id(), ref.child_id.local_frame_id());
-
-  // 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. For example, this would happen when the display resolution or
-  // zoom level changes.
-  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. 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;
+  DCHECK_EQ(surface_id.frame_sink_id(), ref.child_id().frame_sink_id());
+  DCHECK_NE(surface_id.local_frame_id(), ref.child_id().local_frame_id());
+
+  // The current reference will be removed after the next CompositorFrame is
+  // submitted or FrameGenerator is destroyed.
+  references_to_remove_.push_back(ref);
+  cc::SurfaceId old_surface_id = ref.child_id();
+
+  // New surface reference is recorded and will be added at end of this method.
+  ref = cc::SurfaceReference(ref.parent_id(), surface_id);
+  references_to_add_.push_back(ref);
+
+  // If the display root surface has changed, add references from the new
+  // SurfaceId to all embedded surfaces. For example, this would happen when the
+  // display resolution or device scale factor changes.
+  if (window == root_window_)
+    AddNewParentReferences(old_surface_id, surface_id);
+
+  PerformAddSurfaceReferences();
 }
 
 void FrameGenerator::DidReceiveCompositorFrameAck() {}
@@ -114,8 +109,7 @@ void FrameGenerator::OnBeginFrame(const cc::BeginFrameArgs& begin_frame_arags) {
     // 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();
+    PerformRemoveSurfaceReferences();
   }
 }
 
@@ -241,7 +235,7 @@ void FrameGenerator::DrawWindowTree(
 
 cc::SurfaceId FrameGenerator::FindParentSurfaceId(ServerWindow* window) {
   if (window == root_window_)
-    return top_level_root_surface_id_;
+    return root_window_->delegate()->GetRootSurfaceId();
 
   // The root window holds the parent SurfaceId. This SurfaceId will have an
   // invalid LocalFrameId before FrameGenerator has submitted a CompositorFrame.
@@ -252,31 +246,30 @@ cc::SurfaceId FrameGenerator::FindParentSurfaceId(ServerWindow* window) {
 
 void FrameGenerator::AddSurfaceReference(const cc::SurfaceId& parent_id,
                                          const cc::SurfaceId& child_id) {
-  if (parent_id == top_level_root_surface_id_)
-    GetDisplayCompositor()->AddRootSurfaceReference(child_id);
-  else
-    GetDisplayCompositor()->AddSurfaceReference(parent_id, child_id);
-
-  // Add new reference from parent to surface, plus add reference to local
-  // cache.
-  active_references_[child_id.frame_sink_id()] =
-      SurfaceReference({parent_id, child_id});
+  DCHECK_NE(parent_id, child_id);
+
+  // Add new reference from parent to surface and record reference.
+  cc::SurfaceReference ref(parent_id, child_id);
+  active_references_[child_id.frame_sink_id()] = ref;
+  references_to_add_.push_back(ref);
 }
 
 void FrameGenerator::AddFirstReference(const cc::SurfaceId& surface_id,
                                        ServerWindow* window) {
   cc::SurfaceId parent_id = FindParentSurfaceId(window);
 
-  if (parent_id == top_level_root_surface_id_ || parent_id.is_valid()) {
+  if (parent_id.local_frame_id().is_valid()) {
     AddSurfaceReference(parent_id, surface_id);
 
     // For the first display root surface, add references to any child surfaces
-    // that were created before it.
-    if (parent_id == top_level_root_surface_id_) {
+    // that were created before it, since no reference has been added yet.
+    if (window == root_window_) {
       for (auto& child_surface_id : waiting_for_references_)
         AddSurfaceReference(surface_id, child_surface_id);
       waiting_for_references_.clear();
     }
+
+    PerformAddSurfaceReferences();
   } else {
     // This isn't the display root surface and display root surface hasn't
     // submitted a CF yet. We can't add a reference to an unknown SurfaceId.
@@ -290,30 +283,31 @@ void FrameGenerator::AddFirstReference(const cc::SurfaceId& surface_id,
 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());
+  DCHECK_EQ(old_surface_id.frame_sink_id(), new_surface_id.frame_sink_id());
 
-  cc::mojom::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;
+    cc::SurfaceReference& ref = map_entry.second;
+    if (ref.parent_id() == old_surface_id) {
+      ref = cc::SurfaceReference(new_surface_id, ref.child_id());
+      references_to_add_.push_back(ref);
     }
   }
 }
 
-void FrameGenerator::RemoveDeadSurfaceReferences() {
-  if (dead_references_.empty())
+void FrameGenerator::PerformAddSurfaceReferences() {
+  if (references_to_add_.empty())
     return;
 
-  cc::mojom::DisplayCompositor* display_compositor = GetDisplayCompositor();
-  for (auto& ref : dead_references_) {
-    if (ref.parent_id == top_level_root_surface_id_)
-      display_compositor->RemoveRootSurfaceReference(ref.child_id);
-    else
-      display_compositor->RemoveSurfaceReference(ref.parent_id, ref.child_id);
-  }
-  dead_references_.clear();
+  compositor_frame_sink_->AddSurfaceReferences(references_to_add_);
+  references_to_add_.clear();
+}
+
+void FrameGenerator::PerformRemoveSurfaceReferences() {
+  if (references_to_remove_.empty())
+    return;
+
+  compositor_frame_sink_->RemoveSurfaceReferences(references_to_remove_);
+  references_to_remove_.clear();
 }
 
 void FrameGenerator::RemoveFrameSinkReference(
@@ -321,22 +315,15 @@ void FrameGenerator::RemoveFrameSinkReference(
   auto it = active_references_.find(frame_sink_id);
   if (it == active_references_.end())
     return;
-  dead_references_.push_back(it->second);
+  references_to_remove_.push_back(it->second);
   active_references_.erase(it);
 }
 
 void FrameGenerator::RemoveAllSurfaceReferences() {
-  // TODO(kylechar): Remove multiple surfaces with one IPC call.
-  cc::mojom::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);
-  }
+  for (auto& map_entry : active_references_)
+    references_to_remove_.push_back(map_entry.second);
   active_references_.clear();
-}
-
-cc::mojom::DisplayCompositor* FrameGenerator::GetDisplayCompositor() {
-  return root_window_->delegate()->GetDisplayCompositor();
+  PerformRemoveSurfaceReferences();
 }
 
 void FrameGenerator::OnWindowDestroying(ServerWindow* window) {