Remove SurfaceFactory::Create and SurfaceFactory::Destroy

It is no longer possible to explicitly construct and destroy surfaces. A
SurfaceFactory instance now handles only one surface at a time. Once the
local frame id passed to SubmitCompositorFrame changes, the factory gets
rid of the old surface and creates a new one.

BUG=658607

Committed: https://crrev.com/b902fd366ba91757dbee99d5f821f00d2de1d181
Cr-Commit-Position: refs/heads/master@{#432312}
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2485473003
Cr-Commit-Position: refs/heads/master@{#434724}

android_webview/browser/hardware_renderer.cc

@@ -167,7 +167,6 @@ void HardwareRenderer::DrawGL(AwDrawGLInfo* draw_info) {
 void HardwareRenderer::AllocateSurface() {
   DCHECK(!child_id_.is_valid());
   child_id_ = surface_id_allocator_->GenerateId();
-  surface_factory_->Create(child_id_);
   surfaces_->AddChildId(cc::SurfaceId(frame_sink_id_, child_id_));
 }
 
@@ -177,9 +176,8 @@ void HardwareRenderer::DestroySurface() {
   // Submit an empty frame to force any existing resources to be returned.
   surface_factory_->SubmitCompositorFrame(child_id_, cc::CompositorFrame(),
                                           cc::SurfaceFactory::DrawCallback());
-
   surfaces_->RemoveChildId(cc::SurfaceId(frame_sink_id_, child_id_));
-  surface_factory_->Destroy(child_id_);
+  surface_factory_->EvictSurface();
   child_id_ = cc::LocalFrameId();
 }
 

android_webview/browser/surfaces_instance.cc

@@ -85,8 +85,7 @@ SurfacesInstance::~SurfacesInstance() {
   g_surfaces_instance = nullptr;
 
   DCHECK(child_ids_.empty());
-  if (root_id_.is_valid())
-    surface_factory_->Destroy(root_id_);
+  surface_factory_->EvictSurface();
 
   surface_manager_->InvalidateFrameSinkId(frame_sink_id_);
 }
@@ -138,7 +137,6 @@ void SurfacesInstance::DrawAndSwap(const gfx::Size& viewport,
 
   if (!root_id_.is_valid()) {
     root_id_ = surface_id_allocator_->GenerateId();
-    surface_factory_->Create(root_id_);
     display_->SetLocalFrameId(root_id_, 1.f);
   }
   surface_factory_->SubmitCompositorFrame(root_id_, std::move(frame),

blimp/client/core/compositor/blimp_compositor.cc

@@ -192,8 +192,7 @@ void BlimpCompositor::RequestCopyOfOutput(
     host_->SetNeedsCommit();
   } else if (local_frame_id_.is_valid()) {
     // Make a copy request for the surface directly.
-    surface_factory_->RequestCopyOfSurface(local_frame_id_,
-                                           std::move(copy_request));
+    surface_factory_->RequestCopyOfSurface(std::move(copy_request));
   }
 }
 
@@ -325,7 +324,6 @@ void BlimpCompositor::SubmitCompositorFrame(cc::CompositorFrame frame) {
     DCHECK(layer_->children().empty());
 
     local_frame_id_ = surface_id_allocator_->GenerateId();
-    surface_factory_->Create(local_frame_id_);
     current_surface_size_ = surface_size;
 
     // manager must outlive compositors using it.
@@ -350,8 +348,7 @@ void BlimpCompositor::SubmitCompositorFrame(cc::CompositorFrame frame) {
                  weak_ptr_factory_.GetWeakPtr()));
 
   for (auto& copy_request : copy_requests_for_next_swap_) {
-    surface_factory_->RequestCopyOfSurface(local_frame_id_,
-                                           std::move(copy_request));
+    surface_factory_->RequestCopyOfSurface(std::move(copy_request));
   }
   copy_requests_for_next_swap_.clear();
 }
@@ -423,7 +420,7 @@ void BlimpCompositor::DestroyDelegatedContent() {
   // Remove any references for the surface layer that uses this
   // |local_frame_id_|.
   layer_->RemoveAllChildren();
-  surface_factory_->Destroy(local_frame_id_);
+  surface_factory_->EvictSurface();
   local_frame_id_ = cc::LocalFrameId();
 }
 

cc/surfaces/direct_compositor_frame_sink.cc

@@ -82,20 +82,15 @@ void DirectCompositorFrameSink::DetachFromClient() {
   // Unregister the SurfaceFactoryClient here instead of the dtor so that only
   // one client is alive for this namespace at any given time.
   surface_manager_->UnregisterSurfaceFactoryClient(frame_sink_id_);
-  if (delegated_local_frame_id_.is_valid())
-    factory_.Destroy(delegated_local_frame_id_);
+  factory_.EvictSurface();
 
   CompositorFrameSink::DetachFromClient();
 }
 
 void DirectCompositorFrameSink::SubmitCompositorFrame(CompositorFrame frame) {
   gfx::Size frame_size = frame.render_pass_list.back()->output_rect.size();
   if (frame_size.IsEmpty() || frame_size != last_swap_frame_size_) {
-    if (delegated_local_frame_id_.is_valid()) {
-      factory_.Destroy(delegated_local_frame_id_);
-    }
     delegated_local_frame_id_ = surface_id_allocator_.GenerateId();
-    factory_.Create(delegated_local_frame_id_);
     last_swap_frame_size_ = frame_size;
   }
   display_->SetLocalFrameId(delegated_local_frame_id_,
@@ -109,7 +104,7 @@ void DirectCompositorFrameSink::SubmitCompositorFrame(CompositorFrame frame) {
 
 void DirectCompositorFrameSink::ForceReclaimResources() {
   if (delegated_local_frame_id_.is_valid())
-    factory_.ClearSurface(delegated_local_frame_id_);
+    factory_.ClearSurface();
 }
 
 void DirectCompositorFrameSink::ReturnResources(

cc/surfaces/display_unittest.cc

@@ -106,6 +106,7 @@ class DisplayTest : public testing::Test {
 
   ~DisplayTest() override {
     manager_.InvalidateFrameSinkId(kArbitraryFrameSinkId);
+    factory_.EvictSurface();
   }
 
   void SetUpDisplay(const RendererSettings& settings,
@@ -196,8 +197,6 @@ TEST_F(DisplayTest, DisplayDamaged) {
   EXPECT_TRUE(scheduler_->display_resized_);
   EXPECT_FALSE(scheduler_->has_new_root_surface);
 
-  factory_.Create(local_frame_id);
-
   // First draw from surface should have full damage.
   RenderPassList pass_list;
   std::unique_ptr<RenderPass> pass = RenderPass::Create();
@@ -414,8 +413,6 @@ TEST_F(DisplayTest, DisplayDamaged) {
               software_output_device_->damage_rect());
     EXPECT_EQ(0u, output_surface_->last_sent_frame()->latency_info.size());
   }
-
-  factory_.Destroy(local_frame_id);
 }
 
 class MockedContext : public TestWebGraphicsContext3D {
@@ -442,7 +439,6 @@ TEST_F(DisplayTest, Finish) {
   display_->SetLocalFrameId(local_frame_id, 1.f);
 
   display_->Resize(gfx::Size(100, 100));
-  factory_.Create(local_frame_id);
 
   {
     RenderPassList pass_list;
@@ -488,8 +484,6 @@ TEST_F(DisplayTest, Finish) {
   EXPECT_CALL(*context_ptr, shallowFinishCHROMIUM());
   display_->Resize(gfx::Size(250, 250));
   testing::Mock::VerifyAndClearExpectations(context_ptr);
-
-  factory_.Destroy(local_frame_id);
 }
 
 class CountLossDisplayClient : public StubDisplayClient {

cc/surfaces/surface_aggregator_perftest.cc

@@ -21,7 +21,6 @@
 namespace cc {
 namespace {
 
-static constexpr FrameSinkId kArbitraryFrameSinkId(1, 1);
 static const base::UnguessableToken kArbitraryToken =
     base::UnguessableToken::Create();
 
@@ -33,8 +32,7 @@ class EmptySurfaceFactoryClient : public SurfaceFactoryClient {
 
 class SurfaceAggregatorPerfTest : public testing::Test {
  public:
-  SurfaceAggregatorPerfTest()
-      : factory_(kArbitraryFrameSinkId, &manager_, &empty_client_) {
+  SurfaceAggregatorPerfTest() {
     context_provider_ = TestContextProvider::Create();
     context_provider_->BindToCurrentThread();
     shared_bitmap_manager_.reset(new TestSharedBitmapManager);
@@ -49,11 +47,14 @@ class SurfaceAggregatorPerfTest : public testing::Test {
                bool optimize_damage,
                bool full_damage,
                const std::string& name) {
+    std::vector<std::unique_ptr<SurfaceFactory>> child_factories(num_surfaces);
+    for (int i = 0; i < num_surfaces; i++)
+      child_factories[i].reset(
+          new SurfaceFactory(FrameSinkId(1, i + 1), &manager_, &empty_client_));
     aggregator_.reset(new SurfaceAggregator(&manager_, resource_provider_.get(),
                                             optimize_damage));
-    for (int i = 1; i <= num_surfaces; i++) {
-      LocalFrameId local_frame_id(i, kArbitraryToken);
-      factory_.Create(local_frame_id);
+    for (int i = 0; i < num_surfaces; i++) {
+      LocalFrameId local_frame_id(i + 1, kArbitraryToken);
       std::unique_ptr<RenderPass> pass(RenderPass::Create());
       CompositorFrame frame;
 
@@ -86,20 +87,21 @@ class SurfaceAggregatorPerfTest : public testing::Test {
       }
       sqs = pass->CreateAndAppendSharedQuadState();
       sqs->opacity = opacity;
-      if (i > 1) {
+      if (i >= 1) {
         SurfaceDrawQuad* surface_quad =
             pass->CreateAndAppendDrawQuad<SurfaceDrawQuad>();
-        surface_quad->SetNew(sqs, gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1),
-                             SurfaceId(kArbitraryFrameSinkId,
-                                       LocalFrameId(i - 1, kArbitraryToken)));
+        surface_quad->SetNew(
+            sqs, gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1),
+            SurfaceId(FrameSinkId(1, i), LocalFrameId(i, kArbitraryToken)));
       }
 
       frame.render_pass_list.push_back(std::move(pass));
-      factory_.SubmitCompositorFrame(local_frame_id, std::move(frame),
-                                     SurfaceFactory::DrawCallback());
+      child_factories[i]->SubmitCompositorFrame(
+          local_frame_id, std::move(frame), SurfaceFactory::DrawCallback());
     }
 
-    factory_.Create(LocalFrameId(num_surfaces + 1, kArbitraryToken));
+    SurfaceFactory root_factory(FrameSinkId(1, num_surfaces + 1), &manager_,
+                                &empty_client_);
     timer_.Reset();
     do {
       std::unique_ptr<RenderPass> pass(RenderPass::Create());
@@ -110,7 +112,7 @@ class SurfaceAggregatorPerfTest : public testing::Test {
           pass->CreateAndAppendDrawQuad<SurfaceDrawQuad>();
       surface_quad->SetNew(
           sqs, gfx::Rect(0, 0, 100, 100), gfx::Rect(0, 0, 100, 100),
-          SurfaceId(kArbitraryFrameSinkId,
+          SurfaceId(FrameSinkId(1, num_surfaces),
                     LocalFrameId(num_surfaces, kArbitraryToken)));
 
       if (full_damage)
@@ -119,28 +121,27 @@ class SurfaceAggregatorPerfTest : public testing::Test {
         pass->damage_rect = gfx::Rect(0, 0, 1, 1);
 
       frame.render_pass_list.push_back(std::move(pass));
-      factory_.SubmitCompositorFrame(
+
+      root_factory.SubmitCompositorFrame(
           LocalFrameId(num_surfaces + 1, kArbitraryToken), std::move(frame),
           SurfaceFactory::DrawCallback());
 
       CompositorFrame aggregated = aggregator_->Aggregate(
-          SurfaceId(kArbitraryFrameSinkId,
+          SurfaceId(FrameSinkId(1, num_surfaces + 1),
                     LocalFrameId(num_surfaces + 1, kArbitraryToken)));
       timer_.NextLap();
     } while (!timer_.HasTimeLimitExpired());
 
     perf_test::PrintResult("aggregator_speed", "", name, timer_.LapsPerSecond(),
                            "runs/s", true);
-
-    factory_.Destroy(LocalFrameId(num_surfaces + 1, kArbitraryToken));
-    for (int i = 1; i <= num_surfaces; i++)
-      factory_.Destroy(LocalFrameId(i, kArbitraryToken));
+    for (int i = 0; i < num_surfaces; i++)
+      child_factories[i]->EvictSurface();
+    root_factory.EvictSurface();
   }
 
  protected:
   SurfaceManager manager_;
   EmptySurfaceFactoryClient empty_client_;
-  SurfaceFactory factory_;
   scoped_refptr<TestContextProvider> context_provider_;
   std::unique_ptr<SharedBitmapManager> shared_bitmap_manager_;
   std::unique_ptr<ResourceProvider> resource_provider_;