Replace CompositorFrameSinkSupport::WillDrawSurface With RepeatingCallback

cc/surfaces/surface.cc

Index: cc/surfaces/surface.cc
diff --git a/cc/surfaces/surface.cc b/cc/surfaces/surface.cc
index 80d5a7ab32587d9b62b8c34e0e91c51ba4d1c394..b6ddeb412b106b42c7ed7d8c093960821ae139ed 100644
--- a/cc/surfaces/surface.cc
+++ b/cc/surfaces/surface.cc
@@ -50,7 +50,9 @@ void Surface::SetPreviousFrameSurface(Surface* surface) {
   surface->TakeLatencyInfoFromPendingFrame(&frame.metadata.latency_info);
 }
 
-void Surface::QueueFrame(CompositorFrame frame, const DrawCallback& callback) {
+void Surface::QueueFrame(CompositorFrame frame,
+                         const DrawCallback& callback,
+                         const WillDrawCallback& will_draw_callback) {
   TakeLatencyInfoFromPendingFrame(&frame.metadata.latency_info);
 
   base::Optional<FrameData> previous_pending_frame_data =
@@ -66,13 +68,14 @@ void Surface::QueueFrame(CompositorFrame frame, const DrawCallback& callback) {
   bool is_pending_frame = !blocking_surfaces_.empty();
 
   if (is_pending_frame) {
-    pending_frame_data_ = FrameData(std::move(frame), callback);
+    pending_frame_data_ =
+        FrameData(std::move(frame), callback, will_draw_callback);
     // Ask the surface manager to inform |this| when its dependencies are
     // resolved.
     factory_->manager()->RequestSurfaceResolution(this);
   } else {
     // If there are no blockers, then immediately activate the frame.
-    ActivateFrame(FrameData(std::move(frame), callback));
+    ActivateFrame(FrameData(std::move(frame), callback, will_draw_callback));
   }
 
   // Returns resources for the previous pending frame.
@@ -80,7 +83,7 @@ void Surface::QueueFrame(CompositorFrame frame, const DrawCallback& callback) {
 }
 
 void Surface::EvictFrame() {
-  QueueFrame(CompositorFrame(), DrawCallback());
+  QueueFrame(CompositorFrame(), DrawCallback(), WillDrawCallback());
   active_frame_data_.reset();
 }
 
@@ -143,8 +146,11 @@ void Surface::ActivatePendingFrameForDeadline() {
 }
 
 Surface::FrameData::FrameData(CompositorFrame&& frame,
-                              const DrawCallback& draw_callback)
-    : frame(std::move(frame)), draw_callback(draw_callback) {}
+                              const DrawCallback& draw_callback,
+                              const WillDrawCallback& will_draw_callback)
+    : frame(std::move(frame)),
+      draw_callback(draw_callback),
+      will_draw_callback(will_draw_callback) {}
 
 Surface::FrameData::FrameData(FrameData&& other) = default;
 
@@ -282,6 +288,13 @@ void Surface::RunDrawCallbacks() {
   }
 }
 
+void Surface::RunWillDrawCallbacks(const gfx::Rect& damage_rect) {

[Fady Samuel, 2017/04/20 12:45:48]

I'm really not sure why this is plural. RunWillDrawCallback is more appropriate.

Also, this code below is a bit harder to read (I know we do this elsewhere and I
didn't update the code:

if (!active_frame_data_ || active_frame_data_->will_draw_callback.is_null())
  return;

active_frame_data_->will_draw_callback.Run(surface_id_.local_surface_id(),
                                                                          
damage_rect);

This is maybe a bit easier to read.

[danakj, 2017/04/20 13:56:45]

+1 to RunWillDrawCallback. Early out is fine, but also doesn't seem important
for a one-line function. It gains more value when there's more nesting or longer
logic.

[Alex Z., 2017/04/20 13:58:05]

Done. I got the plural form RunDrawCallbacks. I fixed both names.

+  if (active_frame_data_ && !active_frame_data_->will_draw_callback.is_null()) {
+    active_frame_data_->will_draw_callback.Run(surface_id_.local_surface_id(),
+                                               damage_rect);
+  }
+}
+
 void Surface::AddDestructionDependency(SurfaceSequence sequence) {
   destruction_dependencies_.push_back(sequence);
 }