cc: Consider Surface active if frame received recently

cc/surfaces/display_scheduler.cc

Index: cc/surfaces/display_scheduler.cc
diff --git a/cc/surfaces/display_scheduler.cc b/cc/surfaces/display_scheduler.cc
index 98f52e52b60ca4c53a846032eb57eaf77017ef1e..f454fac0f4901af7f9370170ef570f8d304b4ea3 100644
--- a/cc/surfaces/display_scheduler.cc
+++ b/cc/surfaces/display_scheduler.cc
@@ -96,6 +96,24 @@ void DisplayScheduler::OutputSurfaceLost() {
   ScheduleBeginFrameDeadline();
 }
 
+void DisplayScheduler::UpdateSurfaceDamageExpectations() {
+  expect_damage_from_root_surface_ = root_surface_damaged_;

[sunnyps, 2015/07/23 03:02:40]

Set root_surface_damaged_ to false here instead of below.

[brianderson, 2015/07/23 18:21:46]

This will work for the case where this method is called on line 126, however it
would be incorrect for the call on 277, which is why I kept it out.

+
+  std::vector<SurfaceId> partial_union =
+      base::STLSetUnion<std::vector<SurfaceId>>(
+          child_surface_ids_damaged_, child_surface_ids_damaged_prev_);
+  child_surface_ids_to_expect_damage_from_ =
+      base::STLSetUnion<std::vector<SurfaceId>>(
+          partial_union, child_surface_ids_damaged_prev_prev_);
+
+  child_surface_ids_damaged_prev_prev_.swap(child_surface_ids_damaged_prev_);
+  child_surface_ids_damaged_prev_.swap(child_surface_ids_damaged_);
+  child_surface_ids_damaged_.clear();
+
+  all_active_child_surfaces_ready_to_draw_ =
+      child_surface_ids_to_expect_damage_from_.empty();
+}
+
 void DisplayScheduler::DrawAndSwap() {
   TRACE_EVENT0("cc", "DisplayScheduler::DrawAndSwap");
   DCHECK_LT(pending_swaps_, max_pending_swaps_);
@@ -105,19 +123,11 @@ void DisplayScheduler::DrawAndSwap() {
   if (!success)
     return;
 
-  child_surface_ids_to_expect_damage_from_ =
-      base::STLSetIntersection<std::vector<SurfaceId>>(
-          child_surface_ids_damaged_, child_surface_ids_damaged_prev_);
-
-  child_surface_ids_damaged_prev_.swap(child_surface_ids_damaged_);
-  child_surface_ids_damaged_.clear();
+  UpdateSurfaceDamageExpectations();
 
+  // Update state regarding what needs to be drawn.
   needs_draw_ = false;
   entire_display_damaged_ = false;
-  all_active_child_surfaces_ready_to_draw_ =
-      child_surface_ids_to_expect_damage_from_.empty();
-
-  expect_damage_from_root_surface_ = root_surface_damaged_;
   root_surface_damaged_ = false;
 }
 
@@ -249,12 +259,27 @@ void DisplayScheduler::AttemptDrawAndSwap() {
   begin_frame_deadline_task_.Cancel();
   begin_frame_deadline_task_time_ = base::TimeTicks();
 
-  if (needs_draw_ && !output_surface_lost_) {
-    if (pending_swaps_ < max_pending_swaps_ && !root_surface_resources_locked_)
-      DrawAndSwap();
+  bool stay_active = !output_surface_lost_ &&
+                     (needs_draw_ || expect_damage_from_root_surface_ ||
+                      child_surface_ids_to_expect_damage_from_.size() != 0);

[sunnyps, 2015/07/23 03:02:40]

Can you use all_active_child_surfaces_ready_to_draw_ here?

[brianderson, 2015/07/23 18:21:46]

I think that would work, however
"child_surface_ids_to_expect_damage_from_.size() != 0" expresses the intent more
directly and is more orthogonal to the other conditions since
all_active_child_surfaces_ready_to_draw_ has some overlap in meaning with
needs_draw_.

I should probably change this to the following though:
!child_surface_ids_to_expect_damage_from_.empty()

+
+  if (stay_active) {
+    if (needs_draw_) {
+      if (pending_swaps_ < max_pending_swaps_ &&
+          !root_surface_resources_locked_)
+        DrawAndSwap();
+    } else {
+      // In order to properly go idle, make sure to update expectations that
+      // will cause stay_active to go false even if we have nothing to draw.

[sunnyps, 2015/07/23 03:02:40]

nit: "stay_active" instead of "stay active"

[brianderson, 2015/07/23 18:21:46]

Looks like it's already stay_active, but I'll add |'s around it.

+      // Verify no child surfaces are currently damaged, since
+      // UpdateSurfaceDamageExpectatations will clear that list.
+      DCHECK(child_surface_ids_damaged_.empty());

[sunnyps, 2015/07/23 03:02:40]

Can probably add more DCHECKs here - like DCHECK(!entire_display_damaged_) etc.

[brianderson, 2015/07/23 18:21:46]

Sounds good. I will added a DCHECK(!root_surface_damaged_) too.

+      UpdateSurfaceDamageExpectations();
+    }
   } else {
     // We are going idle, so reset expectations.
     child_surface_ids_to_expect_damage_from_.clear();
+    child_surface_ids_damaged_prev_prev_.clear();
     child_surface_ids_damaged_prev_.clear();
     child_surface_ids_damaged_.clear();
     all_active_child_surfaces_ready_to_draw_ = true;