Revert of Remove SurfaceFactory::Create and SurfaceFactory::Destroy (…

…patchset chromium#40 id:880001 of https://codereview.chromium.org/2485473003/ )

Reason for revert:
Seems to have broken windows compile:
https://build.chromium.org/p/chromium/builders/Win/builds/49156

Original issue's description:
> 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
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
>
> Committed: https://crrev.com/b902fd366ba91757dbee99d5f821f00d2de1d181
> Cr-Commit-Position: refs/heads/master@{#432312}

TBR=reveman@chromium.org,boliu@chromium.org,danakj@chromium.org,dtrainor@chromium.org,fsamuel@chromium.org,jbauman@chromium.org,piman@chromium.org,sky@chromium.org,samans@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=658607

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

android_webview/browser/hardware_renderer.cc

@@ -167,6 +167,7 @@ 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_));
 }
 
@@ -176,8 +177,9 @@ 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_->EvictSurface();
+  surface_factory_->Destroy(child_id_);
   child_id_ = cc::LocalFrameId();
 }
 

android_webview/browser/surfaces_instance.cc

@@ -85,7 +85,8 @@ SurfacesInstance::~SurfacesInstance() {
   g_surfaces_instance = nullptr;
 
   DCHECK(child_ids_.empty());
-  surface_factory_->EvictSurface();
+  if (root_id_.is_valid())
+    surface_factory_->Destroy(root_id_);
 
   surface_manager_->InvalidateFrameSinkId(frame_sink_id_);
 }
@@ -137,6 +138,7 @@ 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,7 +192,8 @@ void BlimpCompositor::RequestCopyOfOutput(
     host_->SetNeedsCommit();
   } else if (local_frame_id_.is_valid()) {
     // Make a copy request for the surface directly.
-    surface_factory_->RequestCopyOfSurface(std::move(copy_request));
+    surface_factory_->RequestCopyOfSurface(local_frame_id_,
+                                           std::move(copy_request));
   }
 }
 
@@ -324,6 +325,7 @@ 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.
@@ -348,7 +350,8 @@ void BlimpCompositor::SubmitCompositorFrame(cc::CompositorFrame frame) {
                  weak_ptr_factory_.GetWeakPtr()));
 
   for (auto& copy_request : copy_requests_for_next_swap_) {
-    surface_factory_->RequestCopyOfSurface(std::move(copy_request));
+    surface_factory_->RequestCopyOfSurface(local_frame_id_,
+                                           std::move(copy_request));
   }
   copy_requests_for_next_swap_.clear();
 }
@@ -420,7 +423,7 @@ void BlimpCompositor::DestroyDelegatedContent() {
   // Remove any references for the surface layer that uses this
   // |local_frame_id_|.
   layer_->RemoveAllChildren();
-  surface_factory_->EvictSurface();
+  surface_factory_->Destroy(local_frame_id_);
   local_frame_id_ = cc::LocalFrameId();
 }
 

cc/surfaces/direct_compositor_frame_sink.cc

@@ -82,15 +82,20 @@ 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_);
-  factory_.EvictSurface();
+  if (delegated_local_frame_id_.is_valid())
+    factory_.Destroy(delegated_local_frame_id_);
 
   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_,

cc/surfaces/display_unittest.cc

