Allow layers to signal that additional sequences are needed before surface destruction

content/browser/compositor/delegated_frame_host.cc

 // Copyright 2014 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 "content/browser/compositor/delegated_frame_host.h"
 
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/compositor_frame_ack.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/resources/single_release_callback.h"
 #include "cc/resources/texture_mailbox.h"
 #include "cc/surfaces/surface_factory.h"
+#include "cc/surfaces/surface_manager.h"
 #include "content/browser/compositor/resize_lock.h"
 #include "content/browser/gpu/compositor_util.h"
 #include "content/common/gpu/client/gl_helper.h"
 #include "content/public/browser/render_widget_host_view_frame_subscriber.h"
 #include "content/public/common/content_switches.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
 #include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/effects/SkLumaColorFilter.h"
 #include "ui/gfx/frame_time.h"
 
 namespace content {
 
+namespace {
+
+void SatisfyCallback(cc::SurfaceManager* manager,
+                     cc::SurfaceSequence sequence) {
+  std::vector<uint32_t> sequences;
+  sequences.push_back(sequence.sequence);
+  manager->DidSatisfySequences(sequence.id_namespace, &sequences);
+}
+
+}  // namespace
+
 ////////////////////////////////////////////////////////////////////////////////
 // DelegatedFrameHostClient
 
 bool DelegatedFrameHostClient::ShouldCreateResizeLock() {
   // On Windows and Linux, holding pointer moves will not help throttling
   // resizes.
   // TODO(piman): on Windows we need to block (nested message loop?) the
   // WM_SIZE event. On Linux we need to throttle at the WM level using
   // _NET_WM_SYNC_REQUEST.
   // TODO(ccameron): Mac browser window resizing is incompletely implemented.
@@ skipping 324 matching lines @@
       resource_collection_ = NULL;
     }
     last_output_surface_id_ = output_surface_id;
   }
   ui::Compositor* compositor = client_->GetCompositor();
   if (frame_size.IsEmpty()) {
     DCHECK(frame_data->resource_list.empty());
     EvictDelegatedFrame();
   } else {
     if (use_surfaces_) {
+      ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
+      cc::SurfaceManager* manager = factory->GetSurfaceManager();
       if (!surface_factory_) {
-        ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
-        cc::SurfaceManager* manager = factory->GetSurfaceManager();
         id_allocator_ =
             factory->GetContextFactory()->CreateSurfaceIdAllocator();
         surface_factory_ =
             make_scoped_ptr(new cc::SurfaceFactory(manager, this));
       }
       if (surface_id_.is_null() || frame_size != current_surface_size_ ||
           frame_size_in_dip != current_frame_size_in_dip_) {
         if (!surface_id_.is_null()) {
-          if (compositor) {
-            std::set<cc::SurfaceSequence> seq;
-            seq.insert(compositor->InsertSurfaceSequenceForNextFrame());
-            // Destruction of this surface needs to wait for compositors that
-            // have drawn using it to swap frames that don't reference it.
-            // TODO(jbauman): Handle cases where the compositor has been
-            // changed since the last draw.
-            surface_factory_->DestroyOnSequence(surface_id_, seq);
-          } else {
-            surface_factory_->Destroy(surface_id_);
-          }
+          std::set<cc::SurfaceSequence> seq;
+          surface_holder_->GetSequenceSet(&seq);
+
+          // Destruction of this surface needs to wait for compositors that
+          // have drawn using it to swap frames that don't reference it.
+          surface_factory_->DestroyOnSequence(surface_id_, seq);
         }
         surface_id_ = id_allocator_->GenerateId();
+        surface_holder_ = make_scoped_refptr(new cc::SurfaceHolder(

[piman, 2014/11/03 23:49:26]

nit: no need for make_scoped_refptr

+            surface_id_,
+            base::Bind(&SatisfyCallback, base::Unretained(manager))));

[piman, 2014/11/03 23:49:26]

Why is Unretained safe? Can you add comments? In particular, surface_holder_ is
refcounted, so it's really hard to reason about lifetime (e.g. something posting
a task keeping the surface_holder_ alive for longer than you would expect).

         surface_factory_->Create(surface_id_, frame_size);
-        client_->GetLayer()->SetShowSurface(surface_id_, frame_size_in_dip);
+        client_->GetLayer()->SetShowSurface(surface_holder_, frame_size_in_dip);
         current_surface_size_ = frame_size;
       }
       scoped_ptr<cc::CompositorFrame> compositor_frame =
           make_scoped_ptr(new cc::CompositorFrame());
       compositor_frame->delegated_frame_data = frame_data.Pass();
 
       compositor_frame->metadata.latency_info.swap(skipped_latency_info_list_);
       compositor_frame->metadata.latency_info.insert(
           compositor_frame->metadata.latency_info.end(),
           latency_info.begin(),
@@ skipping 113 matching lines @@
             resources.end(),
             std::back_inserter(surface_returned_resources_));
   if (!pending_delegated_ack_count_)
     SendReturnedDelegatedResources(last_output_surface_id_);
 }
 
 void DelegatedFrameHost::EvictDelegatedFrame() {
   client_->GetLayer()->SetShowSolidColorContent();
   frame_provider_ = NULL;
   if (!surface_id_.is_null()) {
-    surface_factory_->Destroy(surface_id_);
+    std::set<cc::SurfaceSequence> seq;
+    surface_holder_->GetSequenceSet(&seq);
+    surface_factory_->DestroyOnSequence(surface_id_, seq);
+    surface_holder_ = nullptr;
     surface_id_ = cc::SurfaceId();
   }
   delegated_frame_evictor_->DiscardedFrame();
 }
 
 // static
 void DelegatedFrameHost::CopyFromCompositingSurfaceHasResult(
     const gfx::Size& dst_size_in_pixel,
     const SkColorType color_type,
     const base::Callback<void(bool, const SkBitmap&)>& callback,
@@ skipping 371 matching lines @@
 
   host->ScheduleComposite();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // DelegatedFrameHost, private:
 
 DelegatedFrameHost::~DelegatedFrameHost() {
   ImageTransportFactory::GetInstance()->RemoveObserver(this);
 
-  if (!surface_id_.is_null())
-    surface_factory_->Destroy(surface_id_);
+  if (!surface_id_.is_null()) {
+    std::set<cc::SurfaceSequence> seq;
+    surface_holder_->GetSequenceSet(&seq);
+    surface_factory_->DestroyOnSequence(surface_id_, seq);
+    surface_holder_ = nullptr;
+  }
   if (resource_collection_.get())
     resource_collection_->SetClient(NULL);
 
   DCHECK(!vsync_manager_.get());
 }
 
 void DelegatedFrameHost::RunOnCommitCallbacks() {
   for (std::vector<base::Closure>::const_iterator
       it = on_compositing_did_commit_callbacks_.begin();
       it != on_compositing_did_commit_callbacks_.end(); ++it) {
@@ skipping 54 matching lines @@
 void DelegatedFrameHost::OnLayerRecreated(ui::Layer* old_layer,
                                                 ui::Layer* new_layer) {
   // The new_layer is the one that will be used by our Window, so that's the one
   // that should keep our frame. old_layer will be returned to the
   // RecreateLayer caller, and should have a copy.
   if (frame_provider_.get()) {
     new_layer->SetShowDelegatedContent(frame_provider_.get(),
                                        current_frame_size_in_dip_);
   }
   if (!surface_id_.is_null()) {
-    new_layer->SetShowSurface(surface_id_, current_frame_size_in_dip_);
+    new_layer->SetShowSurface(surface_holder_, current_frame_size_in_dip_);
   }
 }
 
 }  // namespace content