Introducing SurfaceReferenceFactory

cc/surfaces/surface_embedding_base.h

Index: cc/surfaces/surface_embedding_base.h
diff --git a/cc/surfaces/surface_embedding_base.h b/cc/surfaces/surface_embedding_base.h
new file mode 100644
index 0000000000000000000000000000000000000000..0533c3ff441618f3d6ae371c574a45f4c0f87b7d
--- /dev/null
+++ b/cc/surfaces/surface_embedding_base.h
@@ -0,0 +1,89 @@
+// 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_EMBEDDING_BASE_H_
+#define CC_SURFACES_SURFACE_EMBEDDING_BASE_H_
+
+#include <memory>
+
+#include "base/memory/ptr_util.h"
+#include "base/memory/ref_counted.h"
+#include "cc/surfaces/surface_id.h"
+#include "cc/surfaces/surface_reference_owner.h"
+#include "cc/surfaces/surface_sequence.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace cc {
+
+struct SurfaceInfo {

[Fady Samuel, 2016/12/12 16:12:01]

Move to a separate file.

+  SurfaceId id;
+  float scale = 1.f;
+  gfx::Size size;
+
+  SurfaceInfo() {}

[Fady Samuel, 2016/12/12 16:12:00]

SurfaceInfo() = default;

+  SurfaceInfo(const SurfaceId& id, float scale, gfx::Size size)
+      : id(id), scale(scale), size(size) {}
+  bool operator==(const SurfaceInfo& info) const {

[Fady Samuel, 2016/12/12 16:12:00]

Implement !=

+    return id == info.id && scale == info.scale && size == info.size;
+  }
+};
+
+class CompositorFrameMetadata;
+class SurfaceEmbedding;
+typedef scoped_refptr<SurfaceEmbedding> SurfaceEmbeddingPtr;

[Fady Samuel, 2016/12/12 16:12:00]

This typedef isn't adding value. Please be explicit.

+
+/* The base class for the references returned by SurfaceReferenceFactory and
+ * its subclasses.
+ * The objects of this class hold on to their corresponding surface reference
+ * until destroy is called or until the object goes out of scope.
+ * To keep things as lightweight as possible, the base class only keeps a
+ * pointer to the factory and it's left up to the subclasses to decide what
+ * other information they need to keep. */
+class SurfaceRef {

[Fady Samuel, 2016/12/12 16:12:00]

Please try to have one class per header / cc file pair. Also, the name of the
class should match the name of the file. This is very hard to follow.

+ public:
+  explicit SurfaceRef(const SurfaceEmbedding* factory);

[Fady Samuel, 2016/12/12 16:12:00]

I'd also give this a move only constructor and move assignment operator.

+  virtual ~SurfaceRef();
+
+  void Destroy();
+  void Destroy(CompositorFrameMetadata* metadata);
+  bool IsDestroyed() const;
+
+  const SurfaceEmbedding* factory() const { return factory_.get(); }
+
+ private:
+  scoped_refptr<const SurfaceEmbedding> factory_;
+};
+
+typedef std::unique_ptr<SurfaceRef> SurfaceRefPtr;
+
+class SequenceSurfaceRef : public SurfaceRef {
+ public:
+  SequenceSurfaceRef(const SurfaceEmbedding* factory,

[Fady Samuel, 2016/12/12 16:12:00]

Move constructor and move assignment operator.

+                     const SurfaceSequence& seq);
+
+  const SurfaceSequence& seq() const { return seq_; }
+
+ private:
+  const SurfaceSequence seq_;
+};
+
+class SurfaceEmbedding : public base::RefCounted<SurfaceEmbedding> {
+ public:
+  virtual SurfaceRefPtr CreateReference(SurfaceReferenceOwner*,
+                                        const SurfaceId&) const = 0;
+
+ protected:
+  virtual ~SurfaceEmbedding() = default;
+
+ private:
+  friend class base::RefCounted<SurfaceEmbedding>;
+  virtual void DestroyReference(SurfaceRef*) const = 0;
+  virtual void DestroyReference(SurfaceRef*,
+                                CompositorFrameMetadata*) const = 0;
+  friend class SurfaceRef;
+};
+
+}  // namespace cc
+
+#endif  // CC_SURFACES_SURFACE_EMBEDDING_BASE_H_