Replace CompositorFrameSinkSupport::WillDrawSurface With RepeatingCallback

cc/surfaces/surface.cc

 // Copyright 2014 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 "cc/surfaces/surface.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 32 matching lines @@
 void Surface::SetPreviousFrameSurface(Surface* surface) {
   DCHECK(surface && (HasActiveFrame() || HasPendingFrame()));
   frame_index_ = surface->frame_index() + 1;
   previous_frame_surface_id_ = surface->surface_id();
   CompositorFrame& frame = active_frame_data_ ? active_frame_data_->frame
                                               : pending_frame_data_->frame;
   surface->TakeLatencyInfo(&frame.metadata.latency_info);
   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 =
       std::move(pending_frame_data_);
   pending_frame_data_.reset();
 
   UpdateBlockingSurfaces(previous_pending_frame_data.has_value(), frame);
 
   // Receive and track the resources referenced from the CompositorFrame
   // regardless of whether it's pending or active.
   factory_->ReceiveFromChild(frame.resource_list);
 
   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.
   UnrefFrameResourcesAndRunDrawCallback(std::move(previous_pending_frame_data));
 }
 
 void Surface::EvictFrame() {
-  QueueFrame(CompositorFrame(), DrawCallback());
+  QueueFrame(CompositorFrame(), DrawCallback(), WillDrawCallback());
   active_frame_data_.reset();
 }
 
 void Surface::RequestCopyOfOutput(
     std::unique_ptr<CopyOutputRequest> copy_request) {
   if (!active_frame_data_ ||
       active_frame_data_->frame.render_pass_list.empty()) {
     copy_request->SendEmptyResult();
     return;
   }
@@ skipping 42 matching lines @@
     return;
   }
 
   // If a frame is being activated because of a deadline, then clear its set
   // of blockers.
   blocking_surfaces_.clear();
   ActivatePendingFrame();
 }
 
 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;
 
 Surface::FrameData& Surface::FrameData::operator=(FrameData&& other) = default;
 
 Surface::FrameData::~FrameData() = default;
 
 void Surface::ActivatePendingFrame() {
   DCHECK(pending_frame_data_);
   ActivateFrame(std::move(*pending_frame_data_));
@@ skipping 117 matching lines @@
 }
 
 void Surface::RunDrawCallbacks() {
   if (active_frame_data_ && !active_frame_data_->draw_callback.is_null()) {
     DrawCallback callback = active_frame_data_->draw_callback;
     active_frame_data_->draw_callback = DrawCallback();
     callback.Run();
   }
 }
 
+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);
 }
 
 void Surface::SatisfyDestructionDependencies(
     std::unordered_set<SurfaceSequence, SurfaceSequenceHash>* sequences,
     std::unordered_set<FrameSinkId, FrameSinkIdHash>* valid_frame_sink_ids) {
   base::EraseIf(destruction_dependencies_,
                 [sequences, valid_frame_sink_ids](SurfaceSequence seq) {
                   return (!!sequences->erase(seq) ||
@@ skipping 42 matching lines @@
     frame->metadata.latency_info.swap(*latency_info);
     return;
   }
   std::copy(frame->metadata.latency_info.begin(),
             frame->metadata.latency_info.end(),
             std::back_inserter(*latency_info));
   frame->metadata.latency_info.clear();
 }
 
 }  // namespace cc