Mus+Ash: Use standard cc mechanism for surface lifetime.

services/ui/ws/frame_generator.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "services/ui/ws/frame_generator.h"
 
 #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 "gpu/ipc/client/gpu_channel_host.h"
 #include "services/ui/surfaces/compositor_frame_sink.h"
 #include "services/ui/ws/frame_generator_delegate.h"
 #include "services/ui/ws/server_window.h"
 #include "services/ui/ws/server_window_surface.h"
 #include "services/ui/ws/server_window_surface_manager.h"
 
 namespace ui {
 
 namespace ws {
 
 FrameGenerator::FrameGenerator(
     FrameGeneratorDelegate* delegate,
     scoped_refptr<DisplayCompositor> display_compositor)
     : delegate_(delegate),
       display_compositor_(display_compositor),
       frame_sink_id_(0, display_compositor->GenerateNextClientId()),
       draw_timer_(false, false),
       weak_factory_(this) {
   DCHECK(delegate_);
+  surface_sequence_generator_.set_frame_sink_id(frame_sink_id_);
 }
 
 FrameGenerator::~FrameGenerator() {
+  ReleaseAllSurfaceReferences();
   // Invalidate WeakPtrs now to avoid callbacks back into the
   // FrameGenerator during destruction of |compositor_frame_sink_|.
   weak_factory_.InvalidateWeakPtrs();
   compositor_frame_sink_.reset();
 }
 
 void FrameGenerator::OnGpuChannelEstablished(
     scoped_refptr<gpu::GpuChannelHost> channel) {
   if (widget_ != gfx::kNullAcceleratedWidget) {
     compositor_frame_sink_ = base::MakeUnique<surfaces::CompositorFrameSink>(
@@ skipping 56 matching lines @@
     frame_pending_ = true;
     compositor_frame_sink_->SubmitCompositorFrame(
         std::move(frame),
         base::Bind(&FrameGenerator::DidDraw, weak_factory_.GetWeakPtr()));
   }
   dirty_rect_ = gfx::Rect();
 }
 
 void FrameGenerator::DidDraw() {
   frame_pending_ = false;
-  delegate_->OnCompositorFrameDrawn();
   if (!dirty_rect_.IsEmpty())
     WantToDraw();
 }
 
 cc::CompositorFrame FrameGenerator::GenerateCompositorFrame(
-    const gfx::Rect& output_rect) const {
+    const gfx::Rect& output_rect) {
   const cc::RenderPassId render_pass_id(1, 1);
   std::unique_ptr<cc::RenderPass> render_pass = cc::RenderPass::Create();
   render_pass->SetNew(render_pass_id, output_rect, dirty_rect_,
                       gfx::Transform());
 
   bool may_contain_video = false;
   DrawWindowTree(render_pass.get(), delegate_->GetRootWindow(), gfx::Vector2d(),
                  1.0f, &may_contain_video);
 
   std::unique_ptr<cc::DelegatedFrameData> frame_data(
@@ skipping 23 matching lines @@
   frame.delegated_frame_data = std::move(frame_data);
   frame.metadata.may_contain_video = may_contain_video;
   return frame;
 }
 
 void FrameGenerator::DrawWindowTree(
     cc::RenderPass* pass,
     ServerWindow* window,
     const gfx::Vector2d& parent_to_root_origin_offset,
     float opacity,
-    bool* may_contain_video) const {
+    bool* may_contain_video) {
   if (!window->visible())
     return;
 
   ServerWindowSurface* default_surface =
       window->surface_manager() ? window->surface_manager()->GetDefaultSurface()
                                 : nullptr;
 
   const gfx::Rect absolute_bounds =
       window->bounds() + parent_to_root_origin_offset;
   const ServerWindow::Windows& children = window->children();
@@ skipping 21 matching lines @@
     const gfx::Rect bounds_at_origin(window->bounds().size());
     // TODO(fsamuel): These clipping and visible rects are incorrect. They need
     // to be populated from CompositorFrame structs.
     sqs->SetAll(quad_to_target_transform,
                 bounds_at_origin.size() /* layer_bounds */,
                 bounds_at_origin /* visible_layer_bounds */,
                 bounds_at_origin /* clip_rect */, false /* is_clipped */,
                 combined_opacity, SkXfermode::kSrcOver_Mode,
                 0 /* sorting-context_id */);
     auto* quad = pass->CreateAndAppendDrawQuad<cc::SurfaceDrawQuad>();
+    AddOrUpdateSurfaceReference(default_surface);
     quad->SetAll(sqs, bounds_at_origin /* rect */,
                  gfx::Rect() /* opaque_rect */,
                  bounds_at_origin /* visible_rect */, true /* needs_blending*/,
                  default_surface->GetSurfaceId());
     if (default_surface->may_contain_video())
       *may_contain_video = true;
   }
   if (underlay_surface) {
     const gfx::Rect underlay_absolute_bounds =
         absolute_bounds - window->underlay_offset();
     gfx::Transform quad_to_target_transform;
     quad_to_target_transform.Translate(underlay_absolute_bounds.x(),
                                        underlay_absolute_bounds.y());
     cc::SharedQuadState* sqs = pass->CreateAndAppendSharedQuadState();
     const gfx::Rect bounds_at_origin(
         underlay_surface->last_submitted_frame_size());
     sqs->SetAll(quad_to_target_transform,
                 bounds_at_origin.size() /* layer_bounds */,
                 bounds_at_origin /* visible_layer_bounds */,
                 bounds_at_origin /* clip_rect */, false /* is_clipped */,
                 combined_opacity, SkXfermode::kSrcOver_Mode,
                 0 /* sorting-context_id */);
 
     auto* quad = pass->CreateAndAppendDrawQuad<cc::SurfaceDrawQuad>();
+    AddOrUpdateSurfaceReference(underlay_surface);
     quad->SetAll(sqs, bounds_at_origin /* rect */,
                  gfx::Rect() /* opaque_rect */,
                  bounds_at_origin /* visible_rect */, true /* needs_blending*/,
                  underlay_surface->GetSurfaceId());
     DCHECK(!underlay_surface->may_contain_video());
   }
 }
 
+void FrameGenerator::AddOrUpdateSurfaceReference(
+    ServerWindowSurface* window_surface) {
+  if (!window_surface->has_frame())
+    return;
+  cc::SurfaceId surface_id = window_surface->GetSurfaceId();
+  cc::SurfaceManager* surface_manager = display_compositor_->manager();
+  auto it = dependencies_.find(surface_id.frame_sink_id());
+  if (it == dependencies_.end()) {
+    cc::Surface* surface = surface_manager->GetSurfaceForId(surface_id);
+    if (!surface) {
+      LOG(ERROR) << "Attempting to add dependency to nonexistent surface "
+                 << surface_id.ToString();
+      return;
+    }
+    SurfaceDependency dependency = {
+        surface_id.local_frame_id(),
+        surface_sequence_generator_.CreateSurfaceSequence()};
+    surface->AddDestructionDependency(dependency.sequence);

[rjkroege, 2016/10/07 19:34:50]

I think that this logic could be refactored in the future

[Fady Samuel, 2016/10/07 20:31:33]

This is going away once kylechar@ finishes his surface refcounting work.

[rjkroege, 2016/10/07 20:48:13]

can you add a TODO before landing?

[Fady Samuel, 2016/10/07 21:04:32]

Done.

+    dependencies_[surface_id.frame_sink_id()] = dependency;
+    // Observe |window_surface|'s window so that we can release references when
+    // the window is destroyed.
+    window_surface->window()->AddObserver(this);
+    return;
+  }
+
+  // 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)
+    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());
+
+  AddOrUpdateSurfaceReference(window_surface);

[rjkroege, 2016/10/07 19:34:50]

What is going on with this recursion? Is this ever reached? It's not clear to me
if it will always stop.

[Fady Samuel, 2016/10/07 20:31:33]

It will. The reason we got here is because we already had a stale dependency. We
released the reference in the line above so it has been cleared from the
dependencies_ map and so we'll enter the if statement above.

[rjkroege, 2016/10/07 20:48:13]

Please insert this something like this as a comment before landing.

[Fady Samuel, 2016/10/07 21:04:32]

Done.

+}
+
+void FrameGenerator::ReleaseFrameSinkReference(
+    const cc::FrameSinkId& frame_sink_id) {
+  auto it = dependencies_.find(frame_sink_id);
+  if (it == dependencies_.end())
+    return;
+  std::vector<uint32_t> sequences;
+  sequences.push_back(it->second.sequence.sequence);
+  cc::SurfaceManager* surface_manager = display_compositor_->manager();
+  surface_manager->DidSatisfySequences(frame_sink_id_, &sequences);
+  dependencies_.erase(it);
+}
+
+void FrameGenerator::ReleaseAllSurfaceReferences() {
+  cc::SurfaceManager* surface_manager = display_compositor_->manager();
+  std::vector<uint32_t> sequences;
+  for (auto& dependency : dependencies_)
+    sequences.push_back(dependency.second.sequence.sequence);
+  surface_manager->DidSatisfySequences(frame_sink_id_, &sequences);
+  dependencies_.clear();
+}
+
+void FrameGenerator::OnWindowDestroying(ServerWindow* window) {
+  window->RemoveObserver(this);
+  ServerWindowSurfaceManager* surface_manager = window->surface_manager();
+  // If FrameGenerator was observing |window|, then that means it had a surface
+  // at some point in time and should have a ServerWindowSurfaceManager.
+  DCHECK(surface_manager);
+  ServerWindowSurface* default_surface = surface_manager->GetDefaultSurface();
+  if (default_surface)
+    ReleaseFrameSinkReference(default_surface->frame_sink_id());
+  ServerWindowSurface* underlay_surface = surface_manager->GetUnderlaySurface();
+  if (underlay_surface)
+    ReleaseFrameSinkReference(underlay_surface->frame_sink_id());
+}
+
 }  // namespace ws
 
 }  // namespace ui