Surface Synchronization: Reset deadline on invalid SurfaceId

If a SurfaceLayer is given an invalid SurfaceId then it should not block
on that invalid SurfaceId and the specified deadline should play no part
in the deadline of the CompositorFrame.

Bug: 672962
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I71c31315ff0dbcd0ab9697e98c5f58aa3691bf67
Reviewed-on: https://chromium-review.googlesource.com/1110028Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569406}

cc/layers/surface_layer.cc

@@ -30,8 +30,13 @@ void SurfaceLayer::SetPrimarySurfaceId(const viz::SurfaceId& surface_id,
     return;
   }
   primary_surface_id_ = surface_id;
-  if (!deadline_policy.use_existing_deadline())
+  // We should never block or set a deadline on an invalid
+  // |primary_surface_id_|.
+  if (!primary_surface_id_.is_valid()) {
+    deadline_in_frames_ = 0u;
+  } else if (!deadline_policy.use_existing_deadline()) {
     deadline_in_frames_ = deadline_policy.deadline_in_frames();
+  }
   UpdateDrawsContent(HasDrawableContent());
   SetNeedsCommit();
 }

cc/layers/surface_layer_unittest.cc

@@ -98,6 +98,25 @@ TEST_F(SurfaceLayerTest, UseInfiniteDeadlineForNewSurfaceLayer) {
   EXPECT_EQ(std::numeric_limits<uint32_t>::max(), layer->deadline_in_frames());
 }
 
+// This test verifies that if an invalid primary surface ID is set then the
+// deadline will be reset to 0 frames.
+TEST_F(SurfaceLayerTest, ResetDeadlineOnInvalidSurfaceId) {
+  scoped_refptr<SurfaceLayer> layer = SurfaceLayer::Create();
+  layer_tree_host_->SetRootLayer(layer);
+  viz::SurfaceId primary_id(
+      kArbitraryFrameSinkId,
+      viz::LocalSurfaceId(1, base::UnguessableToken::Create()));
+  layer->SetPrimarySurfaceId(primary_id,
+                             DeadlinePolicy::UseSpecifiedDeadline(3u));
+  EXPECT_EQ(3u, layer->deadline_in_frames());
+
+  // Reset the surface layer to an invalid SurfaceId. Verify that the deadline
+  // is reset.
+  layer->SetPrimarySurfaceId(viz::SurfaceId(),
+                             DeadlinePolicy::UseSpecifiedDeadline(3u));
+  EXPECT_EQ(0u, layer->deadline_in_frames());
+}
+
 // This test verifies that SurfaceLayer properties are pushed across to
 // SurfaceLayerImpl.
 TEST_F(SurfaceLayerTest, PushProperties) {

ui/compositor/layer_unittest.cc

@@ -1892,10 +1892,12 @@ TEST_F(LayerWithDelegateTest, ExternalContent) {
   EXPECT_EQ(before.get(), child->cc_layer_for_testing());
 
   // Showing surface content changes the underlying cc layer.
+  viz::FrameSinkId frame_sink_id(1u, 1u);
+  viz::ParentLocalSurfaceIdAllocator allocator;
   before = child->cc_layer_for_testing();
-  child->SetShowPrimarySurface(viz::SurfaceId(), gfx::Size(10, 10),
-                               SK_ColorWHITE,
-                               cc::DeadlinePolicy::UseDefaultDeadline(), false);
+  child->SetShowPrimarySurface(
+      viz::SurfaceId(frame_sink_id, allocator.GenerateId()), gfx::Size(10, 10),
+      SK_ColorWHITE, cc::DeadlinePolicy::UseDefaultDeadline(), false);
   scoped_refptr<cc::Layer> after = child->cc_layer_for_testing();
   const auto* surface = static_cast<cc::SurfaceLayer*>(after.get());
   EXPECT_TRUE(after.get());
@@ -1903,8 +1905,8 @@ TEST_F(LayerWithDelegateTest, ExternalContent) {
   EXPECT_EQ(base::nullopt, surface->deadline_in_frames());
 
   child->SetShowPrimarySurface(
-      viz::SurfaceId(), gfx::Size(10, 10), SK_ColorWHITE,
-      cc::DeadlinePolicy::UseSpecifiedDeadline(4u), false);
+      viz::SurfaceId(frame_sink_id, allocator.GenerateId()), gfx::Size(10, 10),
+      SK_ColorWHITE, cc::DeadlinePolicy::UseSpecifiedDeadline(4u), false);
   EXPECT_EQ(4u, surface->deadline_in_frames());
 }