Fix cc_unittests with surface sync enabled.

By default LayerTreeSettings::enable_surface_synchronization was false.
Most cc_unittests were running with the default. This CL switches the
default to true and fixes most tests.

The primary change for tests is with regards to LocalSurfaceId
allocation. With surface sync off LocalSurfaceIds would be allocated
automatically. With surface sync on the test has to ensure new
LocalSurfaceIds are allocated at the correct time.

Bug: 985009
Change-Id: I9bd62da2b96f9eddb4ea85656c61364c0ee5ada8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1774309
Commit-Queue: kylechar <kylechar@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692220}

cc/layers/texture_layer_unittest.cc

@@ -44,6 +44,7 @@
 #include "components/viz/common/resources/returned_resource.h"
 #include "components/viz/common/resources/shared_bitmap.h"
 #include "components/viz/common/resources/transferable_resource.h"
+#include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "components/viz/service/display/software_output_device.h"
 #include "components/viz/test/fake_output_surface.h"
 #include "gpu/GLES2/gl2extchromium.h"
@@ -268,8 +269,10 @@ TEST_F(TextureLayerTest, ShutdownWithResource) {
           viz::SingleReleaseCallback::Create(test_data_.sw_release_callback_));
     }
 
-    host->SetViewportRectAndScale(gfx::Rect(10, 10), 1.f,
-                                  viz::LocalSurfaceIdAllocation());
+    viz::ParentLocalSurfaceIdAllocator allocator;
+    allocator.GenerateId();
+    host->SetViewportRectAndScale(
+        gfx::Rect(10, 10), 1.f, allocator.GetCurrentLocalSurfaceIdAllocation());
     host->SetVisible(true);
     host->SetRootLayer(layer);
 

cc/test/layer_tree_test.cc

@@ -4,6 +4,9 @@
 
 #include "cc/test/layer_tree_test.h"
 
+#include <memory>
+#include <string>
+
 #include "base/bind.h"
 #include "base/cfi_buildflags.h"
 #include "base/command_line.h"
@@ -824,7 +827,15 @@ void LayerTreeTest::DoBeginTest() {
   beginning_ = true;
   SetupTree();
   WillBeginTest();
+  bool allocate_local_surface_id = !skip_allocate_initial_local_surface_id_ &&
+                                   settings_.enable_surface_synchronization;
+  if (allocate_local_surface_id)
+    GenerateNewLocalSurfaceId();
   BeginTest();
+  if (allocate_local_surface_id) {
+    PostSetLocalSurfaceIdAllocationToMainThread(
+        GetCurrentLocalSurfaceIdAllocation());
+  }
   beginning_ = false;
   if (end_when_begin_returns_)
     RealEndTest();
@@ -837,6 +848,19 @@ void LayerTreeTest::DoBeginTest() {
   }
 }
 
