Getting rid of CompositorFrameMetadata::satisfies_sequences

cc/layers/surface_layer.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/layers/surface_layer.h"
 
 #include <stdint.h>
 
 #include "base/macros.h"
+#include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "cc/layers/surface_layer_impl.h"
 #include "cc/output/swap_promise.h"
 #include "cc/surfaces/surface_sequence_generator.h"
 #include "cc/trees/layer_tree_host.h"
 #include "cc/trees/swap_promise_manager.h"
 
 namespace cc {
 
 class SatisfySwapPromise : public SwapPromise {
  public:
-  SatisfySwapPromise(SurfaceSequence sequence,
-                     const SurfaceLayer::SatisfyCallback& satisfy_callback)
-      : sequence_(sequence), satisfy_callback_(satisfy_callback) {}
+  SatisfySwapPromise(
+      SurfaceSequence sequence,
+      const SurfaceLayer::SatisfyCallback& satisfy_callback,
+      scoped_refptr<base::SingleThreadTaskRunner> main_task_runner)
+      : sequence_(sequence),
+        satisfy_callback_(satisfy_callback),
+        main_task_runner_(main_task_runner) {}
 
   ~SatisfySwapPromise() override {}
 
  private:
   void DidActivate() override {}
   void DidSwap(CompositorFrameMetadata* metadata) override {
-    metadata->satisfies_sequences.push_back(sequence_.sequence);

[Fady Samuel, 2016/12/08 22:21:17]

DCHECK(main_task_runner_->BelongsToCurrentThread()); would be helpful here to
clarify.

+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(satisfy_callback_, sequence_));
   }
 
   DidNotSwapAction DidNotSwap(DidNotSwapReason reason) override {
     satisfy_callback_.Run(sequence_);

[danakj, 2016/12/08 21:39:19]

I can't help but notice this is running the callback directly without posting
it. From what I can tell this works because the only user in the renderer (where
the compositor is threaded) is canvas which only sends something over mojo IPC
when it runs this callback. But I think you should be consistent here, either
both run directly or both post. Perhaps both run directly is fine for now..

     return DidNotSwapAction::BREAK_PROMISE;
   }
   int64_t TraceId() const override { return 0; }
 
   SurfaceSequence sequence_;
   SurfaceLayer::SatisfyCallback satisfy_callback_;
+  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(SatisfySwapPromise);
 };
 
 scoped_refptr<SurfaceLayer> SurfaceLayer::Create(
     const SatisfyCallback& satisfy_callback,
     const RequireCallback& require_callback) {
   return make_scoped_refptr(
       new SurfaceLayer(satisfy_callback, require_callback));
 }
@@ skipping 59 matching lines @@
                             ->GetSurfaceSequenceGenerator()
                             ->CreateSurfaceSequence();
     require_callback_.Run(surface_id_, destroy_sequence_);
   }
 }
 
 void SurfaceLayer::SatisfyDestroySequence() {
   if (!layer_tree_host())
     return;
   DCHECK(destroy_sequence_.is_valid());
   std::unique_ptr<SatisfySwapPromise> satisfy(

[danakj, 2016/12/08 21:39:19]

can u change this to auto and base::MakeUnique while u'r changing it

-      new SatisfySwapPromise(destroy_sequence_, satisfy_callback_));
+      new SatisfySwapPromise(destroy_sequence_, satisfy_callback_,
+                             base::ThreadTaskRunnerHandle::Get()));
   layer_tree_host()->GetSwapPromiseManager()->QueueSwapPromise(
       std::move(satisfy));
   destroy_sequence_ = SurfaceSequence();
 }
 
 }  // namespace cc