Extract Damage tracking from DisplayScheduler

This CL extracts surface damage tracking logic from DisplayScheduler
to new class DisplayDamageTracker. After the change DisplayScheduler
is only doing deadline planning based on damage information from
tracker.

Bug: 805739
Change-Id: Ibafb6e17a6ab9a3b2e0db466dcc031059896f50b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944631Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#722968}

components/viz/service/BUILD.gn

@@ -27,6 +27,8 @@ viz_component("service") {
     "display/display.cc",
     "display/display.h",
     "display/display_client.h",
+    "display/display_damage_tracker.cc",
+    "display/display_damage_tracker.h",
     "display/display_resource_provider.cc",
     "display/display_resource_provider.h",
     "display/display_scheduler.cc",
@@ -421,6 +423,7 @@ viz_source_set("unit_tests") {
   sources = [
     "display/bsp_tree_unittest.cc",
     "display/copy_output_scaling_pixeltest.cc",
+    "display/display_damage_tracker_unittest.cc",
     "display/display_resource_provider_unittest.cc",
     "display/display_scheduler_unittest.cc",
     "display/display_unittest.cc",

components/viz/service/display/display.cc

@@ -213,8 +213,6 @@ Display::~Display() {
       context->RemoveObserver(this);
     if (skia_output_surface_)
       skia_output_surface_->RemoveContextLostObserver(this);
-    if (scheduler_)
-      surface_manager_->RemoveObserver(scheduler_.get());
   }
 
   // Un-register as DisplaySchedulerClient to prevent us from being called in a
@@ -222,7 +220,7 @@ Display::~Display() {
   if (scheduler_)
     scheduler_->SetClient(nullptr);
 
-  RunDrawCallbacks();
+  damage_tracker_->RunDrawCallbacks();
 }
 
 void Display::Initialize(DisplayClient* client,
@@ -233,8 +231,6 @@ void Display::Initialize(DisplayClient* client,
   gpu::ScopedAllowScheduleGpuTask allow_schedule_gpu_task;
   client_ = client;
   surface_manager_ = surface_manager;
-  if (scheduler_)
-    surface_manager_->AddObserver(scheduler_.get());
 
   output_surface_->BindToClient(this);
   if (output_surface_->software_device())
@@ -245,6 +241,11 @@ void Display::Initialize(DisplayClient* client,
 
   InitializeRenderer(enable_shared_images);
 
+  damage_tracker_ = std::make_unique<DisplayDamageTracker>(surface_manager_,
+                                                           aggregator_.get());
+  if (scheduler_)
+    scheduler_->SetDamageTracker(damage_tracker_.get());
+
   // This depends on assumptions that Display::Initialize will happen on the
   // same callstack as the ContextProvider being created/initialized or else
   // it could miss a callback before setting this.
@@ -274,9 +275,7 @@ void Display::SetLocalSurfaceId(const LocalSurfaceId& id,
   current_surface_id_ = SurfaceId(frame_sink_id_, id);
   device_scale_factor_ = device_scale_factor;
 
-  UpdateRootFrameMissing();
-  if (scheduler_)
-    scheduler_->SetNewRootSurface(current_surface_id_);
+  damage_tracker_->SetNewRootSurface(current_surface_id_);
 }
 
 void Display::SetVisible(bool visible) {
@@ -307,8 +306,8 @@ void Display::Resize(const gfx::Size& size) {
 
   swapped_since_resize_ = false;
   current_surface_size_ = size;
-  if (scheduler_)
-    scheduler_->DisplayResized();
+
+  damage_tracker_->DisplayResized();
 }
 
 void Display::DisableSwapUntilResize(
@@ -346,11 +345,7 @@ void Display::SetColorMatrix(const SkMatrix44& matrix) {
       aggregator_->SetFullDamageForSurface(current_surface_id_);
   }
 
-  if (scheduler_) {
-    BeginFrameAck ack;
-    ack.has_damage = true;
-    scheduler_->ProcessSurfaceDamage(current_surface_id_, ack, true);
-  }
+  damage_tracker_->SetRootSurfaceDamaged();
 }
 
 void Display::SetColorSpace(const gfx::ColorSpace& device_color_space,
@@ -436,13 +431,7 @@ void Display::InitializeRenderer(bool enable_shared_images) {
 }
 
 bool Display::IsRootFrameMissing() const {
-  Surface* surface = surface_manager_->GetSurfaceForId(current_surface_id_);
-  return !surface || !surface->HasActiveFrame();
-}
-
-void Display::UpdateRootFrameMissing() {
-  if (scheduler_)
-    scheduler_->SetRootFrameMissing(IsRootFrameMissing());
+  return damage_tracker_->root_frame_missing();
 }
 
 void Display::OnContextLost() {
@@ -523,7 +512,7 @@ bool Display::DrawAndSwap() {
                            swapped_trace_id_);
 
   // Run callbacks early to allow pipelining and collect presented callbacks.
-  RunDrawCallbacks();
+  damage_tracker_->RunDrawCallbacks();
 
   frame.metadata.latency_info.insert(frame.metadata.latency_info.end(),
                                      stored_latency_info_.begin(),
@@ -779,46 +768,7 @@ void Display::DidReceivePresentationFeedback(
 
 void Display::SetNeedsRedrawRect(const gfx::Rect& damage_rect) {
   aggregator_->SetFullDamageForSurface(current_surface_id_);
-  if (scheduler_) {
-    BeginFrameAck ack;
-    ack.has_damage = true;
-    scheduler_->ProcessSurfaceDamage(current_surface_id_, ack, true);
-  }
-}
-
-bool Display::SurfaceDamaged(const SurfaceId& surface_id,
-                             const BeginFrameAck& ack) {
-  if (!ack.has_damage)
-    return false;
-  bool display_damaged = false;
-  if (aggregator_) {
-    display_damaged |=
-        aggregator_->NotifySurfaceDamageAndCheckForDisplayDamage(surface_id);
-  }
-  if (surface_id == current_surface_id_) {
-    display_damaged = true;
-    UpdateRootFrameMissing();
-  }
-  if (display_damaged)
-    surfaces_to_ack_on_next_draw_.push_back(surface_id);
-  return display_damaged;
-}
-
-void Display::SurfaceDestroyed(const SurfaceId& surface_id) {
-  TRACE_EVENT0("viz", "Display::SurfaceDestroyed");
-  if (aggregator_)
-    aggregator_->ReleaseResources(surface_id);
-}
-
-bool Display::SurfaceHasUnackedFrame(const SurfaceId& surface_id) const {
-  if (!surface_manager_)
-    return false;
-
-  Surface* surface = surface_manager_->GetSurfaceForId(surface_id);
-  if (!surface)
-    return false;
-
-  return surface->HasUnackedActiveFrame();
+  damage_tracker_->SetRootSurfaceDamaged();
 }
 
 void Display::DidFinishFrame(const BeginFrameAck& ack) {
@@ -1007,24 +957,6 @@ void Display::RemoveOverdrawQuads(CompositorFrame* frame) {
   }
 }
 
-void Display::RunDrawCallbacks() {
-  for (const auto& surface_id : surfaces_to_ack_on_next_draw_) {
-    Surface* surface = surface_manager_->GetSurfaceForId(surface_id);
-    if (surface)
-      surface->SendAckToClient();
-  }
-  surfaces_to_ack_on_next_draw_.clear();
-  // |surfaces_to_ack_on_next_draw_| does not cover surfaces that are being
-  // embedded for the first time, so also go through SurfaceAggregator's list.
-  if (aggregator_) {
-    for (const auto& id_entry : aggregator_->previous_contained_surfaces()) {
-      Surface* surface = surface_manager_->GetSurfaceForId(id_entry.first);
-      if (surface)
-        surface->SendAckToClient();
-    }
-  }
-}
-
 void Display::SetPreferredFrameInterval(base::TimeDelta interval) {
   client_->SetPreferredFrameInterval(interval);
 }

components/viz/service/display/display.h

@@ -133,10 +133,6 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
 
   // DisplaySchedulerClient implementation.
   bool DrawAndSwap() override;
-  bool SurfaceHasUnackedFrame(const SurfaceId& surface_id) const override;
-  bool SurfaceDamaged(const SurfaceId& surface_id,
-                      const BeginFrameAck& ack) override;
-  void SurfaceDestroyed(const SurfaceId& surface_id) override;
   void DidFinishFrame(const BeginFrameAck& ack) override;
 
   // OutputSurfaceClient implementation.
@@ -210,8 +206,6 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
   // TODO(cblume, crbug.com/900973): |enable_shared_images| is a temporary
   // solution that unblocks us until SharedImages are threadsafe in WebView.
   void InitializeRenderer(bool enable_shared_images = true);
-  void UpdateRootFrameMissing();
-  void RunDrawCallbacks();
 
   // ContextLostObserver implementation.
   void OnContextLost() override;
@@ -238,6 +232,7 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
 #endif
   std::unique_ptr<OutputSurface> output_surface_;
   SkiaOutputSurface* const skia_output_surface_;
+  std::unique_ptr<DisplayDamageTracker> damage_tracker_;
   std::unique_ptr<DisplayScheduler> scheduler_;
   std::unique_ptr<DisplayResourceProvider> resource_provider_;
   std::unique_ptr<SurfaceAggregator> aggregator_;
@@ -247,7 +242,6 @@ class VIZ_SERVICE_EXPORT Display : public DisplaySchedulerClient,
   std::unique_ptr<DirectRenderer> renderer_;
   SoftwareRenderer* software_renderer_ = nullptr;
   std::vector<ui::LatencyInfo> stored_latency_info_;
-  std::vector<SurfaceId> surfaces_to_ack_on_next_draw_;
 
   // |pending_presentation_group_timings_| stores a
   // Display::PresentationGroupTiming for each group currently waiting for

components/viz/service/display/display_damage_tracker.cc

+// Copyright 2019 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/service/display/display_damage_tracker.h"
+
+#include "base/trace_event/trace_event.h"
+#include "components/viz/common/surfaces/surface_info.h"
+#include "components/viz/service/display/surface_aggregator.h"
+#include "components/viz/service/surfaces/surface.h"
+#include "components/viz/service/surfaces/surface_manager.h"
+
+namespace viz {
+
+DisplayDamageTracker::DisplayDamageTracker(SurfaceManager* surface_manager,
+                                           SurfaceAggregator* aggregator)
+    : surface_manager_(surface_manager), aggregator_(aggregator) {
+  DCHECK(surface_manager_);
+  DCHECK(aggregator_);
+  surface_manager_->AddObserver(this);
+}
+
+DisplayDamageTracker::~DisplayDamageTracker() {
+  surface_manager_->RemoveObserver(this);
+}
+
+void DisplayDamageTracker::AddObserver(Observer* observer) {
+  observers_.AddObserver(observer);
+}
+
+void DisplayDamageTracker::RemoveObserver(Observer* observer) {
+  observers_.RemoveObserver(observer);
+}
+
+void DisplayDamageTracker::SetRootFrameMissing(bool missing) {
+  TRACE_EVENT1("viz", "DisplayDamageTracker::SetRootFrameMissing", "missing",
+               missing);
+  if (root_frame_missing_ == missing)
+    return;
+
+  root_frame_missing_ = missing;
+  NotifyRootFrameMissing(missing);
+}
+
+void DisplayDamageTracker::SetNewRootSurface(const SurfaceId& root_surface_id) {
+  TRACE_EVENT0("viz", "DisplayDamageTracker::SetNewRootSurface");
+  root_surface_id_ = root_surface_id;
+  UpdateRootFrameMissing();
+  SetRootSurfaceDamaged();
+}
+
+void DisplayDamageTracker::SetRootSurfaceDamaged() {
+  BeginFrameAck ack;
+  ack.has_damage = true;
+  ProcessSurfaceDamage(root_surface_id_, ack, true);
+}
+
+bool DisplayDamageTracker::IsRootSurfaceValid() const {
+  return root_surface_id_.is_valid();
+}
+
+void DisplayDamageTracker::DisplayResized() {
+  expecting_root_surface_damage_because_of_resize_ = true;
+  // Technically we don't have any damage yet, but we need to draw after resize,
+  // so we report display damaged here.
+  NotifyDisplayDamaged();
+}
+
+void DisplayDamageTracker::ProcessSurfaceDamage(const SurfaceId& surface_id,
+                                                const BeginFrameAck& ack,
+                                                bool display_damaged) {
+  TRACE_EVENT1("viz", "DisplayDamageTracker::SurfaceDamaged", "surface_id",
+               surface_id.ToString());
+
+  if (surface_id == root_surface_id_)
+    expecting_root_surface_damage_because_of_resize_ = false;
+
+  // Update surface state.
+  bool valid_ack = ack.sequence_number != BeginFrameArgs::kInvalidFrameNumber;
+  if (valid_ack) {
+    auto it = surface_states_.find(surface_id);
+    // Ignore stray acknowledgments for prior BeginFrames, to ensure we don't
+    // override a newer sequence number in the surface state. We may receive
+    // such stray acks e.g. when a CompositorFrame activates in a later
+    // BeginFrame than it was created.
+    if (it != surface_states_.end() &&
+        (it->second.last_ack.source_id != ack.source_id ||
+         it->second.last_ack.sequence_number < ack.sequence_number)) {
+      it->second.last_ack = ack;
+    } else {
+      valid_ack = false;
+    }
+  }
+
+  if (display_damaged) {
+    NotifyDisplayDamaged();
+  } else if (valid_ack) {
+    NotifyPendingSurfacesChanged();
+  }
+}
+
+bool DisplayDamageTracker::SurfaceHasUnackedFrame(
+    const SurfaceId& surface_id) const {
+  Surface* surface = surface_manager_->GetSurfaceForId(surface_id);
+  if (!surface)
+    return false;
+
+  return surface->HasUnackedActiveFrame();
+}
+
+bool DisplayDamageTracker::HasPendingSurfaces(
+    const BeginFrameArgs& begin_frame_args) {
+  for (auto& entry : surface_states_) {
+    const SurfaceId& surface_id = entry.first;
+    const SurfaceBeginFrameState& state = entry.second;
+
+    // Surface is ready if it hasn't received the current BeginFrame or receives
+    // BeginFrames from a different source and thus likely belongs to a
+    // different surface hierarchy.
+    uint64_t source_id = begin_frame_args.source_id;
+    uint64_t sequence_number = begin_frame_args.sequence_number;
+    if (!state.last_args.IsValid() || state.last_args.source_id != source_id ||
+        state.last_args.sequence_number != sequence_number) {
+      continue;
+    }
+
+    // Surface is ready if it has acknowledged the current BeginFrame.
+    if (state.last_ack.source_id == source_id &&
+        state.last_ack.sequence_number == sequence_number) {
+      continue;
+    }
+
+    // Surface is ready if there is an unacked active CompositorFrame, because
+    // its producer is CompositorFrameAck throttled.
+    if (SurfaceHasUnackedFrame(surface_id))
+      continue;
+
+    TRACE_EVENT_INSTANT2("viz", "DisplayDamageTracker::HasPendingSurfaces",
+                         TRACE_EVENT_SCOPE_THREAD, "has_pending_surfaces", true,
+                         "pending_surface_id", surface_id.ToString());
+
+    return true;
+  }
+
+  TRACE_EVENT_INSTANT1("viz", "DisplayDamageTracker::HasPendingSurfaces",
+                       TRACE_EVENT_SCOPE_THREAD, "has_pending_surfaces", false);
+
+  return false;
+}
+
+void DisplayDamageTracker::OnSurfaceMarkedForDestruction(
+    const SurfaceId& surface_id) {
+  auto it = surface_states_.find(surface_id);
+  if (it == surface_states_.end())
+    return;
+  surface_states_.erase(it);
+
+  NotifyPendingSurfacesChanged();
+}
+
+bool DisplayDamageTracker::OnSurfaceDamaged(const SurfaceId& surface_id,
+                                            const BeginFrameAck& ack) {
+  bool display_damaged = false;
+  if (ack.has_damage) {
+    display_damaged =
+        aggregator_->NotifySurfaceDamageAndCheckForDisplayDamage(surface_id);
+
+    if (surface_id == root_surface_id_)
+      display_damaged = true;
+    if (display_damaged)
+      surfaces_to_ack_on_next_draw_.push_back(surface_id);
+  }
+
+  if (surface_id == root_surface_id_)
+    UpdateRootFrameMissing();
+
+  ProcessSurfaceDamage(surface_id, ack, display_damaged);
+
+  return display_damaged;
+}
+
+void DisplayDamageTracker::OnSurfaceDestroyed(const SurfaceId& surface_id) {
+  TRACE_EVENT0("viz", "DisplayDamageTracker::SurfaceDestroyed");
+  aggregator_->ReleaseResources(surface_id);
+}
+
+void DisplayDamageTracker::OnSurfaceDamageExpected(const SurfaceId& surface_id,
+                                                   const BeginFrameArgs& args) {
+  TRACE_EVENT1("viz", "DisplayDamageTracker::SurfaceDamageExpected",
+               "surface_id", surface_id.ToString());
+  // Insert a new state for the surface if we don't know of it yet. We don't use
+  // OnSurfaceCreated() for this, because it may not be called if a
+  // CompositorFrameSinkSupport starts submitting frames to a different Display,
+  // but continues using the same Surface, or if a Surface does not activate its
+  // first CompositorFrame immediately.
+  surface_states_[surface_id].last_args = args;
+  NotifyPendingSurfacesChanged();
+}
+
+void DisplayDamageTracker::UpdateRootFrameMissing() {
+  Surface* surface = surface_manager_->GetSurfaceForId(root_surface_id_);
+  SetRootFrameMissing(!surface || !surface->HasActiveFrame());
+}
+
+void DisplayDamageTracker::RunDrawCallbacks() {
+  for (const auto& surface_id : surfaces_to_ack_on_next_draw_) {
+    Surface* surface = surface_manager_->GetSurfaceForId(surface_id);
+    if (surface)
+      surface->SendAckToClient();
+  }
+  surfaces_to_ack_on_next_draw_.clear();
+  // |surfaces_to_ack_on_next_draw_| does not cover surfaces that are being
+  // embedded for the first time, so also go through SurfaceAggregator's list.
+  for (const auto& id_entry : aggregator_->previous_contained_surfaces()) {
+    Surface* surface = surface_manager_->GetSurfaceForId(id_entry.first);
+    if (surface)
+      surface->SendAckToClient();
+  }
+}
+
+void DisplayDamageTracker::NotifyDisplayDamaged() {
+  for (auto& observer : observers_)
+    observer.OnDisplayDamaged();
+}
+
+void DisplayDamageTracker::NotifyRootFrameMissing(bool missing) {
+  for (auto& observer : observers_)
+    observer.OnRootFrameMissing(missing);
+}
+
+void DisplayDamageTracker::NotifyPendingSurfacesChanged() {
+  for (auto& observer : observers_)
+    observer.OnPendingSurfacesChanged();
+}
+
+}  // namespace viz

components/viz/service/display/display_damage_tracker.h

+// Copyright 2019 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.
+
+#ifndef COMPONENTS_VIZ_SERVICE_DISPLAY_DISPLAY_DAMAGE_TRACKER_H_
+#define COMPONENTS_VIZ_SERVICE_DISPLAY_DISPLAY_DAMAGE_TRACKER_H_
+
+#include <memory>
+
+#include "base/containers/flat_map.h"
+#include "base/observer_list.h"
+#include "components/viz/common/frame_sinks/begin_frame_args.h"
+#include "components/viz/common/surfaces/surface_id.h"
+#include "components/viz/service/surfaces/surface_observer.h"
+#include "components/viz/service/viz_service_export.h"
+
+namespace viz {
+
+class SurfaceAggregator;
+class SurfaceInfo;
+class SurfaceManager;
+
+// DisplayDamageTracker is used to track Surfaces damage that belong to current
+// Display. It tracks pending damage when clients received BeginFrames but
+// didn't replied yet and it tracks whether damage to those surfaces contribute
+// to Display damage. Used by DisplayScheduler to determine frame deadlines.
+class VIZ_SERVICE_EXPORT DisplayDamageTracker : public SurfaceObserver {
+ public:
+  class VIZ_SERVICE_EXPORT Observer {
+   public:
+    virtual ~Observer() = default;
+    virtual void OnDisplayDamaged() = 0;
+    virtual void OnRootFrameMissing(bool missing) = 0;
+    virtual void OnPendingSurfacesChanged() = 0;
+  };
+
+  DisplayDamageTracker(SurfaceManager* surface_manager,
+                       SurfaceAggregator* aggregator);
+  ~DisplayDamageTracker() override;
+
+  DisplayDamageTracker(const DisplayDamageTracker&) = delete;
+  DisplayDamageTracker& operator=(const DisplayDamageTracker&) = delete;
+
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
+  // Notification that there was a resize and we should expect root surface
+  // damage.
+  void DisplayResized();
+
+  // Notification that the root surface changed.
+  void SetNewRootSurface(const SurfaceId& root_surface_id);
+
+  // Mark root surface as damaged.
+  void SetRootSurfaceDamaged();
+
+  // Send Surface Acks to damaged surfaces after draw.
+  void RunDrawCallbacks();
+
+  // This returns whether there are pending surfaces. SurfaceIs pending if the
+  // corresponding CompositorFrameSink has received BeginFrame but hasn't
+  // replied with Ack yet.
+  bool HasPendingSurfaces(const BeginFrameArgs& begin_frame_args);
+
+  bool root_frame_missing() const { return root_frame_missing_; }
+  bool IsRootSurfaceValid() const;
+
+  bool expecting_root_surface_damage_because_of_resize() const {
+    return expecting_root_surface_damage_because_of_resize_;
+  }
+
+  void reset_expecting_root_surface_damage_because_of_resize() {
+    expecting_root_surface_damage_because_of_resize_ = false;
+  }
+
+  // SurfaceObserver implementation.
+  void OnFirstSurfaceActivation(const SurfaceInfo& surface_info) override {}
+  void OnSurfaceActivated(const SurfaceId& surface_id,
+                          base::Optional<base::TimeDelta> duration) override {}
+  void OnSurfaceMarkedForDestruction(const SurfaceId& surface_id) override;
+  bool OnSurfaceDamaged(const SurfaceId& surface_id,
+                        const BeginFrameAck& ack) override;
+  void OnSurfaceDestroyed(const SurfaceId& surface_id) override;
+  void OnSurfaceDamageExpected(const SurfaceId& surface_id,
+                               const BeginFrameArgs& args) override;
+
+ protected:
+  struct SurfaceBeginFrameState {
+    BeginFrameArgs last_args;
+    BeginFrameAck last_ack;
+  };
+
+  friend class base::RefCounted<DisplayDamageTracker>;
+  virtual bool SurfaceHasUnackedFrame(const SurfaceId& surface_id) const;
+  virtual void UpdateRootFrameMissing();
+  void SetRootFrameMissing(bool missing);
+
+  // Indicates that there was damage to one of the surfaces.
+  void ProcessSurfaceDamage(const SurfaceId& surface_id,
+                            const BeginFrameAck& ack,
+                            bool display_damaged);
+
+  // Used to send corresponding notifications to observers.
+  void NotifyDisplayDamaged();
+  void NotifyRootFrameMissing(bool missing);
+  void NotifyPendingSurfacesChanged();
+
+  base::ObserverList<Observer>::Unchecked observers_;
+  SurfaceManager* const surface_manager_;
+  SurfaceAggregator* const aggregator_;
+
+  bool root_frame_missing_ = true;
+
+  bool expecting_root_surface_damage_because_of_resize_ = false;
+
+  base::flat_map<SurfaceId, SurfaceBeginFrameState> surface_states_;
+  std::vector<SurfaceId> surfaces_to_ack_on_next_draw_;
+
+  SurfaceId root_surface_id_;
+};
+
+}  // namespace viz
+
+#endif  // COMPONENTS_VIZ_SERVICE_DISPLAY_DISPLAY_DAMAGE_TRACKER_H_

components/viz/service/display/display_damage_tracker_unittest.cc

+// Copyright 2015 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/service/display/display_damage_tracker.h"
+
+#include "base/test/null_task_runner.h"
+#include "components/viz/common/quads/compositor_frame.h"
+#include "components/viz/common/quads/render_pass.h"
+#include "components/viz/common/surfaces/frame_sink_id.h"
+#include "components/viz/common/surfaces/parent_local_surface_id_allocator.h"
+#include "components/viz/service/display/display_resource_provider.h"
+#include "components/viz/service/display/surface_aggregator.h"
+#include "components/viz/service/display_embedder/server_shared_bitmap_manager.h"
+#include "components/viz/service/frame_sinks/compositor_frame_sink_support.h"
+#include "components/viz/service/frame_sinks/frame_sink_manager_impl.h"
+#include "components/viz/test/compositor_frame_helpers.h"
+#include "components/viz/test/fake_external_begin_frame_source.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace viz {
+namespace {
+constexpr FrameSinkId kRootFrameSinkId(3, 3);
+constexpr FrameSinkId kChildFrameSinkId(4, 4);
+}  // namespace
+
+class DisplayDamageTrackerTest : public testing::Test {
+ public:
+  DisplayDamageTrackerTest()
+      : manager_(&shared_bitmap_manager_),
+        resource_provider_(DisplayResourceProvider::kSoftware,
+                           nullptr,
+                           &shared_bitmap_manager_,
+                           false),
+        aggregator_(manager_.surface_manager(),
+                    &resource_provider_,
+                    false,
+                    false),
+        root_client_(&manager_, kRootFrameSinkId),
+        task_runner_(base::MakeRefCounted<base::NullTaskRunner>()),
+        fake_begin_frame_source_(0.f, false),
+        damage_tracker_(
+            std::make_unique<DisplayDamageTracker>(manager_.surface_manager(),
+                                                   &aggregator_)) {
+    manager_.RegisterFrameSinkId(kRootFrameSinkId, false);
+    manager_.RegisterBeginFrameSource(&fake_begin_frame_source_,
+                                      kRootFrameSinkId);
+    damage_tracker_->SetNewRootSurface(root_client_.surface_id());
+  }
+
+  ~DisplayDamageTrackerTest() override {
+    manager_.UnregisterBeginFrameSource(&fake_begin_frame_source_);
+  }
+
+ protected:
+  class Client {
+   public:
+    Client(FrameSinkManagerImpl* manager, FrameSinkId frame_sink_id)
+        : frame_sink_id_(frame_sink_id),
+          support_(std::make_unique<CompositorFrameSinkSupport>(
+              nullptr,
+              manager,
+              frame_sink_id,
+              /*is_root=*/true,
+              /*needs_sync_points=*/true)) {
+      MakeNewSurfaceId();
+      support_->SetNeedsBeginFrame(true);
+    }
+
+    SurfaceId MakeNewSurfaceId() {
+      id_allocator_.GenerateId();
+      local_surface_id_ =
+          id_allocator_.GetCurrentLocalSurfaceIdAllocation().local_surface_id();
+      return SurfaceId(frame_sink_id_, local_surface_id_);
+    }
+
+    SurfaceId surface_id() {
+      return SurfaceId(frame_sink_id_, local_surface_id_);
+    }
+
+    void SubmitCompositorFrame(const BeginFrameArgs& args) {
+      RenderPassList pass_list;
+      auto pass = RenderPass::Create();
+      pass->output_rect = gfx::Rect(0, 0, 100, 100);
+      pass->damage_rect = gfx::Rect(10, 10, 1, 1);
+      pass->id = 1u;
+      pass_list.push_back(std::move(pass));
+
+      BeginFrameAck ack;
+      ack.source_id = args.source_id;
+      ack.sequence_number = args.sequence_number;
+      ack.has_damage = true;
+
+      CompositorFrame frame = CompositorFrameBuilder()
+                                  .SetRenderPassList(std::move(pass_list))
+                                  .SetBeginFrameAck(ack)
+                                  .Build();
+
+      support_->SubmitCompositorFrame(local_surface_id_, std::move(frame));
+    }
+
+    FrameSinkId frame_sink_id_;
+    LocalSurfaceId local_surface_id_;
+    std::unique_ptr<CompositorFrameSinkSupport> support_;
+    ParentLocalSurfaceIdAllocator id_allocator_;
+  };
+
+  void TickBeginFrame() {
+    // Ack previous surfaces
+    damage_tracker_->RunDrawCallbacks();
+
+    last_begin_frame_args_ =
+        fake_begin_frame_source_.CreateBeginFrameArgs(BEGINFRAME_FROM_HERE);
+    fake_begin_frame_source_.TestOnBeginFrame(last_begin_frame_args_);
+  }
+
+  ServerSharedBitmapManager shared_bitmap_manager_;
+  FrameSinkManagerImpl manager_;
+  DisplayResourceProvider resource_provider_;
+  SurfaceAggregator aggregator_;
+  Client root_client_;
+  scoped_refptr<base::NullTaskRunner> task_runner_;
+  FakeExternalBeginFrameSource fake_begin_frame_source_;
+  std::unique_ptr<DisplayDamageTracker> damage_tracker_;
+  BeginFrameArgs last_begin_frame_args_;
+};
+
+TEST_F(DisplayDamageTrackerTest, Basic) {
+  TickBeginFrame();
+
+  // Check that we don't have root surface
+  EXPECT_TRUE(damage_tracker_->IsRootSurfaceValid());
+  EXPECT_TRUE(damage_tracker_->root_frame_missing());
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  // Submit root surface and check that we have root surface and no pending
+  // surfaces
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  // Begin next frame
+  TickBeginFrame();
+
+  EXPECT_TRUE(damage_tracker_->IsRootSurfaceValid());
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+  EXPECT_TRUE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  // Submit CF and check that we have root surface and no pending surfaces
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+}
+
+TEST_F(DisplayDamageTrackerTest, Resize) {
+  EXPECT_TRUE(damage_tracker_->root_frame_missing());
+
+  // Submit initial frame
+  TickBeginFrame();
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+
+  // Expect no damage because of resize
+  EXPECT_FALSE(
+      damage_tracker_->expecting_root_surface_damage_because_of_resize());
+
+  // Resize display and expect there is root frame, but with expected damage
+  damage_tracker_->DisplayResized();
+  EXPECT_TRUE(
+      damage_tracker_->expecting_root_surface_damage_because_of_resize());
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+
+  // Submit next frame
+  TickBeginFrame();
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+
+  // Expecting no damage
+  EXPECT_FALSE(
+      damage_tracker_->expecting_root_surface_damage_because_of_resize());
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+}
+
+TEST_F(DisplayDamageTrackerTest, NewRoot) {
+  EXPECT_TRUE(damage_tracker_->root_frame_missing());
+
+  // Submit initial frame
+  TickBeginFrame();
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+
+  // Root shouldn't be missing and no pending surfaces
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  // Set new surface and expect root is missing
+  damage_tracker_->SetNewRootSurface(root_client_.MakeNewSurfaceId());
+  EXPECT_TRUE(damage_tracker_->root_frame_missing());
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  // Submit new root
+  TickBeginFrame();
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+
+  // Check that root is in place and no surface pending.
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+}
+
+TEST_F(DisplayDamageTrackerTest, TwoSurfaces) {
+  Client embedded_client(&manager_, kChildFrameSinkId);
+  manager_.RegisterFrameSinkHierarchy(kRootFrameSinkId, kChildFrameSinkId);
+
+  EXPECT_TRUE(damage_tracker_->root_frame_missing());
+
+  // Submit initial frame
+  TickBeginFrame();
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+  embedded_client.SubmitCompositorFrame(last_begin_frame_args_);
+
+  // Expect no pending surfaces
+  EXPECT_FALSE(damage_tracker_->root_frame_missing());
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  // Begin next frame and check submission in order embedded_client, root_client
+  TickBeginFrame();
+  EXPECT_TRUE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  embedded_client.SubmitCompositorFrame(last_begin_frame_args_);
+  EXPECT_TRUE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  // Begin next frame and check submission in order root_client, embedded_client
+  TickBeginFrame();
+  EXPECT_TRUE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  root_client_.SubmitCompositorFrame(last_begin_frame_args_);
+  EXPECT_TRUE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  embedded_client.SubmitCompositorFrame(last_begin_frame_args_);
+  EXPECT_FALSE(damage_tracker_->HasPendingSurfaces(last_begin_frame_args_));
+
+  manager_.UnregisterFrameSinkHierarchy(kRootFrameSinkId, kChildFrameSinkId);
+}
+
+}  // namespace viz

components/viz/service/display/display_scheduler.h

@@ -15,28 +15,24 @@
 #include "components/viz/common/display/renderer_settings.h"
 #include "components/viz/common/frame_sinks/begin_frame_source.h"
 #include "components/viz/common/surfaces/surface_id.h"
-#include "components/viz/service/surfaces/surface_observer.h"
+#include "components/viz/service/display/display_damage_tracker.h"
 #include "components/viz/service/viz_service_export.h"
 
 namespace viz {
 
 class BeginFrameSource;
-class SurfaceInfo;
 
 class VIZ_SERVICE_EXPORT DisplaySchedulerClient {
  public:
   virtual ~DisplaySchedulerClient() {}
 
   virtual bool DrawAndSwap() = 0;
-  virtual bool SurfaceHasUnackedFrame(const SurfaceId& surface_id) const = 0;
-  virtual bool SurfaceDamaged(const SurfaceId& surface_id,
-                              const BeginFrameAck& ack) = 0;
-  virtual void SurfaceDestroyed(const SurfaceId& surface_id) = 0;
   virtual void DidFinishFrame(const BeginFrameAck& ack) = 0;
 };
 
-class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
-                                            public SurfaceObserver {
+class VIZ_SERVICE_EXPORT DisplayScheduler
+    : public BeginFrameObserverBase,
+      public DisplayDamageTracker::Observer {
  public:
   DisplayScheduler(BeginFrameSource* begin_frame_source,
                    base::SingleThreadTaskRunner* task_runner,
@@ -47,13 +43,10 @@ class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
   int pending_swaps() const { return pending_swaps_; }
 
   void SetClient(DisplaySchedulerClient* client);
+  void SetDamageTracker(DisplayDamageTracker* damage_tracker);
 
   void SetVisible(bool visible);
 
-  // Notifies that the root surface doesn't exist or doesn't have an active
-  // frame and therefore draw is not possible.
-  void SetRootFrameMissing(bool missing);
-
   void ForceImmediateSwapIfPossible();
   void SetNeedsOneBeginFrame();
   base::TimeTicks current_frame_time() const {
@@ -63,11 +56,6 @@ class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
     return current_begin_frame_args_.frame_time +
            current_begin_frame_args_.interval;
   }
-  virtual void DisplayResized();
-  virtual void SetNewRootSurface(const SurfaceId& root_surface_id);
-  virtual void ProcessSurfaceDamage(const SurfaceId& surface_id,
-                                    const BeginFrameAck& ack,
-                                    bool display_damaged);
 
   virtual void DidSwapBuffers();
   void DidReceiveSwapBuffersAck();
@@ -78,16 +66,11 @@ class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
   bool OnBeginFrameDerivedImpl(const BeginFrameArgs& args) override;
   void OnBeginFrameSourcePausedChanged(bool paused) override;
 
-  // SurfaceObserver implementation.
-  void OnFirstSurfaceActivation(const SurfaceInfo& surface_info) override;
-  void OnSurfaceActivated(const SurfaceId& surface_id,
-                          base::Optional<base::TimeDelta> duration) override;
-  void OnSurfaceMarkedForDestruction(const SurfaceId& surface_id) override;
-  bool OnSurfaceDamaged(const SurfaceId& surface_id,
-                        const BeginFrameAck& ack) override;
-  void OnSurfaceDestroyed(const SurfaceId& surface_id) override;
-  void OnSurfaceDamageExpected(const SurfaceId& surface_id,
-                               const BeginFrameArgs& args) override;
+  // DisplayDamageTrackerObserver implementation.
+  void OnDisplayDamaged() override;
+  void OnRootFrameMissing(bool missing) override;
+  void OnPendingSurfacesChanged() override;
+
   void set_needs_draw() { needs_draw_ = true; }
 
  protected:
@@ -131,6 +114,7 @@ class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
   bool UpdateHasPendingSurfaces();
 
   DisplaySchedulerClient* client_;
+  DisplayDamageTracker* damage_tracker_ = nullptr;
   BeginFrameSource* begin_frame_source_;
   base::SingleThreadTaskRunner* task_runner_;
 
@@ -144,11 +128,9 @@ class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
 
   bool visible_;
   bool output_surface_lost_;
-  bool root_frame_missing_;
 
   bool inside_begin_frame_deadline_interval_;
   bool needs_draw_;
-  bool expecting_root_surface_damage_because_of_resize_;
   bool has_pending_surfaces_;
 
   struct SurfaceBeginFrameState {
@@ -164,8 +146,6 @@ class VIZ_SERVICE_EXPORT DisplayScheduler : public BeginFrameObserverBase,
 
   bool observing_begin_frame_source_;
 
-  SurfaceId root_surface_id_;
-
   base::WeakPtrFactory<DisplayScheduler> weak_ptr_factory_{this};
 
  private:

content/browser/compositor/gpu_process_transport_factory.cc

@@ -484,9 +484,9 @@ void GpuProcessTransportFactory::EstablishedGpuChannel(
       GetFrameSinkManager(), data->display.get(), data->display_client.get(),
       context_provider, shared_worker_context_provider(),
       compositor->task_runner(), GetGpuMemoryBufferManager());
+  compositor->SetLayerTreeFrameSink(std::move(layer_tree_frame_sink));
   data->display->Resize(compositor->size());
   data->display->SetOutputIsSecure(data->output_is_secure);
-  compositor->SetLayerTreeFrameSink(std::move(layer_tree_frame_sink));
 }
 
 void GpuProcessTransportFactory::DisableGpuCompositing(