+void LayerTreeTest::SkipAllocateInitialLocalSurfaceId() {
+  skip_allocate_initial_local_surface_id_ = true;
+}
+
+const viz::LocalSurfaceIdAllocation&
+LayerTreeTest::GetCurrentLocalSurfaceIdAllocation() const {
+  return allocator_.GetCurrentLocalSurfaceIdAllocation();
+}
+
+void LayerTreeTest::GenerateNewLocalSurfaceId() {
+  allocator_.GenerateId();
+}
+
 void LayerTreeTest::SetupTree() {
   if (!layer_tree_host()->root_layer()) {
     layer_tree_host()->SetRootLayer(Layer::Create());

cc/test/layer_tree_test.h

@@ -14,6 +14,7 @@
 #include "cc/trees/compositor_mode.h"
 #include "cc/trees/layer_tree_host.h"
 #include "cc/trees/layer_tree_host_impl.h"
+#include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
 #include "components/viz/test/test_gpu_memory_buffer_manager.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -112,6 +113,11 @@ class LayerTreeTest : public testing::Test, public TestHooks {
  protected:
   LayerTreeTest();
 
+  void SkipAllocateInitialLocalSurfaceId();
+  const viz::LocalSurfaceIdAllocation& GetCurrentLocalSurfaceIdAllocation()
+      const;
+  void GenerateNewLocalSurfaceId();
+
   virtual void InitializeSettings(LayerTreeSettings* settings) {}
 
   void RealEndTest();
@@ -258,6 +264,8 @@ class LayerTreeTest : public testing::Test, public TestHooks {
   std::unique_ptr<TestTaskGraphRunner> task_graph_runner_;
   base::CancelableOnceClosure timeout_;
   scoped_refptr<viz::TestContextProvider> compositor_contexts_;
+  bool skip_allocate_initial_local_surface_id_ = false;
+  viz::ParentLocalSurfaceIdAllocator allocator_;
   base::WeakPtr<LayerTreeTest> main_thread_weak_ptr_;
   base::WeakPtrFactory<LayerTreeTest> weak_factory_{this};
 };

cc/trees/layer_tree_host_unittest_copyrequest.cc

@@ -321,7 +321,7 @@ class LayerTreeHostCopyRequestTestLayerDestroyed
 
         // Prevent drawing so we can't make a copy of the impl_destroyed layer.
         layer_tree_host()->SetViewportRectAndScale(
-            gfx::Rect(), 1.f, viz::LocalSurfaceIdAllocation());
+            gfx::Rect(), 1.f, GetCurrentLocalSurfaceIdAllocation());
         break;
       case 2:
         // Flush the message loops and make sure the callbacks run.
@@ -768,7 +768,8 @@ class LayerTreeHostTestAsyncTwoReadbacksWithoutDraw
     if (layer_tree_host()->SourceFrameNumber() == 1) {
       // Allow drawing.
       layer_tree_host()->SetViewportRectAndScale(
-          gfx::Rect(root_->bounds()), 1.f, viz::LocalSurfaceIdAllocation());
+          gfx::Rect(root_->bounds()), 1.f,
+          GetCurrentLocalSurfaceIdAllocation());
 
       AddCopyRequest(copy_layer_.get());
     }
@@ -1154,8 +1155,9 @@ class LayerTreeHostCopyRequestTestDestroyBeforeCopy
                                base::Unretained(this)));
         copy_layer_->RequestCopyOfOutput(std::move(request));
 
+        // Stop drawing.
         layer_tree_host()->SetViewportRectAndScale(
-            gfx::Rect(), 1.f, viz::LocalSurfaceIdAllocation());
+            gfx::Rect(), 1.f, GetCurrentLocalSurfaceIdAllocation());
         break;
       }
       case 2:
@@ -1168,7 +1170,7 @@ class LayerTreeHostCopyRequestTestDestroyBeforeCopy
         // Allow us to draw now.
         layer_tree_host()->SetViewportRectAndScale(
             gfx::Rect(layer_tree_host()->root_layer()->bounds()), 1.f,
-            viz::LocalSurfaceIdAllocation());
+            GetCurrentLocalSurfaceIdAllocation());
         break;
       case 4:
         EXPECT_EQ(1, callback_count_);
@@ -1243,7 +1245,7 @@ class LayerTreeHostCopyRequestTestShutdownBeforeCopy
         copy_layer_->RequestCopyOfOutput(std::move(request));
 
         layer_tree_host()->SetViewportRectAndScale(
-            gfx::Rect(), 1.f, viz::LocalSurfaceIdAllocation());
+            gfx::Rect(), 1.f, GetCurrentLocalSurfaceIdAllocation());
         break;
       }
       case 2:

cc/trees/layer_tree_host_unittest_damage.cc

@@ -107,7 +107,7 @@ class LayerTreeHostDamageTestSetViewportRectAndScale
     switch (layer_tree_host()->SourceFrameNumber()) {
       case 1:
         layer_tree_host()->SetViewportRectAndScale(
-            gfx::Rect(15, 15), 1.f, viz::LocalSurfaceIdAllocation());
+            gfx::Rect(15, 15), 1.f, GetCurrentLocalSurfaceIdAllocation());
         break;
     }
   }

