Revert "Android OOP-D: DisplayScheduler should only report draw-inducing damage"

This reverts commit 60d359fd.

Reason for revert: This logic could still be flaky (what if we go invisible after deferring commits), and isn't needed after https://chromium-review.googlesource.com/c/chromium/src/+/1184376, so let's revert.

Original change's description:
> Android OOP-D: DisplayScheduler should only report draw-inducing damage
> 
> Currently DisplayScheduler reports damage to SurfaceManager in any case
> where a damaged surface ID damages the display. While these cases will
> trigger a draw when visible, they don't necessarily trigger a draw
> in any defined timeframe. This can lead to issues. Consider:
> 
> 1) Display 1 is not visible, embeds surface A.
> 2) Surface A sends CF 1, we don't immediately ack as we will wait for
>    Display 1 to draw.
> 3) Display 2 becomes visible, embeds surface A with a new surface ID.
> 4) Display 2 will never update, as no new frames will be produced for
>    surface A as the renderer is continually waiting on the ack for
>    CF 1.
> 
> By having DisplayScheduler report no damage if invisible, we always
> ack frames in a timely manner. This may allow a renderer to produce
> some extra frames when its display is invisible, but this should be
> limited as the renderer itself receives visibility notifications and
> won't get additional BeginFrames once visibility is fully processed.
> 
> Bug: 875895
> Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
> Change-Id: I272de950d8e4d4c819c445dc8162ac360afb80a1
> Reviewed-on: https://chromium-review.googlesource.com/1182862
> Reviewed-by: Fady Samuel <fsamuel@chromium.org>
> Commit-Queue: Eric Karl <ericrk@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#584917}

TBR=fsamuel@chromium.org,ericrk@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 875895
Change-Id: Idefad0b80d08b3c9fecf655b27b3c6ffb85eca10
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel
Reviewed-on: https://chromium-review.googlesource.com/1232335Reviewed-by: Eric Karl <ericrk@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593003}

components/viz/service/display/display_scheduler.cc

@@ -329,11 +329,7 @@ bool DisplayScheduler::OnSurfaceDamaged(const SurfaceId& surface_id,
   bool damaged = client_->SurfaceDamaged(surface_id, ack);
   ProcessSurfaceDamage(surface_id, ack, damaged);
 
-  // If we are not visible, return false. We may never draw in this
-  // state, and other displays may have embedded this surface without
-  // damage. Those displays shouldn't have their frame production
-  // blocked by waiting for this ack.
-  return damaged && visible_;
+  return damaged;
 }
 
 void DisplayScheduler::OnSurfaceDiscarded(const SurfaceId& surface_id) {

components/viz/service/display/display_scheduler_unittest.cc

@@ -48,8 +48,7 @@ class FakeDisplaySchedulerClient : public DisplaySchedulerClient {
 
   bool SurfaceDamaged(const SurfaceId& surface_id,
                       const BeginFrameAck& ack) override {
-    undrawn_surfaces_.insert(surface_id);
-    return true;
+    return false;
   }
 
   void SurfaceDiscarded(const SurfaceId& surface_id) override {}
@@ -62,13 +61,16 @@ class FakeDisplaySchedulerClient : public DisplaySchedulerClient {
 
   void SetNextDrawAndSwapFails() { next_draw_and_swap_fails_ = true; }
 
+  void SurfaceDamaged(const SurfaceId& surface_id) {
+    undrawn_surfaces_.insert(surface_id);
+  }
+
   const BeginFrameAck& last_begin_frame_ack() { return last_begin_frame_ack_; }
 
  protected:
   int draw_and_swap_count_;
   bool next_draw_and_swap_fails_;
   std::set<SurfaceId> undrawn_surfaces_;
-  std::set<SurfaceId> non_damaging_surfaces_;
   BeginFrameAck last_begin_frame_ack_;
 };
 
@@ -112,8 +114,6 @@ class TestDisplayScheduler : public DisplayScheduler {
 
   bool has_pending_surfaces() { return has_pending_surfaces_; }
 
-  bool is_visible() const { return visible_; }
-
  protected:
   int scheduler_begin_frame_deadline_count_;
 };
@@ -152,11 +152,9 @@ class DisplaySchedulerTest : public testing::Test {
   }
 
   void SurfaceDamaged(const SurfaceId& surface_id) {
-    // While our fake client always returns true for damage, OnSurfaceDamaged
-    // should only return true if we are visible.
-    EXPECT_EQ(
-        scheduler_.is_visible(),
-        scheduler_.OnSurfaceDamaged(surface_id, AckForCurrentBeginFrame()));
+    client_.SurfaceDamaged(surface_id);
+    scheduler_.ProcessSurfaceDamage(surface_id, AckForCurrentBeginFrame(),
+                                    true);
   }
 
  protected:

components/viz/service/surfaces/surface_observer.h

@@ -40,8 +40,7 @@ class SurfaceObserver {
   // in response to a BeginFrame, or a CopyOutputRequest is issued.
   //
   // |ack.sequence_number| is only valid if called in response to a BeginFrame.
-  // Should return true if this causes a Display to be damaged resulting in a
-  // draw.
+  // Should return true if this causes a Display to be damaged.
   virtual bool OnSurfaceDamaged(const SurfaceId& surface_id,
                                 const BeginFrameAck& ack) = 0;