Introducing SurfaceReferenceFactory

cc/surfaces/surface_ref_base.h

Index: cc/surfaces/surface_ref_base.h
diff --git a/cc/surfaces/surface_ref_base.h b/cc/surfaces/surface_ref_base.h
new file mode 100644
index 0000000000000000000000000000000000000000..5e567699b8d634edc67d8d2ffea0933d0c8dccb4
--- /dev/null
+++ b/cc/surfaces/surface_ref_base.h
@@ -0,0 +1,106 @@
+// Copyright (c) 2016 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.
+
+#ifndef CC_SURFACES_SURFACE_REF_BASE_H_
+#define CC_SURFACES_SURFACE_REF_BASE_H_
+
+#include <memory>
+
+#include "cc/surfaces/surface_id.h"
+#include "cc/surfaces/surface_sequence.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace cc {
+
+class LayerTreeHost;
+class SurfaceRefInternal;
+class CompositorFrameMetadata;
+typedef std::unique_ptr<SurfaceRefInternal> SurfaceRefPtr;
+template <class T>
+class SurfaceRefBase;
+
+class SurfaceRefInternal {

[Fady Samuel, 2016/11/30 19:51:29]

kylechar@ and I were chatting. My current thinking is what you built really is a
SurfaceEmbedding that has "SurfaceInfo" and SurfaceSequence/SurfaceReference.

+ public:
+  virtual SurfaceRefPtr Clone() const = 0;
+
+  ~SurfaceRefInternal();
+
+  SurfaceRefInternal* SetId(const SurfaceId& id) {

[Fady Samuel, 2016/11/30 19:51:29]

Can we get rid of these setters? I'd prefer if id, scale, and size are all
const.

+    DCHECK(!is_active_);
+    id_ = id;
+    return this;
+  }
+  SurfaceRefInternal* SetScale(float scale) {
+    DCHECK(!is_active_);
+    scale_ = scale;
+    return this;
+  }
+  SurfaceRefInternal* SetSize(const gfx::Size& size) {
+    DCHECK(!is_active_);
+    size_ = size;
+    return this;
+  }
+  void SetHost(LayerTreeHost* host) {

[Fady Samuel, 2016/11/30 19:51:29]

I'd prefer if the base class didn't know about LayerTreeHost. Not all Mus
clients will have a LayerTreeHost.

+    DCHECK(!is_active_);
+    host_ = host;
+  }
+  void AllowSurfaceDestruction() {

[Fady Samuel, 2016/11/30 19:51:29]

I'm super confused about these things. What does AllowSurfaceDestruction, and
BlockSurfaceDestruction do and why do we ned them?

+    if (!is_active_)
+      return;
+    DCHECK(id_.is_valid());
+    AllowImpl();
+    is_active_ = false;
+  }
+  void BlockSurfaceDestruction() {
+    if (is_active_)
+      return;
+    BlockImpl();
+    is_active_ = true;
+  }
+  bool is_active() { return is_active_; }

[Fady Samuel, 2016/11/30 19:51:29]

What does this mean?

+
+  void SetMetadata(const SurfaceId& id, float scale, const gfx::Size& size) {

[Fady Samuel, 2016/11/30 19:51:29]

Do these quantities change? Maybe make this in the constructor's initializer
list instead?

+    SetId(id);
+    SetScale(scale);
+    SetSize(size);
+  }
+
+  void DelegateLock(CompositorFrameMetadata* metadata) {
+    DelegateLockImpl(metadata);
+    is_active_ = false;
+  }
+
+  const SurfaceId& id() const { return id_; }
+  float scale() const { return scale_; }
+  const gfx::Size& size() const { return size_; }
+  LayerTreeHost* host() const { return host_; }

[Fady Samuel, 2016/11/30 19:51:29]

SurfaceRef/SurfaceEmbedding shouldn't know about LayerTreeHost.

+
+ protected:
+  virtual void AllowImpl() = 0;
+  virtual void BlockImpl() = 0;
+  virtual void DelegateLockImpl(CompositorFrameMetadata*) = 0;
+
+ private:
+  SurfaceId id_;
+  float scale_ = 1.f;
+  gfx::Size size_;
+  bool is_active_ = false;

[Saman Sami, 2016/11/30 18:05:52]

rename?

+  LayerTreeHost* host_ = nullptr;
+  template <class U>
+  friend class SurfaceRefBase;
+};
+
+template <class T>
+class SurfaceRefBase : public SurfaceRefInternal {
+ public:
+  SurfaceRefPtr Clone() const override {
+    SurfaceRefPtr res(new T(*(T*)this));

[Fady Samuel, 2016/11/30 19:51:29]

Use static_cast

+    res->is_active_ = false;
+    return res;
+  }
+};
+
+}  // namespace cc
+
+#endif  // CC_SURFACES_SURFACE_REF_BASE_H_