cc/trees/layer_tree_host_unittest_masks.cc

@@ -423,8 +423,10 @@ class LayerTreeTestMaskLayerWithScaling : public LayerTreeTest {
     switch (layer_tree_host()->SourceFrameNumber()) {
       case 1:
         gfx::Size double_root_size(200, 200);
+        GenerateNewLocalSurfaceId();
         layer_tree_host()->SetViewportRectAndScale(
-            gfx::Rect(double_root_size), 2.f, viz::LocalSurfaceIdAllocation());
+            gfx::Rect(double_root_size), 2.f,
+            GetCurrentLocalSurfaceIdAllocation());
         break;
     }
   }

cc/trees/layer_tree_host_unittest_picture.cc

@@ -192,8 +192,9 @@ class LayerTreeHostPictureTestResizeViewportWithGpuRaster
         // Change the picture layer's size along with the viewport, so it will
         // consider picking a new tile size.
         picture_->SetBounds(gfx::Size(768, 1056));
+        GenerateNewLocalSurfaceId();
         layer_tree_host()->SetViewportRectAndScale(
-            gfx::Rect(768, 1056), 1.f, viz::LocalSurfaceIdAllocation());
+            gfx::Rect(768, 1056), 1.f, GetCurrentLocalSurfaceIdAllocation());
         break;
       case 2:
         EndTest();

cc/trees/layer_tree_host_unittest_proxy.cc

@@ -82,6 +82,11 @@ class LayerTreeHostProxyTestSetNeedsAnimate : public LayerTreeHostProxyTest {
   LayerTreeHostProxyTestSetNeedsAnimate& operator=(
       const LayerTreeHostProxyTestSetNeedsAnimate&) = delete;
 
+  void InitializeSettings(LayerTreeSettings* settings) override {
+    // TODO(crbug.com/985009): Fix test with surface sync enabled.
+    settings->enable_surface_synchronization = false;
+  }
+
   void BeginTest() override {
     EXPECT_EQ(ProxyMain::NO_PIPELINE_STAGE,
               GetProxyMain()->max_requested_pipeline_stage());
@@ -155,6 +160,11 @@ class LayerTreeHostProxyTestSetNeedsUpdateLayersWhileAnimating
   LayerTreeHostProxyTestSetNeedsUpdateLayersWhileAnimating& operator=(
       const LayerTreeHostProxyTestSetNeedsUpdateLayersWhileAnimating&) = delete;
 
+  void InitializeSettings(LayerTreeSettings* settings) override {
+    // TODO(crbug.com/985009): Fix test with surface sync enabled.
+    settings->enable_surface_synchronization = false;
+  }
+
   void BeginTest() override { proxy()->SetNeedsAnimate(); }
 
   void WillBeginMainFrame() override {
@@ -199,6 +209,11 @@ class LayerTreeHostProxyTestSetNeedsCommitWhileAnimating
   LayerTreeHostProxyTestSetNeedsCommitWhileAnimating& operator=(
       const LayerTreeHostProxyTestSetNeedsCommitWhileAnimating&) = delete;
 
+  void InitializeSettings(LayerTreeSettings* settings) override {
+    // TODO(crbug.com/985009): Fix test with surface sync enabled.
+    settings->enable_surface_synchronization = false;
+  }
+
   void BeginTest() override { proxy()->SetNeedsAnimate(); }
 
   void WillBeginMainFrame() override {

cc/trees/layer_tree_settings.h

@@ -111,7 +111,7 @@ class CC_EXPORT LayerTreeSettings {
 
   // Indicates that the LayerTreeHost should defer commits unless it has a valid
   // viz::LocalSurfaceId set.
-  bool enable_surface_synchronization = false;
+  bool enable_surface_synchronization = true;
 
   // Indicates the case when a sub-frame gets its own LayerTree because it's
   // rendered in a different process from its ancestor frames.