Replacing SurfaceReferenceBase and SequenceSurfaceReference with Closures

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/memory/ptr_util.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(
-      std::unique_ptr<SurfaceReferenceBase> surface_ref,
+      std::unique_ptr<ScopedSurfaceReferenceBase> surface_ref,
       scoped_refptr<base::SingleThreadTaskRunner> main_task_runner)
       : surface_ref_(std::move(surface_ref)),
         main_task_runner_(std::move(main_task_runner)) {}
 
   ~SatisfySwapPromise() override {}
 
  private:
   void DidActivate() override {}
 
   void WillSwap(CompositorFrameMetadata* metadata) override {}
 
   void DidSwap() override {
     main_task_runner_->DeleteSoon(FROM_HERE, surface_ref_.release());
   }
 
   DidNotSwapAction DidNotSwap(DidNotSwapReason reason) override {
     main_task_runner_->DeleteSoon(FROM_HERE, surface_ref_.release());
     return DidNotSwapAction::BREAK_PROMISE;
   }
 
   int64_t TraceId() const override { return 0; }
 
-  std::unique_ptr<SurfaceReferenceBase> surface_ref_;
+  std::unique_ptr<ScopedSurfaceReferenceBase> surface_ref_;
   scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(SatisfySwapPromise);
 };
 
 scoped_refptr<SurfaceLayer> SurfaceLayer::Create(
     scoped_refptr<SurfaceReferenceFactory> ref_factory) {
   return make_scoped_refptr(new SurfaceLayer(std::move(ref_factory)));
 }
 
 SurfaceLayer::SurfaceLayer(scoped_refptr<SurfaceReferenceFactory> ref_factory)
     : ref_factory_(std::move(ref_factory)) {}
 
 SurfaceLayer::~SurfaceLayer() {
   DCHECK(!layer_tree_host());
 }
 
 void SurfaceLayer::SetSurfaceInfo(const SurfaceInfo& surface_info,
                                   bool stretch_content_to_fill_bounds) {
   RemoveCurrentReference();
   surface_info_ = surface_info;
   if (layer_tree_host()) {
     current_ref_ =

[danakj, 2017/01/10 16:46:30]

Everything is using the base types so I don't understand really the point of the
subclasses for sequence and for reference. They seem like internal details of
the factory at this point.

CreateReference could return a base::Closure() to call when you're done with it,
and has the sequence/reference bound into the Closure so it goes back to the
factory, for instance.

Can you help me understand why this class hierarchy is more helpful?

[Fady Samuel, 2017/01/10 18:14:01]

All we care about is that the details of the reference are not part of the
SurfaceLayer, so yes, I think a callback can work here without the class
hierarchy.

         ref_factory_->CreateReference(layer_tree_host(), surface_info_.id());
   }
   stretch_content_to_fill_bounds_ = stretch_content_to_fill_bounds;
   UpdateDrawsContent(HasDrawableContent());
   SetNeedsPushProperties();
 }
 
 std::unique_ptr<LayerImpl> SurfaceLayer::CreateLayerImpl(
     LayerTreeImpl* tree_impl) {
   return SurfaceLayerImpl::Create(tree_impl, id());
@@ skipping 27 matching lines @@
 void SurfaceLayer::RemoveCurrentReference() {
   if (!current_ref_)
     return;
   auto swap_promise = base::MakeUnique<SatisfySwapPromise>(
       std::move(current_ref_), base::ThreadTaskRunnerHandle::Get());
   layer_tree_host()->GetSwapPromiseManager()->QueueSwapPromise(
       std::move(swap_promise));
 }
 
 }  // namespace cc