Surface Synchronization: Throttle child synchronization events

This CL enables throttling of child synchronization events. In a child-
initiated synchronization, the child's frame will likely activate
before the dependent parent's CompositorFrame arrives. Upon activation,
the child will see that the new surface does not cause any damage to the
display (because it's not embedded by any parent yet) and calls the
draw callback immediately, thereby sending back a DidReceiveCompositorFrameAck
to the client immediately.

This, in turn, results in the client sending another CompositorFrame to Viz,
and another and so on, letting the child get further and further ahead of the
parent. This results in a lot of wasted work because the child's surface will
not be shown on screen until the parent embeds it and so multiple child
initiated synchronization events are pointless.

This CL throttles DidReceiveCompositorFrameAck of the child on child-initiated
synchronization events until the parent references the child.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Change-Id: I62416d87f8d871b32f4caad565cff515a9c55445
Bug: 672962, 879207
Reviewed-on: https://chromium-review.googlesource.com/1236931
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Saman Sami <samans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596300}

components/viz/service/frame_sinks/compositor_frame_sink_support.cc

@@ -392,6 +392,14 @@ SubmitResult CompositorFrameSinkSupport::MaybeSubmitCompositorFrameInternal(
     // to determine the freshness of a surface at aggregation time.
     const LocalSurfaceId& last_created_local_surface_id =
         last_created_surface_id_.local_surface_id();
+    bool last_surface_has_dependent_frame =
+        prev_surface && prev_surface->HasDependentFrame();
+
+    bool child_initiated_synchronization_event =
+        last_created_local_surface_id.is_valid() &&
+        local_surface_id.child_sequence_number() >
+            last_created_local_surface_id.child_sequence_number();
+
     // Neither sequence numbers of the LocalSurfaceId can decrease and at least
     // one must increase.
     bool monotonically_increasing_id =
@@ -401,8 +409,7 @@ SubmitResult CompositorFrameSinkSupport::MaybeSubmitCompositorFrameInternal(
              last_created_local_surface_id.child_sequence_number()) &&
         (local_surface_id.parent_sequence_number() >
              last_created_local_surface_id.parent_sequence_number() ||
-         local_surface_id.child_sequence_number() >
-             last_created_local_surface_id.child_sequence_number());
+         child_initiated_synchronization_event);
 
     if (!surface_info.is_valid() || !monotonically_increasing_id) {
       TRACE_EVENT_INSTANT0("viz", "Surface Invariants Violation",
@@ -410,7 +417,15 @@ SubmitResult CompositorFrameSinkSupport::MaybeSubmitCompositorFrameInternal(
       return SubmitResult::SURFACE_INVARIANTS_VIOLATION;
     }
 
-    current_surface = CreateSurface(surface_info);
+    // If the last Surface doesn't have a dependent frame, and this frame
+    // corresponds to a child-initiated synchronization event then defer this
+    // Surface until a dependent frame arrives. This throttles child submission
+    // of CompositorFrames to the parent's embedding rate.
+    const bool block_activation_on_parent =
+        child_initiated_synchronization_event &&
+        !last_surface_has_dependent_frame;
+
+    current_surface = CreateSurface(surface_info, block_activation_on_parent);
     last_created_surface_id_ = SurfaceId(frame_sink_id_, local_surface_id);
     surface_manager_->SurfaceDamageExpected(current_surface->surface_id(),
                                             last_begin_frame_args_);
@@ -564,10 +579,12 @@ void CompositorFrameSinkSupport::UpdateNeedsBeginFramesInternal() {
 }
 
 Surface* CompositorFrameSinkSupport::CreateSurface(
-    const SurfaceInfo& surface_info) {
+    const SurfaceInfo& surface_info,
+    bool block_activation_on_parent) {
   return surface_manager_->CreateSurface(
       weak_factory_.GetWeakPtr(), surface_info,
-      frame_sink_manager_->GetPrimaryBeginFrameSource(), needs_sync_tokens_);
+      frame_sink_manager_->GetPrimaryBeginFrameSource(), needs_sync_tokens_,
+      block_activation_on_parent);
 }
 
 SubmitResult CompositorFrameSinkSupport::MaybeSubmitCompositorFrame(

components/viz/service/frame_sinks/compositor_frame_sink_support.h

@@ -197,7 +197,8 @@ class VIZ_SERVICE_EXPORT CompositorFrameSinkSupport
   bool WantsAnimateOnlyBeginFrames() const override;
 
   void UpdateNeedsBeginFramesInternal();
-  Surface* CreateSurface(const SurfaceInfo& surface_info);
+  Surface* CreateSurface(const SurfaceInfo& surface_info,
+                         bool block_activation_on_parent);
 
   // For the sync API calls, if we are blocking a client callback, runs it once
   // BeginFrame and FrameAck are done.

components/viz/service/frame_sinks/compositor_frame_sink_support_unittest.cc

@@ -554,26 +554,64 @@ TEST_F(CompositorFrameSinkSupportTest, MonotonicallyIncreasingLocalSurfaceIds) {
   LocalSurfaceId local_surface_id4(5, 3, kArbitraryToken);
   LocalSurfaceId local_surface_id5(8, 1, kArbitraryToken);
   LocalSurfaceId local_surface_id6(9, 3, kArbitraryToken);
+
+  // LocalSurfaceId1(6, 1)
   auto result = support->MaybeSubmitCompositorFrame(
       local_surface_id1, MakeDefaultCompositorFrame(), base::nullopt, 0,
       mojom::CompositorFrameSink::SubmitCompositorFrameSyncCallback());
   EXPECT_EQ(SubmitResult::ACCEPTED, result);
+
+  // LocalSurfaceId(6, 2): Child-initiated synchronization.
   result = support->MaybeSubmitCompositorFrame(
       local_surface_id2, MakeDefaultCompositorFrame(), base::nullopt, 0,
       mojom::CompositorFrameSink::SubmitCompositorFrameSyncCallback());
   EXPECT_EQ(SubmitResult::ACCEPTED, result);
+
+  // Since the Surface corresponding to |local_surface_id1| was not a dependency
+  // anywhere then the Surface corresponding to |local_surface_id2| will not
+  // activate until it becomes a dependency.
+  Surface* last_created_surface = support->GetLastCreatedSurfaceForTesting();
+  EXPECT_EQ(local_surface_id2,
+            last_created_surface->surface_id().local_surface_id());
+  EXPECT_FALSE(last_created_surface->HasActiveFrame());
+
+  SurfaceId surface_id2(kAnotherArbitraryFrameSinkId, local_surface_id2);
+  auto frame =
+      CompositorFrameBuilder()
+          .AddDefaultRenderPass()
+          .SetActivationDependencies({surface_id2})
+          .SetReferencedSurfaces({SurfaceRange(base::nullopt, surface_id2)})
+          .Build();
+  result = support_->MaybeSubmitCompositorFrame(
+      local_surface_id_, std::move(frame), base::nullopt, 0,
+      mojom::CompositorFrameSink::SubmitCompositorFrameSyncCallback());
+  EXPECT_EQ(SubmitResult::ACCEPTED, result);
+
+  // Submitting a CompositorFrame to the parent FrameSink with a dependency on
+  // |local_surface_id2| causes that Surface's CompositorFrame to activate.
+  EXPECT_TRUE(last_created_surface->HasActiveFrame());
+
+  // LocalSurfaceId(7, 2): Parent-initiated synchronization.
   result = support->MaybeSubmitCompositorFrame(
       local_surface_id3, MakeDefaultCompositorFrame(), base::nullopt, 0,
       mojom::CompositorFrameSink::SubmitCompositorFrameSyncCallback());
   EXPECT_EQ(SubmitResult::ACCEPTED, result);
+
+  // LocalSurfaceId(5, 3): Surface Invariants Violation. Not monotonically
+  // increasing.
   result = support->MaybeSubmitCompositorFrame(
       local_surface_id4, MakeDefaultCompositorFrame(), base::nullopt, 0,
       mojom::CompositorFrameSink::SubmitCompositorFrameSyncCallback());
   EXPECT_EQ(SubmitResult::SURFACE_INVARIANTS_VIOLATION, result);
+
+  // LocalSurfaceId(8, 1): Surface Invariants Violation. Not monotonically
+  // increasing.
   result = support->MaybeSubmitCompositorFrame(
       local_surface_id5, MakeDefaultCompositorFrame(), base::nullopt, 0,
       mojom::CompositorFrameSink::SubmitCompositorFrameSyncCallback());
   EXPECT_EQ(SubmitResult::SURFACE_INVARIANTS_VIOLATION, result);
+
+  // LocalSurfaceId(9, 3): Parent AND child-initiated synchronization.
   result = support->MaybeSubmitCompositorFrame(
       local_surface_id6, MakeDefaultCompositorFrame(), base::nullopt, 0,
       mojom::CompositorFrameSink::SubmitCompositorFrameSyncCallback());

components/viz/service/frame_sinks/surface_synchronization_unittest.cc

@@ -2166,9 +2166,19 @@ TEST_F(SurfaceSynchronizationTest, LatestInFlightSurface) {
   // Verify that there is a temporary reference for child_id3.
   EXPECT_TRUE(HasTemporaryReference(child_id3));
 
+  // The surface corresponding to |child_id3| will not be activated until
+  // a parent embeds it because it's a child initiated synchronization.
+  EXPECT_EQ(GetSurfaceForId(child_id2),
+            GetLatestInFlightSurface(SurfaceRange(child_id1, child_id4)));
+
+  parent_support().SubmitCompositorFrame(
+      parent_id.local_surface_id(),
+      MakeCompositorFrame({child_id3}, {SurfaceRange(base::nullopt, child_id3)},
+                          std::vector<TransferableResource>()));
+
   EXPECT_EQ(GetSurfaceForId(child_id3),
             GetLatestInFlightSurface(SurfaceRange(child_id1, child_id4)));
-  EXPECT_THAT(GetChildReferences(parent_id), UnorderedElementsAre(child_id1));
+  EXPECT_THAT(GetChildReferences(parent_id), UnorderedElementsAre(child_id3));
 
   // If the primary surface is active, we return it.
   EXPECT_EQ(GetSurfaceForId(child_id3),
@@ -2780,5 +2790,204 @@ TEST_F(SurfaceSynchronizationTest,
   EXPECT_EQ(parent_id, parent_support().last_activated_surface_id());
 }
 
+// If a parent CompositorFrame embeds a child Surface newer than the throttled
+// child then the throttled child surface is immediately unthrottled. This
+// unblocks the child to make progress to catch up with the parent.
+TEST_F(SurfaceSynchronizationTest,
+       ParentEmbeddingFutureChildUnblocksCurrentChildSurface) {
+  SetFrameSinkHierarchy(kParentFrameSink, kChildFrameSink1);
+  const SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+  const SurfaceId child_id1 = MakeSurfaceId(kChildFrameSink1, 1, 1);
+  const SurfaceId child_id2 = MakeSurfaceId(kChildFrameSink1, 1, 2);
+  const SurfaceId child_id3 = MakeSurfaceId(kChildFrameSink1, 1, 3);
+
+  // |child_id1| Surface should immediately activate.
+  child_support1().SubmitCompositorFrame(child_id1.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+  Surface* child_surface1 = GetSurfaceForId(child_id1);
+  ASSERT_NE(nullptr, child_surface1);
+  EXPECT_FALSE(child_surface1->HasPendingFrame());
+  EXPECT_TRUE(child_surface1->HasActiveFrame());
+
+  // |child_id2| Surface should not activate because |child_id1| was never
+  // added as a dependency by a parent.
+  child_support1().SubmitCompositorFrame(child_id2.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+  Surface* child_surface2 = GetSurfaceForId(child_id2);
+  ASSERT_NE(nullptr, child_surface2);
+  EXPECT_TRUE(child_surface2->HasPendingFrame());
+  EXPECT_FALSE(child_surface2->HasActiveFrame());
+
+  // The parent finally embeds a child surface that hasn't arrived which
+  // activates |child_id2|'s Surface in order for the child to make forward
+  // progress.
+  parent_support().SubmitCompositorFrame(
+      parent_id.local_surface_id(),
+      MakeCompositorFrame({child_id3}, {SurfaceRange(base::nullopt, child_id3)},
+                          std::vector<TransferableResource>(),
+                          MakeDefaultDeadline()));
+
+  EXPECT_FALSE(child_surface2->HasPendingFrame());
+  EXPECT_TRUE(child_surface2->HasActiveFrame());
+}
+
+// A child surface can be blocked on its own activation dependencies and on a
+// parent embedding it as an activation dependency. Even if a child's activation
+// dependencies arrive, it may not activate until it is embedded.
+TEST_F(SurfaceSynchronizationTest, ChildBlockedOnDependenciesAndParent) {
+  const SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+  const SurfaceId child_id1 = MakeSurfaceId(kChildFrameSink1, 1, 1);
+  const SurfaceId child_id2 = MakeSurfaceId(kChildFrameSink1, 1, 2);
+  const SurfaceId child_id3 = MakeSurfaceId(kChildFrameSink2, 1);
+
+  // |child_id1| Surface should immediately activate.
+  child_support1().SubmitCompositorFrame(child_id1.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+  Surface* child_surface1 = GetSurfaceForId(child_id1);
+  ASSERT_NE(nullptr, child_surface1);
+  EXPECT_FALSE(child_surface1->HasPendingFrame());
+  EXPECT_TRUE(child_surface1->HasActiveFrame());
+
+  // |child_id2| Surface should not activate because |child_id1| was never
+  // added as a dependency by a parent AND it depends on |child_id3| which has
+  // not yet arrived.
+  child_support1().SubmitCompositorFrame(
+      child_id2.local_surface_id(),
+      MakeCompositorFrame({child_id3}, {SurfaceRange(base::nullopt, child_id3)},
+                          std::vector<TransferableResource>(),
+                          MakeDefaultDeadline()));
+  Surface* child_surface2 = GetSurfaceForId(child_id2);
+  ASSERT_NE(nullptr, child_surface2);
+  EXPECT_TRUE(child_surface2->HasPendingFrame());
+  EXPECT_FALSE(child_surface2->HasActiveFrame());
+  EXPECT_THAT(child_surface2->activation_dependencies(),
+              UnorderedElementsAre(child_id3));
+
+  // |child_id2|'s dependency has arrived but |child_id2| Surface has not
+  // activated because it is still throttled by its parent. It will not
+  // activate until its parent arrives.
+  child_support2().SubmitCompositorFrame(child_id3.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+  EXPECT_THAT(child_surface2->activation_dependencies(), IsEmpty());
+  EXPECT_TRUE(child_surface2->HasPendingFrame());
+  EXPECT_FALSE(child_surface2->HasActiveFrame());
+
+  // The parent finally embeds a |child_id2| which activates |child_id2|'s
+  // Surface.
+  parent_support().SubmitCompositorFrame(
+      parent_id.local_surface_id(),
+      MakeCompositorFrame({child_id2}, {SurfaceRange(base::nullopt, child_id2)},
+                          std::vector<TransferableResource>(),
+                          MakeDefaultDeadline()));
+
+  EXPECT_FALSE(child_surface2->HasPendingFrame());
+  EXPECT_TRUE(child_surface2->HasActiveFrame());
+}
+
+// Similar to the previous test, a child surface can be blocked on its own
+// activation dependencies and on a parent embedding it as an activation
+// dependency. In this case, the parent CompositorFrame arrives first, and then
+// the activation dependency.
+TEST_F(SurfaceSynchronizationTest, ChildBlockedOnDependenciesAndParent2) {
+  const SurfaceId parent_id = MakeSurfaceId(kParentFrameSink, 1);
+  const SurfaceId child_id1 = MakeSurfaceId(kChildFrameSink1, 1, 1);
+  const SurfaceId child_id2 = MakeSurfaceId(kChildFrameSink1, 1, 2);
+  const SurfaceId child_id3 = MakeSurfaceId(kChildFrameSink2, 1);
+
+  // |child_id1| Surface should immediately activate.
+  child_support1().SubmitCompositorFrame(child_id1.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+  Surface* child_surface1 = GetSurfaceForId(child_id1);
+  ASSERT_NE(nullptr, child_surface1);
+  EXPECT_FALSE(child_surface1->HasPendingFrame());
+  EXPECT_TRUE(child_surface1->HasActiveFrame());
+
+  // |child_id2| Surface should not activate because |child_id1| was never
+  // added as a dependency by a parent AND it depends on |child_id3| which has
+  // not yet arrived.
+  child_support1().SubmitCompositorFrame(
+      child_id2.local_surface_id(),
+      MakeCompositorFrame({child_id3}, {SurfaceRange(base::nullopt, child_id3)},
+                          std::vector<TransferableResource>(),
+                          MakeDefaultDeadline()));
+  Surface* child_surface2 = GetSurfaceForId(child_id2);
+  ASSERT_NE(nullptr, child_surface2);
+  EXPECT_TRUE(child_surface2->HasPendingFrame());
+  EXPECT_FALSE(child_surface2->HasActiveFrame());
+  EXPECT_THAT(child_surface2->activation_dependencies(),
+              UnorderedElementsAre(child_id3));
+  EXPECT_FALSE(child_surface2->HasDependentFrame());
+
+  // The parent embeds |child_id2| but |child_id2|'s Surface cannot activate
+  // until its dependencies arrive.
+  parent_support().SubmitCompositorFrame(
+      parent_id.local_surface_id(),
+      MakeCompositorFrame({child_id2}, {SurfaceRange(base::nullopt, child_id2)},
+                          std::vector<TransferableResource>(),
+                          MakeDefaultDeadline()));
+  EXPECT_TRUE(child_surface2->HasPendingFrame());
+  EXPECT_FALSE(child_surface2->HasActiveFrame());
+  EXPECT_THAT(child_surface2->activation_dependencies(),
+              UnorderedElementsAre(child_id3));
+  EXPECT_TRUE(child_surface2->HasDependentFrame());
+
+  // |child_id2|'s dependency has arrived and |child_id2|'s Surface finally
+  // activates.
+  child_support2().SubmitCompositorFrame(child_id3.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+  EXPECT_THAT(child_surface2->activation_dependencies(), IsEmpty());
+  EXPECT_FALSE(child_surface2->HasPendingFrame());
+  EXPECT_TRUE(child_surface2->HasActiveFrame());
+}
+
+// This test verifies that if a child-initiated synchronization is blocked
+// on a parent, then the frame will still activate after a deadline passes.
+TEST_F(SurfaceSynchronizationTest, ChildBlockedOnParentActivatesAfterDeadline) {
+  const SurfaceId child_id1 = MakeSurfaceId(kChildFrameSink1, 1, 1);
+  const SurfaceId child_id2 = MakeSurfaceId(kChildFrameSink1, 1, 2);
+
+  // |child_id1| Surface should immediately activate.
+  child_support1().SubmitCompositorFrame(child_id1.local_surface_id(),
+                                         MakeDefaultCompositorFrame());
+  Surface* child_surface1 = GetSurfaceForId(child_id1);
+  ASSERT_NE(nullptr, child_surface1);
+  EXPECT_FALSE(child_surface1->HasPendingFrame());
+  EXPECT_TRUE(child_surface1->HasActiveFrame());
+  EXPECT_FALSE(child_surface1->HasDependentFrame());
+  EXPECT_FALSE(child_surface1->has_deadline());
+
+  // |child_id2| Surface should not activate because |child_id1| was never
+  // added as a dependency by a parent.
+  child_support1().SubmitCompositorFrame(
+      child_id2.local_surface_id(),
+      MakeCompositorFrame(empty_surface_ids(), empty_surface_ranges(),
+                          std::vector<TransferableResource>(),
+                          MakeDefaultDeadline()));
+  Surface* child_surface2 = GetSurfaceForId(child_id2);
+  ASSERT_NE(nullptr, child_surface2);
+  EXPECT_TRUE(child_surface2->HasPendingFrame());
+  EXPECT_FALSE(child_surface2->HasActiveFrame());
+  EXPECT_FALSE(child_surface2->HasDependentFrame());
+  EXPECT_TRUE(child_surface2->has_deadline());
+
+  for (int i = 0; i < 3; ++i) {
+    SendNextBeginFrame();
+    // There is still a looming deadline! Eeek!
+    EXPECT_TRUE(child_surface2->HasPendingFrame());
+    EXPECT_FALSE(child_surface2->HasActiveFrame());
+    EXPECT_FALSE(child_surface2->HasDependentFrame());
+    EXPECT_TRUE(child_surface2->has_deadline());
+  }
+
+  SendNextBeginFrame();
+
+  EXPECT_FALSE(child_surface2->HasPendingFrame());
+  EXPECT_TRUE(child_surface2->HasActiveFrame());
+  // We pretend this is true so that subsequent CompositorFrames to the same
+  // surface do not block on the parent again.
+  EXPECT_TRUE(child_surface2->HasDependentFrame());
+  EXPECT_FALSE(child_surface2->has_deadline());
+}
+
 }  // namespace test
 }  // namespace viz

components/viz/service/surfaces/surface.cc

@@ -26,11 +26,13 @@ namespace viz {
 Surface::Surface(const SurfaceInfo& surface_info,
                  SurfaceManager* surface_manager,
                  base::WeakPtr<SurfaceClient> surface_client,
-                 bool needs_sync_tokens)
+                 bool needs_sync_tokens,
+                 bool block_activation_on_parent)
     : surface_info_(surface_info),
       surface_manager_(surface_manager),
       surface_client_(std::move(surface_client)),
-      needs_sync_tokens_(needs_sync_tokens) {
+      needs_sync_tokens_(needs_sync_tokens),
+      block_activation_on_parent_(block_activation_on_parent) {
   TRACE_EVENT_ASYNC_BEGIN1(TRACE_DISABLED_BY_DEFAULT("viz.surface_lifetime"),
                            "Surface", this, "surface_info",
                            surface_info.ToString());
@@ -186,6 +188,20 @@ void Surface::OnChildActivated(const SurfaceId& activated_id) {
   }
 }
 
+void Surface::OnSurfaceDependencyAdded() {
+  if (seen_first_surface_dependency_)
+    return;
+
+  seen_first_surface_dependency_ = true;
+  if (!activation_dependencies_.empty() || !pending_frame_data_)
+    return;
+
+  DCHECK(frame_sink_id_dependencies_.empty());
+
+  // All blockers have been cleared. The surface can be activated now.
+  ActivatePendingFrame(base::nullopt);
+}
+
 void Surface::Close() {
   closed_ = true;
 }
@@ -222,7 +238,16 @@ bool Surface::QueueFrame(
   // regardless of whether it's pending or active.
   surface_client_->ReceiveFromChild(frame.resource_list);
 
-  if (activation_dependencies_.empty()) {
+  if (!seen_first_surface_dependency_) {
+    seen_first_surface_dependency_ =
+        surface_manager_->dependency_tracker()->HasSurfaceBlockedOn(
+            surface_id());
+  }
+
+  bool block_activation =
+      block_activation_on_parent_ && !seen_first_surface_dependency_;
+
+  if (!block_activation && activation_dependencies_.empty()) {
     // If there are no blockers, then immediately activate the frame.
     ActivateFrame(
         FrameData(std::move(frame), frame_index, std::move(presented_callback)),
@@ -298,7 +323,12 @@ void Surface::NotifySurfaceIdAvailable(const SurfaceId& surface_id) {
                dependency.local_surface_id() <= surface_id.local_surface_id();
       });
 
-  if (!activation_dependencies_.empty())
+  // We cannot activate this CompositorFrame if there are still missing
+  // activation dependencies or this surface is blocked on its parent arriving
+  // and the parent has not arrived yet.
+  bool block_activation =
+      block_activation_on_parent_ && !seen_first_surface_dependency_;
+  if (block_activation || !activation_dependencies_.empty())
     return;
 
   DCHECK(frame_sink_id_dependencies_.empty());
@@ -307,6 +337,17 @@ void Surface::NotifySurfaceIdAvailable(const SurfaceId& surface_id) {
   ActivatePendingFrame(base::nullopt);
 }
 
+bool Surface::IsBlockedOn(const SurfaceId& surface_id) const {
+  for (const SurfaceId& dependency : activation_dependencies_) {
+    if (dependency.frame_sink_id() != surface_id.frame_sink_id())
+      continue;
+
+    if (dependency.local_surface_id() <= surface_id.local_surface_id())
+      return true;
+  }
+  return false;
+}
+
 void Surface::ActivatePendingFrameForDeadline(
     base::Optional<base::TimeDelta> duration) {
   if (!pending_frame_data_)
@@ -483,7 +524,12 @@ void Surface::UpdateActivationDependencies(
 
   for (const SurfaceId& surface_id :
        current_frame.metadata.activation_dependencies) {
+    // Inform the Surface |dependency| that it's been added as a dependency in
+    // another Surface's CompositorFrame.
+    surface_manager_->SurfaceDependencyAdded(surface_id);
+
     Surface* dependency = surface_manager_->GetSurfaceForId(surface_id);
+
     // If a activation dependency does not have a corresponding active frame in
     // the display compositor, then it blocks this frame.
     if (!dependency || !dependency->HasActiveFrame()) {

components/viz/service/surfaces/surface.h

@@ -81,7 +81,8 @@ class VIZ_SERVICE_EXPORT Surface final : public SurfaceDeadlineClient {
   Surface(const SurfaceInfo& surface_info,
           SurfaceManager* surface_manager,
           base::WeakPtr<SurfaceClient> surface_client,
-          bool needs_sync_tokens);
+          bool needs_sync_tokens,
+          bool block_activation_on_parent);
   ~Surface();
 
   void SetDependencyDeadline(
@@ -121,6 +122,10 @@ class VIZ_SERVICE_EXPORT Surface final : public SurfaceDeadlineClient {
 
   bool needs_sync_tokens() const { return needs_sync_tokens_; }
 
+  bool block_activation_on_parent() const {
+    return block_activation_on_parent_;
+  }
+
   // Returns false if |frame| is invalid.
   // |frame_rejected_callback| will be called once if the frame will not be
   // displayed.
@@ -136,6 +141,10 @@ class VIZ_SERVICE_EXPORT Surface final : public SurfaceDeadlineClient {
   // frame.
   void NotifySurfaceIdAvailable(const SurfaceId& surface_id);
 
+  // Returns whether the Surface is blocked on the provided |surface_id| or a
+  // predecessor.
+  bool IsBlockedOn(const SurfaceId& surface_id) const;
+
   // Called if a deadline has been hit and this surface is not yet active but
   // it's marked as respecting deadlines.
   void ActivatePendingFrameForDeadline(
@@ -200,6 +209,10 @@ class VIZ_SERVICE_EXPORT Surface final : public SurfaceDeadlineClient {
     return HasActiveFrame() && !active_frame_data_->frame_processed;
   }
 
+  // Returns true if at any point, another Surface's CompositorFrame has
+  // depended on this Surface.
+  bool HasDependentFrame() const { return seen_first_surface_dependency_; }
+
   // SurfaceDeadlineClient implementation:
   void OnDeadline(base::TimeDelta duration) override;
 
@@ -210,6 +223,9 @@ class VIZ_SERVICE_EXPORT Surface final : public SurfaceDeadlineClient {
   // referenced SurfaceRange.
   void OnChildActivated(const SurfaceId& surface_id);
 
+  // Called when this surface is embedded by another Surface's CompositorFrame.
+  void OnSurfaceDependencyAdded();
+
  private:
   struct SequenceNumbers {
     uint32_t parent_sequence_number = 0u;
@@ -296,7 +312,9 @@ class VIZ_SERVICE_EXPORT Surface final : public SurfaceDeadlineClient {
   bool closed_ = false;
   bool seen_first_frame_activation_ = false;
   bool seen_first_surface_embedding_ = false;
+  bool seen_first_surface_dependency_ = false;
   const bool needs_sync_tokens_;
+  const bool block_activation_on_parent_;
 
   base::flat_set<SurfaceId> activation_dependencies_;
   base::flat_set<SurfaceId> late_activation_dependencies_;

components/viz/service/surfaces/surface_dependency_tracker.cc

@@ -34,15 +34,72 @@ void SurfaceDependencyTracker::RequestSurfaceResolution(Surface* surface) {
     }
   }
 
+  // If |surface| is blocking on the arrival of a parent and the parent frame
+  // has not yet arrived then track this |surface|'s SurfaceId by FrameSinkId so
+  // that if a parent refers to it or a more recent surface, then
+  // SurfaceDependencyTracker reports back that a dependency has been added.
+  if (surface->block_activation_on_parent() && !surface->HasDependentFrame()) {
+    surfaces_blocked_on_parent_by_frame_sink_id_[surface->surface_id()
+                                                     .frame_sink_id()]
+        .insert(surface->surface_id());
+  }
+
   UpdateSurfaceDeadline(surface);
 }
 
+bool SurfaceDependencyTracker::HasSurfaceBlockedOn(
+    const SurfaceId& surface_id) const {
+  auto it = blocked_surfaces_from_dependency_.find(surface_id.frame_sink_id());
+  if (it == blocked_surfaces_from_dependency_.end())
+    return false;
+
+  for (const SurfaceId& blocked_surface_by_id : it->second) {
+    Surface* blocked_surface =
+        surface_manager_->GetSurfaceForId(blocked_surface_by_id);
+    if (blocked_surface && blocked_surface->IsBlockedOn(surface_id))
+      return true;
+  }
+  return false;
+}
+
 void SurfaceDependencyTracker::OnSurfaceActivated(Surface* surface) {
   if (!surface->late_activation_dependencies().empty())
     surfaces_with_missing_dependencies_.insert(surface->surface_id());
   else
     surfaces_with_missing_dependencies_.erase(surface->surface_id());
   NotifySurfaceIdAvailable(surface->surface_id());
+  // We treat an activation (by deadline) as being the equivalent of a parent
+  // embedding the surface.
+  OnSurfaceDependencyAdded(surface->surface_id());
+}
+
+void SurfaceDependencyTracker::OnSurfaceDependencyAdded(
+    const SurfaceId& surface_id) {
+  auto it = surfaces_blocked_on_parent_by_frame_sink_id_.find(
+      surface_id.frame_sink_id());
+  if (it == surfaces_blocked_on_parent_by_frame_sink_id_.end())
+    return;
+
+  std::vector<SurfaceId> dependencies_to_notify;
+
+  base::flat_set<SurfaceId>& blocked_surfaces = it->second;
+  for (auto iter = blocked_surfaces.begin(); iter != blocked_surfaces.end();) {
+    if (iter->local_surface_id() <= surface_id.local_surface_id()) {
+      dependencies_to_notify.push_back(*iter);
+      iter = blocked_surfaces.erase(iter);
+    } else {
+      ++iter;
+    }
+  }
+
+  if (blocked_surfaces.empty())
+    surfaces_blocked_on_parent_by_frame_sink_id_.erase(it);
+
+  for (const SurfaceId& dependency : dependencies_to_notify) {
+    Surface* surface = surface_manager_->GetSurfaceForId(dependency);
+    if (surface)
+      surface->OnSurfaceDependencyAdded();
+  }
 }
 
 void SurfaceDependencyTracker::OnSurfaceDependenciesChanged(
@@ -78,6 +135,7 @@ void SurfaceDependencyTracker::OnSurfaceDiscarded(Surface* surface) {
   // Pretend that the discarded surface's SurfaceId is now available to
   // unblock dependencies because we now know the surface will never activate.
   NotifySurfaceIdAvailable(surface->surface_id());
+  OnSurfaceDependencyAdded(surface->surface_id());
 }
 
 void SurfaceDependencyTracker::OnFrameSinkInvalidated(
@@ -85,6 +143,7 @@ void SurfaceDependencyTracker::OnFrameSinkInvalidated(
   // We now know the frame sink will never generated any more frames,
   // thus unblock all dependencies to any future surfaces.
   NotifySurfaceIdAvailable(SurfaceId::MaxSequenceId(frame_sink_id));
+  OnSurfaceDependencyAdded(SurfaceId::MaxSequenceId(frame_sink_id));
 }
 
 void SurfaceDependencyTracker::ActivateLateSurfaceSubtree(Surface* surface) {

components/viz/service/surfaces/surface_dependency_tracker.h

@@ -34,7 +34,12 @@ class VIZ_SERVICE_EXPORT SurfaceDependencyTracker {
   // informed when that surface's dependencies are resolved.
   void RequestSurfaceResolution(Surface* surface);
 
+  // Returns whether the dependency tracker has a surface blocked on the
+  // provided |surface_id|.
+  bool HasSurfaceBlockedOn(const SurfaceId& surface_id) const;
+
   void OnSurfaceActivated(Surface* surface);
+  void OnSurfaceDependencyAdded(const SurfaceId& surface_id);
   void OnSurfaceDependenciesChanged(
       Surface* surface,
       const base::flat_set<FrameSinkId>& added_dependencies,
@@ -68,6 +73,11 @@ class VIZ_SERVICE_EXPORT SurfaceDependencyTracker {
   std::unordered_map<FrameSinkId, base::flat_set<SurfaceId>, FrameSinkIdHash>
       blocked_surfaces_from_dependency_;
 
+  // A map from a FrameSinkid to a set of surfaces with that FrameSinkId that
+  // are blocked on a parent arriving to embed them.
+  std::unordered_map<FrameSinkId, base::flat_set<SurfaceId>, FrameSinkIdHash>
+      surfaces_blocked_on_parent_by_frame_sink_id_;
+
   // The set of SurfaceIds corresponding to Surfaces that have active
   // CompositorFrames with missing dependencies.
   base::flat_set<SurfaceId> surfaces_with_missing_dependencies_;

components/viz/service/surfaces/surface_manager.cc

@@ -106,7 +106,8 @@ Surface* SurfaceManager::CreateSurface(
     base::WeakPtr<SurfaceClient> surface_client,
     const SurfaceInfo& surface_info,
     BeginFrameSource* begin_frame_source,
-    bool needs_sync_tokens) {
+    bool needs_sync_tokens,
+    bool block_activation_on_parent) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
   DCHECK(surface_info.is_valid());
   DCHECK(surface_client);
@@ -116,7 +117,8 @@ Surface* SurfaceManager::CreateSurface(
   auto it = surface_map_.find(surface_info.id());
   if (it == surface_map_.end()) {
     std::unique_ptr<Surface> surface = std::make_unique<Surface>(
-        surface_info, this, surface_client, needs_sync_tokens);
+        surface_info, this, surface_client, needs_sync_tokens,
+        block_activation_on_parent);
     surface->SetDependencyDeadline(std::make_unique<SurfaceDependencyDeadline>(
         surface.get(), begin_frame_source, tick_clock_));
     surface_map_[surface_info.id()] = std::move(surface);
@@ -606,6 +608,10 @@ void SurfaceManager::SurfaceActivated(
   dependency_tracker_.OnSurfaceActivated(surface);
 }
 
+void SurfaceManager::SurfaceDependencyAdded(const SurfaceId& surface_id) {
+  dependency_tracker_.OnSurfaceDependencyAdded(surface_id);
+}
+
 void SurfaceManager::SurfaceDependenciesChanged(
     Surface* surface,
     const base::flat_set<FrameSinkId>& added_dependencies,

components/viz/service/surfaces/surface_manager.h

@@ -85,11 +85,13 @@ class VIZ_SERVICE_EXPORT SurfaceManager {
   Surface* CreateSurface(base::WeakPtr<SurfaceClient> surface_client,
                          const SurfaceInfo& surface_info,
                          BeginFrameSource* begin_frame_source,
-                         bool needs_sync_tokens);
+                         bool needs_sync_tokens,
+                         bool block_activation_on_parent);
 
   // Destroy the Surface once a set of sequence numbers has been satisfied.
   void DestroySurface(const SurfaceId& surface_id);
 
+  // Returns a Surface corresponding to the provided |surface_id|.
   Surface* GetSurfaceForId(const SurfaceId& surface_id);
 
   void AddObserver(SurfaceObserver* obs) { observer_list_.AddObserver(obs); }
@@ -123,6 +125,10 @@ class VIZ_SERVICE_EXPORT SurfaceManager {
   void SurfaceActivated(Surface* surface,
                         base::Optional<base::TimeDelta> duration);
 
+  // Called when this |surface_id| is referenced as an activation dependency
+  // from a parent CompositorFrame.
+  void SurfaceDependencyAdded(const SurfaceId& surface_id);
+
   // Called when the dependencies of a pending CompositorFrame within |surface|
   // has changed.
   void SurfaceDependenciesChanged(