cc: Only add surface ID to embedded_surfaces if fallback does not match

A child surface ID is either a dependency (something the parents wants
to block its CompositorFrame on and may not exist in the display
compositor yet) or a reference (it is known to exist in the display
compositor).

Dependencies are held in CompositorFrameMetadata::embedded_surfaces, and
references are held in CompositorFrameMetadata::referenced_surfaces.

It was possible for a surface ID to end up in both places if the
primary and fallback SurfaceInfos in a SurfaceLayer matched. This
causes issues in an upcoming CL https://codereview.chromium.org/2831213004/
that "closes" an old fallback surface to receiving new CompositorFrames
if a new primary surface is being requested by a parent to reduce latency.

BUG=672962, 711948
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

cc/layers/surface_layer_impl.cc

@@ -66,12 +66,16 @@ void SurfaceLayerImpl::AppendQuads(RenderPass* render_pass,
                                    AppendQuadsData* append_quads_data) {
   AppendRainbowDebugBorder(render_pass);
   SharedQuadState* common_shared_quad_state = nullptr;
-  auto* primary = CreateSurfaceDrawQuad(
-      render_pass, SurfaceDrawQuadType::PRIMARY, primary_surface_info_,
-      &append_quads_data->embedded_surfaces, &common_shared_quad_state);
+  auto* primary =
+      CreateSurfaceDrawQuad(render_pass, SurfaceDrawQuadType::PRIMARY,
+                            primary_surface_info_, &common_shared_quad_state);
   // Emitting a fallback SurfaceDrawQuad is unnecessary if the primary and
   // fallback surface Ids match.
-  if (primary && fallback_surface_info_.id() != primary_surface_info_.id()) {
+  bool needs_fallback =
+      fallback_surface_info_.id() != primary_surface_info_.id();
+  if (primary && needs_fallback) {
+    // Add the primary surface ID as a dependency.
+    append_quads_data->embedded_surfaces.push_back(primary_surface_info_.id());
     // We can use the same SharedQuadState as the primary SurfaceDrawQuad if
     // we don't need a different transform on the fallback.
     bool use_common_shared_quad_state =
@@ -80,7 +84,6 @@ void SurfaceLayerImpl::AppendQuads(RenderPass* render_pass,
             fallback_surface_info_.device_scale_factor();
     primary->fallback_quad = CreateSurfaceDrawQuad(
         render_pass, SurfaceDrawQuadType::FALLBACK, fallback_surface_info_,
-        nullptr /* embedded_surfaces */,
         use_common_shared_quad_state ? &common_shared_quad_state : nullptr);
   }
 }
@@ -89,7 +92,6 @@ SurfaceDrawQuad* SurfaceLayerImpl::CreateSurfaceDrawQuad(
     RenderPass* render_pass,
     SurfaceDrawQuadType surface_draw_quad_type,
     const SurfaceInfo& surface_info,
-    std::vector<SurfaceId>* embedded_surfaces,
     SharedQuadState** common_shared_quad_state) {
   if (!surface_info.is_valid())
     return nullptr;
@@ -139,8 +141,6 @@ SurfaceDrawQuad* SurfaceLayerImpl::CreateSurfaceDrawQuad(
       render_pass->CreateAndAppendDrawQuad<SurfaceDrawQuad>();
   surface_draw_quad->SetNew(shared_quad_state, quad_rect, visible_quad_rect,
                             surface_info.id(), surface_draw_quad_type, nullptr);
-  if (embedded_surfaces)
-    embedded_surfaces->push_back(surface_info.id());
 
   return surface_draw_quad;
 }

cc/layers/surface_layer_impl.h

@@ -30,6 +30,11 @@ class CC_EXPORT SurfaceLayerImpl : public LayerImpl {
     return primary_surface_info_;
   }
 
+  // A fallback Surface is a Surface that is already known to exist in the
+  // display compositor. If surface synchronization is enabled, the display
+  // compositor will use the fallback if the primary surface is unavailable
+  // at the time of surface aggregation. If surface synchronization is not
+  // enabled, then a fallback surface will not be specified.
   void SetFallbackSurfaceInfo(const SurfaceInfo& surface_info);
   const SurfaceInfo& fallback_surface_info() const {
     return fallback_surface_info_;
@@ -51,7 +56,6 @@ class CC_EXPORT SurfaceLayerImpl : public LayerImpl {
       RenderPass* render_pass,
       SurfaceDrawQuadType surface_draw_quad_type,
       const SurfaceInfo& surface_info,
-      std::vector<SurfaceId>* embedded_surfaces,
       SharedQuadState** common_shared_quad_state);
 
   void GetDebugBorderProperties(SkColor* color, float* width) const override;

cc/layers/surface_layer_impl_unittest.cc

@@ -327,7 +327,11 @@ TEST(SurfaceLayerImplTest,
   std::unique_ptr<RenderPass> render_pass = RenderPass::Create();
   AppendQuadsData data;
   surface_layer_impl->AppendQuads(render_pass.get(), &data);
-  EXPECT_THAT(data.embedded_surfaces, UnorderedElementsAre(surface_id1));
+  // As the primary and fallback SurfaceInfos match, there is no reason to
+  // add the primary surface ID to |embedded_surfaces| because it is not an
+  // unresolved dependency. The fallback surface will already be added as a
+  // reference in referenced_surfaces.
+  EXPECT_THAT(data.embedded_surfaces, testing::IsEmpty());
 
   ASSERT_EQ(1u, render_pass->quad_list.size());
   const SurfaceDrawQuad* surface_draw_quad1 =