Remove SurfaceFactory::Create and SurfaceFactory::Destroy

content/browser/renderer_host/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/renderer_host/delegated_frame_host.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 458 matching lines @@
   pending_delegated_ack_count_++;
 
   background_color_ = frame.metadata.root_background_color;
 
   if (frame_size.IsEmpty()) {
     DCHECK(frame.resource_list.empty());
     EvictDelegatedFrame();
   } else {
     ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
     cc::SurfaceManager* manager = factory->GetSurfaceManager();
+    bool allocated_new_local_frame_id = false;
     if (!local_frame_id_.is_valid() || frame_size != current_surface_size_ ||
         frame_size_in_dip != current_frame_size_in_dip_) {
-      if (local_frame_id_.is_valid())
-        surface_factory_->Destroy(local_frame_id_);
       local_frame_id_ = id_allocator_->GenerateId();
-      surface_factory_->Create(local_frame_id_);
-      // manager must outlive compositors using it.
-      client_->DelegatedFrameHostGetLayer()->SetShowSurface(
-          cc::SurfaceId(frame_sink_id_, local_frame_id_),
-          base::Bind(&SatisfyCallback, base::Unretained(manager)),
-          base::Bind(&RequireCallback, base::Unretained(manager)), frame_size,
-          frame_device_scale_factor, frame_size_in_dip);
-      current_surface_size_ = frame_size;
-      current_scale_factor_ = frame_device_scale_factor;
+      allocated_new_local_frame_id = true;
     }
 
     gfx::Size desired_size = client_->DelegatedFrameHostDesiredSizeInDIP();
     if (desired_size != frame_size_in_dip && !desired_size.IsEmpty()) {
       skip_frame = true;
       skipped_latency_info_list_.insert(skipped_latency_info_list_.end(),
                                         frame.metadata.latency_info.begin(),
                                         frame.metadata.latency_info.end());
       frame.metadata.latency_info.clear();
     } else {
       frame.metadata.latency_info.insert(frame.metadata.latency_info.end(),
                                          skipped_latency_info_list_.begin(),
                                          skipped_latency_info_list_.end());
       skipped_latency_info_list_.clear();
     }
 
     cc::SurfaceFactory::DrawCallback ack_callback;
     if (compositor_ && !skip_frame) {
       ack_callback = base::Bind(&DelegatedFrameHost::SurfaceDrawn, AsWeakPtr(),
                                 compositor_frame_sink_id);
     }
     surface_factory_->SubmitCompositorFrame(local_frame_id_, std::move(frame),
                                             ack_callback);
+    if (allocated_new_local_frame_id) {
+      // manager must outlive compositors using it.
+      client_->DelegatedFrameHostGetLayer()->SetShowSurface(
+          cc::SurfaceId(frame_sink_id_, local_frame_id_),
+          base::Bind(&SatisfyCallback, base::Unretained(manager)),
+          base::Bind(&RequireCallback, base::Unretained(manager)), frame_size,
+          frame_device_scale_factor, frame_size_in_dip);
+      current_surface_size_ = frame_size;
+      current_scale_factor_ = frame_device_scale_factor;
+    }
   }
   released_front_lock_ = NULL;
   current_frame_size_in_dip_ = frame_size_in_dip;
   CheckResizeLock();
 
   UpdateGutters();
 
   if (!damage_rect_in_dip.IsEmpty()) {
     client_->DelegatedFrameHostGetLayer()->OnDelegatedFrameDamage(
         damage_rect_in_dip);
@@ skipping 59 matching lines @@
   AttemptFrameSubscriberCapture(damage_rect);
 }
 
 void DelegatedFrameHost::SetBeginFrameSource(
     cc::BeginFrameSource* begin_frame_source) {
   client_->SetBeginFrameSource(begin_frame_source);
 }
 
 void DelegatedFrameHost::EvictDelegatedFrame() {
   client_->DelegatedFrameHostGetLayer()->SetShowSolidColorContent();
   if (local_frame_id_.is_valid()) {

[jbauman, 2016/11/11 03:23:20]

If you remove the destroy, in what sense is the frame evicted?

[Saman Sami, 2016/11/11 17:49:59]

Does it matter whether or not we get rid of the frame immediately? The tests
seem to be doing fine. I can call Reset on surface factory if we need to. (Or
maybe we should just change the name of this method?)

[Fady Samuel, 2016/11/11 18:12:09]

Maybe SurfaceFactory (which we eventually want to merge with
"CompositorFrameSinkSupport"/GpuCompositorFrameSInk) can have a public
EvictFrame() API that evicts the current frame if there is one.

-    surface_factory_->Destroy(local_frame_id_);
     local_frame_id_ = cc::LocalFrameId();
   }
   delegated_frame_evictor_->DiscardedFrame();
   UpdateGutters();
 }
 
 // static
 void DelegatedFrameHost::ReturnSubscriberTexture(
     base::WeakPtr<DelegatedFrameHost> dfh,
     scoped_refptr<OwnedMailbox> subscriber_texture,
@@ skipping 213 matching lines @@
   yuv_readback_pipeline_.reset();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
 // DelegatedFrameHost, private:
 
 DelegatedFrameHost::~DelegatedFrameHost() {
   DCHECK(!compositor_);
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
   factory->GetContextFactory()->RemoveObserver(this);
-
-  if (local_frame_id_.is_valid())
-    surface_factory_->Destroy(local_frame_id_);
+  surface_factory_->Reset();
   factory->GetSurfaceManager()->UnregisterSurfaceFactoryClient(frame_sink_id_);
   factory->GetSurfaceManager()->InvalidateFrameSinkId(frame_sink_id_);
 
   DCHECK(!vsync_manager_.get());
 }
 
 void DelegatedFrameHost::SetCompositor(ui::Compositor* compositor) {
   DCHECK(!compositor_);
   if (!compositor)
     return;
@@ skipping 38 matching lines @@
         std::move(request));
   }
 }
 
 void DelegatedFrameHost::UnlockResources() {
   DCHECK(local_frame_id_.is_valid());
   delegated_frame_evictor_->UnlockFrame();
 }
 
 }  // namespace content