Surface Synchronization: Use system default deadline for child throttling

If a child surface's activation is throttled then use the system default
deadline as the lower bound for the throttling. This ensures that even if a
child does not have activation dependencies, it will still be throttled
by at least the system default deadline.

This CL also adds some debugging methods and comments on FrameDeadline.

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

components/viz/common/BUILD.gn

@@ -90,6 +90,7 @@ viz_component("common") {
     "quads/debug_border_draw_quad.h",
     "quads/draw_quad.cc",
     "quads/draw_quad.h",
+    "quads/frame_deadline.cc",
     "quads/frame_deadline.h",
     "quads/largest_draw_quad.cc",
     "quads/largest_draw_quad.h",

components/viz/common/quads/frame_deadline.cc

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "components/viz/common/quads/frame_deadline.h"
+
+#include <cinttypes>
+
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
+
+namespace viz {
+
+base::TimeTicks FrameDeadline::ToWallTime(
+    base::Optional<uint32_t> default_deadline_in_frames) const {
+  uint32_t deadline_in_frames = deadline_in_frames_;
+  if (use_default_lower_bound_deadline_) {
+    deadline_in_frames =
+        std::max(deadline_in_frames, default_deadline_in_frames.value_or(
+                                         std::numeric_limits<uint32_t>::max()));
+  }
+  return frame_start_time_ + deadline_in_frames * frame_interval_;
+}
+
+std::string FrameDeadline::ToString() const {
+  const base::TimeDelta start_time_delta =
+      frame_start_time_ - base::TimeTicks();
+  return base::StringPrintf(
+      "FrameDeadline(start time: %" PRId64
+      " ms, deadline in frames: %s, frame interval: %" PRId64 " ms)",
+      start_time_delta.InMilliseconds(),
+      use_default_lower_bound_deadline_
+          ? "unresolved"
+          : base::UintToString(deadline_in_frames_).c_str(),
+      frame_interval_.InMilliseconds());
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         const FrameDeadline& frame_deadline) {
+  return out << frame_deadline.ToString();
+}
+
+}  // namespace viz

components/viz/common/quads/frame_deadline.h

@@ -8,9 +8,20 @@
 #include "components/viz/common/viz_common_export.h"
 
 #include "base/time/time.h"
+#include "components/viz/common/frame_sinks/begin_frame_args.h"
 
 namespace viz {
 
+// FrameDeadline is a class that represents a CompositorFrame's deadline for
+// activation. The deadline consists of three components: start time, deadline
+// in frames, and frame interval. All three components are stored individually
+// in this class in order to allow resolution of this deadline that incorporates
+// a system default deadline in the Viz service. In particular, the computation
+// to translate FrameDeadline into wall time is:
+// if use system default lower bound deadline:
+//    start time + max(deadline in frames, default deadline) * frame interval
+// else:
+//    start time + deadline in frames * frame interval
 class VIZ_COMMON_EXPORT FrameDeadline {
  public:
   FrameDeadline() = default;
@@ -27,6 +38,12 @@ class VIZ_COMMON_EXPORT FrameDeadline {
 
   FrameDeadline& operator=(const FrameDeadline& other) = default;
 
+  // Converts this FrameDeadline object into a wall time given a system default
+  // deadline in frames.
+  base::TimeTicks ToWallTime(
+      base::Optional<uint32_t> default_deadline_in_frames =
+          base::nullopt) const;
+
   bool operator==(const FrameDeadline& other) const {
     return other.frame_start_time_ == frame_start_time_ &&
            other.deadline_in_frames_ == deadline_in_frames_ &&
@@ -49,13 +66,18 @@ class VIZ_COMMON_EXPORT FrameDeadline {
     return use_default_lower_bound_deadline_;
   }
 
+  std::string ToString() const;
+
  private:
   base::TimeTicks frame_start_time_;
   uint32_t deadline_in_frames_ = 0u;
-  base::TimeDelta frame_interval_;
+  base::TimeDelta frame_interval_ = BeginFrameArgs::DefaultInterval();
   bool use_default_lower_bound_deadline_ = true;
 };
 
+VIZ_COMMON_EXPORT std::ostream& operator<<(std::ostream& out,
+                                           const FrameDeadline& frame_deadline);
+
 }  // namespace viz
 
 #endif  // COMPONENTS_VIZ_COMMON_QUADS_FRAME_DEADLINE_H_

components/viz/service/frame_sinks/surface_synchronization_unittest.cc

@@ -2756,12 +2756,20 @@ TEST_F(SurfaceSynchronizationTest,
 
   // |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());
+  child_support1().SubmitCompositorFrame(
+      child_id2.local_surface_id(),
+      MakeCompositorFrame(empty_surface_ids(), empty_surface_ranges(),
+                          std::vector<TransferableResource>(),
+                          MakeDeadline(1u)));
   Surface* child_surface2 = GetSurfaceForId(child_id2);
   ASSERT_NE(nullptr, child_surface2);
   EXPECT_TRUE(child_surface2->HasPendingFrame());
   EXPECT_FALSE(child_surface2->HasActiveFrame());
+  EXPECT_TRUE(child_surface2->has_deadline());
+
+  FrameDeadline deadline = MakeDefaultDeadline();
+  base::TimeTicks deadline_wall_time = deadline.ToWallTime();
+  EXPECT_EQ(deadline_wall_time, child_surface2->deadline_for_testing());
 
   // 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
@@ -2970,7 +2978,6 @@ TEST_F(SurfaceSynchronizationTest, ChildBlockedOnParentDeadlineInPast) {
   EXPECT_TRUE(child_surface2->HasActiveFrame());
   EXPECT_FALSE(child_surface2->has_deadline());
 
-  // This failed before the latest change.
   EXPECT_TRUE(child_surface2->HasDependentFrame());
 }
 

components/viz/service/surfaces/surface.cc

@@ -508,9 +508,13 @@ FrameDeadline Surface::ResolveFrameDeadline(
   const FrameDeadline& deadline = current_frame.metadata.deadline;
   uint32_t deadline_in_frames = deadline.deadline_in_frames();
 
+  bool block_activation =
+      block_activation_on_parent_ && !seen_first_surface_dependency_;
+
   // If no default deadline is available then all deadlines are treated as
   // effectively infinite deadlines.
-  if (!default_deadline || deadline.use_default_lower_bound_deadline()) {
+  if (!default_deadline || deadline.use_default_lower_bound_deadline() ||
+      block_activation) {
     deadline_in_frames = std::max(
         deadline_in_frames,
         default_deadline.value_or(std::numeric_limits<uint32_t>::max()));

components/viz/service/surfaces/surface.h

@@ -105,6 +105,10 @@ class VIZ_SERVICE_EXPORT Surface final : public SurfaceDeadlineClient {
 
   bool has_deadline() const { return deadline_ && deadline_->has_deadline(); }
 
+  base::Optional<base::TimeTicks> deadline_for_testing() const {
+    return deadline_->deadline_for_testing();
+  }
+
   // Inherits the same deadline as the one specified by |surface|. A deadline
   // may be set further out in order to avoid doing unnecessary work while a
   // parent surface is blocked on dependencies. A deadline may be shortened

components/viz/service/surfaces/surface_dependency_deadline.cc

@@ -31,8 +31,7 @@ SurfaceDependencyDeadline::~SurfaceDependencyDeadline() {
 bool SurfaceDependencyDeadline::Set(const FrameDeadline& frame_deadline) {
   CancelInternal(false);
   start_time_ = frame_deadline.frame_start_time();
-  deadline_ = start_time_ + frame_deadline.deadline_in_frames() *
-                                frame_deadline.frame_interval();
+  deadline_ = frame_deadline.ToWallTime();
   begin_frame_source_->AddObserver(this);
   return has_deadline();
 }