[viz] Ignore stray BeginFrameAcks in DisplayScheduler.

The DisplayScheduler's surface state tracking mechanism was susceptible
to overriding a surface's state with an older sequence number, which
could cause infinite blocking in wait_for_all_surfaces_before_draw mode.

Bug: 697086
Change-Id: Ife6ea5df9ccef729df2a17ff3f199ee5784c880b
Reviewed-on: https://chromium-review.googlesource.com/700554
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506700}

components/viz/service/display/display_scheduler.cc

@@ -119,10 +119,17 @@ void DisplayScheduler::ProcessSurfaceDamage(const SurfaceId& surface_id,
   bool valid_ack = ack.sequence_number != BeginFrameArgs::kInvalidFrameNumber;
   if (valid_ack) {
     auto it = surface_states_.find(surface_id);
-    if (it != surface_states_.end())
+    // 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
+    } else {
       valid_ack = false;
+    }
   }
 
   bool pending_surfaces_changed = false;

components/viz/service/display/display_scheduler_unittest.cc

@@ -418,6 +418,15 @@ TEST_F(DisplaySchedulerWaitForAllSurfacesTest, WaitForAllSurfacesBeforeDraw) {
   ack = AckForCurrentBeginFrame();
   ack.has_damage = false;
   scheduler_.ProcessSurfaceDamage(sid1, ack, false);
+  EXPECT_GE(now_src().NowTicks(),
+            scheduler_.DesiredBeginFrameDeadlineTimeForTest());
+  // Stray BeginFrameAcks for older BeginFrames are ignored.
+  ack.sequence_number--;
+  scheduler_.ProcessSurfaceDamage(sid1, ack, false);
+  // If the acknowledgment above was not ignored and instead updated the surface
+  // state for sid1, the surface would become a pending surface again, and the
+  // deadline would no longer be immediate. Since it is ignored, we are
+  // expecting the deadline to remain immedate.
   EXPECT_GE(now_src().NowTicks(),
             scheduler_.DesiredBeginFrameDeadlineTimeForTest());
   scheduler_.BeginFrameDeadlineForTest();