exo: Cleanup callback handling.

This makes us acquire callbacks before frame submission instead
of at commit time.

This makes us handle callbacks a bit better when a surface is
committed without having a STH.

Bug: 
Test: exo_unittests
Change-Id: I06aa0914d42c4a9dbfab8e291acd7d051b744de0
Reviewed-on: https://chromium-review.googlesource.com/761558Reviewed-by: Peng Huang <penghuang@chromium.org>
Commit-Queue: David Reveman <reveman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515289}

components/exo/surface.cc

@@ -208,12 +208,15 @@ Surface::~Surface() {
 
   // Call all frame callbacks with a null frame time to indicate that they
   // have been cancelled.
-  for (const auto& frame_callback : pending_frame_callbacks_)
+  frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_);
+  for (const auto& frame_callback : frame_callbacks_)
     frame_callback.Run(base::TimeTicks());
 
   // Call all presentation callbacks with a null presentation time to indicate
   // that they have been cancelled.
-  for (const auto& presentation_callback : pending_presentation_callbacks_)
+  presentation_callbacks_.splice(presentation_callbacks_.end(),
+                                 pending_presentation_callbacks_);
+  for (const auto& presentation_callback : presentation_callbacks_)
     presentation_callback.Run(base::TimeTicks(), base::TimeDelta(), 0);
 
   WMHelper::GetInstance()->ResetDragDropDelegate(window_.get());
@@ -435,13 +438,10 @@ void Surface::Commit() {
   if (delegate_)
     delegate_->OnSurfaceCommit();
   else
-    CommitSurfaceHierarchy(nullptr, nullptr, false);
+    CommitSurfaceHierarchy(false);
 }
 
