Remove SurfaceFactory::Create and SurfaceFactory::Destroy

cc/surfaces/surface_factory.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 "cc/surfaces/surface_factory.h"
 
 #include <utility>
 
 #include "base/trace_event/trace_event.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/copy_output_request.h"
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_factory_client.h"
 #include "cc/surfaces/surface_manager.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace cc {
 SurfaceFactory::SurfaceFactory(const FrameSinkId& frame_sink_id,
                                SurfaceManager* manager,
                                SurfaceFactoryClient* client)
     : frame_sink_id_(frame_sink_id),
       manager_(manager),
       client_(client),
       holder_(client),
       needs_sync_points_(true),
       weak_factory_(this) {}
 
 SurfaceFactory::~SurfaceFactory() {
-  if (!surface_map_.empty()) {
-    LOG(ERROR) << "SurfaceFactory has " << surface_map_.size()
-               << " entries in map on destruction.";
-  }
   DestroyAll();
 }
 
 void SurfaceFactory::DestroyAll() {

[Fady Samuel, 2016/11/11 19:02:20]

Rename this to EvictFrame and make public.

Btw, it just occurs to me that we probably don't need a surface_map_.

   if (manager_) {
     for (auto& pair : surface_map_)
       manager_->Destroy(std::move(pair.second));
   }
   surface_map_.clear();
+  current_local_frame_id_ = LocalFrameId();
 }
 
 void SurfaceFactory::Reset() {
   DestroyAll();
   // Disown Surfaces that are still alive so that they don't try to unref
   // resources that we're not tracking any more.
   weak_factory_.InvalidateWeakPtrs();
   holder_.Reset();
 }
 
@@ skipping 23 matching lines @@
       manager_->GetSurfaceForId(SurfaceId(frame_sink_id_, old_id));
   if (old_surface) {
     it->second->SetPreviousFrameSurface(old_surface);
   }
 }
 
 void SurfaceFactory::SubmitCompositorFrame(const LocalFrameId& local_frame_id,
                                            CompositorFrame frame,
                                            const DrawCallback& callback) {
   TRACE_EVENT0("cc", "SurfaceFactory::SubmitCompositorFrame");
+  DCHECK(local_frame_id.is_valid());
+  if (!current_local_frame_id_.is_valid() ||
+      local_frame_id != current_local_frame_id_) {
+    Create(local_frame_id);
+  }
   OwningSurfaceMap::iterator it = surface_map_.find(local_frame_id);
   DCHECK(it != surface_map_.end());
   DCHECK(it->second->factory().get() == this);
   // Tell the SurfaceManager if this is the first frame submitted with this
   // LocalFrameId.
   if (!it->second->HasFrame()) {
     float device_scale_factor = frame.metadata.device_scale_factor;
     gfx::Size frame_size;
     // CompositorFrames may not be populated with a RenderPass in unit tests.
     if (!frame.render_pass_list.empty())
       frame_size = frame.render_pass_list[0]->output_rect.size();
     manager_->SurfaceCreated(it->second->surface_id(), frame_size,
                              device_scale_factor);
   }
   it->second->QueueFrame(std::move(frame), callback);
   if (!manager_->SurfaceModified(SurfaceId(frame_sink_id_, local_frame_id))) {
     TRACE_EVENT_INSTANT0("cc", "Damage not visible.", TRACE_EVENT_SCOPE_THREAD);
     it->second->RunDrawCallbacks();
   }
+  if (current_local_frame_id_.is_valid() &&
+      local_frame_id != current_local_frame_id_)

[Fady Samuel, 2016/11/11 19:02:20]

Add braces. Call SetPreviousFrameSurface here before destroy.

+    Destroy(current_local_frame_id_);
+  current_local_frame_id_ = local_frame_id;
 }
 
 void SurfaceFactory::RequestCopyOfSurface(
     const LocalFrameId& local_frame_id,
     std::unique_ptr<CopyOutputRequest> copy_request) {
   OwningSurfaceMap::iterator it = surface_map_.find(local_frame_id);
   if (it == surface_map_.end()) {
     copy_request->SendEmptyResult();
     return;
   }
@@ skipping 14 matching lines @@
 
 void SurfaceFactory::RefResources(const TransferableResourceArray& resources) {
   holder_.RefResources(resources);
 }
 
 void SurfaceFactory::UnrefResources(const ReturnedResourceArray& resources) {
   holder_.UnrefResources(resources);
 }
 
 }  // namespace cc