Always update surface references.

This CL ensures that if the set of referenced surfaces from the active
tree changes then a new CompositorFrame will be submitted. Before this
CL LayerTreeHostImpl didn't look at referenced surfaces in HasDamage()
so if there is no other damage then it will skip submitting a
CompositorFrame.

For this to happen the SurfaceLayer can't be visible, as updating the
fallback SurfaceId will otherwise contribute to the damage rect.

Bug: 854798
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Id3f196bc151d6118e31c51f2bedc62fc773ee65f
Reviewed-on: https://chromium-review.googlesource.com/1114066Reviewed-by: enne <enne@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570177}

cc/trees/layer_tree_host_impl.cc

@@ -929,6 +929,18 @@ bool LayerTreeHostImpl::HasDamage() const {
   if (!viewport_damage_rect_.IsEmpty())
     return true;
 
+  // If the set of referenced surfaces has changed then we must submit a new
+  // CompositorFrame to update surface references.
+  if (last_draw_referenced_surfaces_ != active_tree()->SurfaceLayerIds())
+    return true;
+
+  // If we have a new LocalSurfaceId, we must always submit a CompositorFrame
+  // because the parent is blocking on us.
+  if (last_draw_local_surface_id_ !=
+      child_local_surface_id_allocator_.GetCurrentLocalSurfaceId()) {
+    return true;
+  }
+
   const LayerTreeImpl* active_tree = active_tree_.get();
 
   // If the root render surface has no visible damage, then don't generate a
@@ -941,15 +953,9 @@ bool LayerTreeHostImpl::HasDamage() const {
   bool must_always_swap =
       layer_tree_frame_sink_->capabilities().must_always_swap;
 
-  // If we have a new LocalSurfaceId, we must always submit a CompositorFrame
-  // because the parent is blocking on us.
-  bool local_surface_id_changed =
-      last_draw_local_surface_id_ !=
-      child_local_surface_id_allocator_.GetCurrentLocalSurfaceId();
-
   return root_surface_has_visible_damage ||
          active_tree_->property_trees()->effect_tree.HasCopyRequests() ||
-         must_always_swap || hud_wants_to_draw_ || local_surface_id_changed;
+         must_always_swap || hud_wants_to_draw_;
 }
 
 DrawResult LayerTreeHostImpl::CalculateRenderPasses(FrameData* frame) {
@@ -1912,9 +1918,14 @@ viz::CompositorFrameMetadata LayerTreeHostImpl::MakeCompositorFrameMetadata() {
         IsActivelyScrolling() || mutator_host_->NeedsTickAnimations();
   }
 
-  for (auto& surface_id : active_tree_->SurfaceLayerIds())
+  const base::flat_set<viz::SurfaceId>& referenced_surfaces =
+      active_tree_->SurfaceLayerIds();
+  for (auto& surface_id : referenced_surfaces)
     metadata.referenced_surfaces.push_back(surface_id);
 
+  if (last_draw_referenced_surfaces_ != referenced_surfaces)
+    last_draw_referenced_surfaces_ = referenced_surfaces;
+
   const auto* inner_viewport_scroll_node = InnerViewportScrollNode();
   if (!inner_viewport_scroll_node)
     return metadata;

cc/trees/layer_tree_host_impl.h

@@ -1080,6 +1080,7 @@ class CC_EXPORT LayerTreeHostImpl
   uint32_t next_frame_token_ = 1u;
 
   viz::LocalSurfaceId last_draw_local_surface_id_;
+  base::flat_set<viz::SurfaceId> last_draw_referenced_surfaces_;
   base::Optional<RenderFrameMetadata> last_draw_render_frame_metadata_;
   viz::ChildLocalSurfaceIdAllocator child_local_surface_id_allocator_;
 

cc/trees/layer_tree_host_impl_unittest.cc

@@ -4527,6 +4527,44 @@ TEST_F(LayerTreeHostImplTest, ActivationDependenciesInMetadata) {
   }
 }
 
+// Verify that updating the set of referenced surfaces for the active tree
+// causes a new CompositorFrame to be submitted, even if there is no other
+// damage.
+TEST_F(LayerTreeHostImplTest, SurfaceReferencesChangeCausesDamage) {
+  SetupScrollAndContentsLayers(gfx::Size(100, 100));
+  host_impl_->SetViewportSize(gfx::Size(50, 50));
+  auto* fake_layer_tree_frame_sink =
+      static_cast<FakeLayerTreeFrameSink*>(host_impl_->layer_tree_frame_sink());
+
+  // Submit an initial CompositorFrame with an empty set of referenced surfaces.
+  host_impl_->active_tree()->BuildPropertyTreesForTesting();
+  host_impl_->active_tree()->SetSurfaceLayerIds({});
+  host_impl_->SetFullViewportDamage();
+  DrawFrame();
+
+  {
+    const viz::CompositorFrameMetadata& metadata =
+        fake_layer_tree_frame_sink->last_sent_frame()->metadata;
+    EXPECT_THAT(metadata.referenced_surfaces, testing::IsEmpty());
+  }
+
+  const viz::SurfaceId surface_id = MakeSurfaceId(viz::FrameSinkId(1, 1), 1);
+
+  // Update the set of referenced surfaces to contain |surface_id| but don't
+  // make any other changes that would cause damage. This mimics updating the
+  // SurfaceLayer for an offscreen tab.
+  host_impl_->active_tree()->BuildPropertyTreesForTesting();
+  host_impl_->active_tree()->SetSurfaceLayerIds({surface_id});
+  DrawFrame();
+
+  {
+    const viz::CompositorFrameMetadata& metadata =
+        fake_layer_tree_frame_sink->last_sent_frame()->metadata;
+    EXPECT_THAT(metadata.referenced_surfaces,
+                testing::UnorderedElementsAre(surface_id));
+  }
+}
+
 TEST_F(LayerTreeHostImplTest, CompositorFrameMetadata) {
   SetupScrollAndContentsLayers(gfx::Size(100, 100));
   host_impl_->SetViewportSize(gfx::Size(50, 50));

testing/buildbot/filters/viz.browser_tests.filter

 #### HostFrameSinkManager::CreateCompositorFrameSinkSupport crash.
 # http://crbug.com/807465
 -ArcAccessibilityHelperBridgeBrowserTest.PreferenceChange
-
-# Timing out in extensions::ResultCatcher::GetNextResult().
-# https://crbug.com/854798
--TabCaptureApiPixelTest.OffscreenTabEndToEnd
--TabCaptureApiPixelTest.OffscreenTabEvilTests