-void Surface::CommitSurfaceHierarchy(
-    std::list<FrameCallback>* frame_callbacks,
-    std::list<PresentationCallback>* presentation_callbacks,
-    bool synchronized) {
+void Surface::CommitSurfaceHierarchy(bool synchronized) {
   if (needs_commit_surface_ && (synchronized || !IsSynchronized())) {
     needs_commit_surface_ = false;
     synchronized = true;
@@ -489,14 +489,13 @@ void Surface::CommitSurfaceHierarchy(
     if (needs_update_buffer_transform)
       UpdateBufferTransform();
 
-    // Move pending frame callbacks to the end of frame_callbacks.
-    if (frame_callbacks)
-      frame_callbacks->splice(frame_callbacks->end(), pending_frame_callbacks_);
+    // Move pending frame callbacks to the end of |frame_callbacks_|.
+    frame_callbacks_.splice(frame_callbacks_.end(), pending_frame_callbacks_);
 
-    // Move pending presentation callbacks to the end of presentation_callbacks.
-    if (presentation_callbacks)
-      presentation_callbacks->splice(presentation_callbacks->end(),
-                                     pending_presentation_callbacks_);
+    // Move pending presentation callbacks to the end of
+    // |presentation_callbacks_|.
+    presentation_callbacks_.splice(presentation_callbacks_.end(),
+                                   pending_presentation_callbacks_);
 
     UpdateContentSize();
 
@@ -537,21 +536,34 @@ void Surface::CommitSurfaceHierarchy(
 
   surface_hierarchy_content_bounds_ = gfx::Rect(content_size_);
 
-  // The top most sub-surface is at the front of the RenderPass's quad_list,
-  // so we need composite sub-surface in reversed order.
   for (const auto& sub_surface_entry : base::Reversed(sub_surfaces_)) {
     auto* sub_surface = sub_surface_entry.first;
     gfx::Point origin = sub_surface_entry.second;
     // Synchronously commit all pending state of the sub-surface and its
     // descendants.
-    sub_surface->CommitSurfaceHierarchy(frame_callbacks, presentation_callbacks,
-                                        synchronized);
+    sub_surface->CommitSurfaceHierarchy(synchronized);
     surface_hierarchy_content_bounds_.Union(
         sub_surface->surface_hierarchy_content_bounds() +
         origin.OffsetFromOrigin());
   }
 }
 
+void Surface::AppendSurfaceHierarchyCallbacks(
+    std::list<FrameCallback>* frame_callbacks,
+    std::list<PresentationCallback>* presentation_callbacks) {
+  // Move frame callbacks to the end of |frame_callbacks|.
+  frame_callbacks->splice(frame_callbacks->end(), frame_callbacks_);
+  // Move presentation callbacks to the end of |presentation_callbacks|.
+  presentation_callbacks->splice(presentation_callbacks->end(),
+                                 presentation_callbacks_);
+
+  for (const auto& sub_surface_entry : base::Reversed(sub_surfaces_)) {
+    auto* sub_surface = sub_surface_entry.first;
+    sub_surface->AppendSurfaceHierarchyCallbacks(frame_callbacks,
+                                                 presentation_callbacks);
+  }
+}
+
 void Surface::AppendSurfaceHierarchyContentsToFrame(
     const gfx::Point& origin,
     float device_scale_factor,

components/exo/surface.h

@@ -149,11 +149,15 @@ class Surface final : public ui::PropertyHandler {
   // recursively calling CommitSurfaceHierarchy() for each sub-surface.
   // If |synchronized| is set to false, then synchronized surfaces should not
   // commit pending state.
-  void CommitSurfaceHierarchy(
+  void CommitSurfaceHierarchy(bool synchronized);
+
+  // This will append current callbacks for surface and its descendants to
+  // |frame_callbacks| and |presentation_callbacks|.
+  void AppendSurfaceHierarchyCallbacks(
       std::list<FrameCallback>* frame_callbacks,
-      std::list<PresentationCallback>* presentation_callbacks,
-      bool synchronized);
+      std::list<PresentationCallback>* presentation_callbacks);
 
+  // This will append contents for surface and its descendants to frame.
   void AppendSurfaceHierarchyContentsToFrame(
       const gfx::Point& origin,
       float device_scale_factor,
@@ -326,6 +330,7 @@ class Surface final : public ui::PropertyHandler {
   // |active_frame_callbacks_| when the effect of the Commit() is scheduled to
   // be drawn. They fire at the first begin frame notification after this.
   std::list<FrameCallback> pending_frame_callbacks_;
+  std::list<FrameCallback> frame_callbacks_;
 
   // These lists contains the callbacks to notify the client when surface
   // contents have been presented. These callbacks move to
@@ -335,6 +340,7 @@ class Surface final : public ui::PropertyHandler {
   // after receiving VSync parameters update for the previous frame. They fire
   // at the next VSync parameters update after that.
   std::list<PresentationCallback> pending_presentation_callbacks_;
+  std::list<PresentationCallback> presentation_callbacks_;
 
   // This is the state that has yet to be committed.
   State pending_state_;

components/exo/surface_tree_host.cc

@@ -211,8 +211,7 @@ void SurfaceTreeHost::UpdateNeedsBeginFrame() {
 
 void SurfaceTreeHost::OnSurfaceCommit() {
   DCHECK(presentation_callbacks_.empty());
-  root_surface_->CommitSurfaceHierarchy(&frame_callbacks_,
-                                        &presentation_callbacks_, false);
+  root_surface_->CommitSurfaceHierarchy(false);
   UpdateHostWindowBounds();
 }
 
@@ -272,6 +271,8 @@ void SurfaceTreeHost::SubmitCompositorFrame() {
     current_begin_frame_ack_.has_damage = true;
   }
   frame.metadata.begin_frame_ack = current_begin_frame_ack_;
+  root_surface_->AppendSurfaceHierarchyCallbacks(&frame_callbacks_,
+                                                 &presentation_callbacks_);
   if (!presentation_callbacks_.empty()) {
     // If overflow happens, we increase it again.
     if (!++presentation_token_)