@@ -106,7 +106,6 @@ class DisplayTest : public testing::Test {
 
   ~DisplayTest() override {
     manager_.InvalidateFrameSinkId(kArbitraryFrameSinkId);
-    factory_.EvictSurface();
   }
 
   void SetUpDisplay(const RendererSettings& settings,
@@ -197,6 +196,8 @@ 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();
@@ -413,6 +414,8 @@ 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 {
@@ -439,6 +442,7 @@ 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;
@@ -484,6 +488,8 @@ 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,6 +21,7 @@
 namespace cc {
 namespace {
 
+static constexpr FrameSinkId kArbitraryFrameSinkId(1, 1);
 static const base::UnguessableToken kArbitraryToken =
     base::UnguessableToken::Create();
 
@@ -32,7 +33,8 @@ class EmptySurfaceFactoryClient : public SurfaceFactoryClient {
 
 class SurfaceAggregatorPerfTest : public testing::Test {
  public:
-  SurfaceAggregatorPerfTest() {
+  SurfaceAggregatorPerfTest()
+      : factory_(kArbitraryFrameSinkId, &manager_, &empty_client_) {
     context_provider_ = TestContextProvider::Create();
     context_provider_->BindToCurrentThread();
     shared_bitmap_manager_.reset(new TestSharedBitmapManager);
@@ -47,14 +49,11 @@ class SurfaceAggregatorPerfTest : public testing::Test {
                bool optimize_damage,
                bool full_damage,
                const std::string& name) {
-    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 = 0; i < num_surfaces; i++) {
-      LocalFrameId local_frame_id(i + 1, kArbitraryToken);
+    for (int i = 1; i <= num_surfaces; i++) {
+      LocalFrameId local_frame_id(i, kArbitraryToken);
+      factory_.Create(local_frame_id);
       std::unique_ptr<RenderPass> pass(RenderPass::Create());
       CompositorFrame frame;
 
@@ -87,21 +86,20 @@ 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(FrameSinkId(1, i), LocalFrameId(i, kArbitraryToken)));
+        surface_quad->SetNew(sqs, gfx::Rect(0, 0, 1, 1), gfx::Rect(0, 0, 1, 1),
+                             SurfaceId(kArbitraryFrameSinkId,
+                                       LocalFrameId(i - 1, kArbitraryToken)));
       }
 
       frame.render_pass_list.push_back(std::move(pass));
-      child_factories[i]->SubmitCompositorFrame(
-          local_frame_id, std::move(frame), SurfaceFactory::DrawCallback());
+      factory_.SubmitCompositorFrame(local_frame_id, std::move(frame),
+                                     SurfaceFactory::DrawCallback());
     }
 
-    SurfaceFactory root_factory(FrameSinkId(1, num_surfaces + 1), &manager_,
-                                &empty_client_);
+    factory_.Create(LocalFrameId(num_surfaces + 1, kArbitraryToken));
     timer_.Reset();
     do {
       std::unique_ptr<RenderPass> pass(RenderPass::Create());
@@ -112,7 +110,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(FrameSinkId(1, num_surfaces),
+          SurfaceId(kArbitraryFrameSinkId,
                     LocalFrameId(num_surfaces, kArbitraryToken)));
 
       if (full_damage)
@@ -121,27 +119,28 @@ class SurfaceAggregatorPerfTest : public testing::Test {
         pass->damage_rect = gfx::Rect(0, 0, 1, 1);
 
       frame.render_pass_list.push_back(std::move(pass));
-
-      root_factory.SubmitCompositorFrame(
+      factory_.SubmitCompositorFrame(
           LocalFrameId(num_surfaces + 1, kArbitraryToken), std::move(frame),
           SurfaceFactory::DrawCallback());
 
       CompositorFrame aggregated = aggregator_->Aggregate(
-          SurfaceId(FrameSinkId(1, num_surfaces + 1),
+          SurfaceId(kArbitraryFrameSinkId,
                     LocalFrameId(num_surfaces + 1, kArbitraryToken)));
       timer_.NextLap();
     } while (!timer_.HasTimeLimitExpired());
 
     perf_test::PrintResult("aggregator_speed", "", name, timer_.LapsPerSecond(),
                            "runs/s", true);
-    for (int i = 0; i < num_surfaces; i++)
-      child_factories[i]->EvictSurface();
-    root_factory.EvictSurface();
+
+    factory_.Destroy(LocalFrameId(num_surfaces + 1, kArbitraryToken));
+    for (int i = 1; i <= num_surfaces; i++)
+      factory_.Destroy(LocalFrameId(i, kArbitraryToken));
   }
 
  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_;