Make cc::SurfaceId unguessable

cc/surfaces/surface_id.h

Index: cc/surfaces/surface_id.h
diff --git a/cc/surfaces/surface_id.h b/cc/surfaces/surface_id.h
index bbc485f9009f90eaf577205f31dd7d2533310b6e..2d80468457610d0503aec03a330029e72c73ec17 100644
--- a/cc/surfaces/surface_id.h
+++ b/cc/surfaces/surface_id.h
@@ -10,36 +10,82 @@
 
 #include <functional>
 
+#include "base/hash.h"
+#include "base/strings/stringprintf.h"
+
 namespace cc {
 
-struct SurfaceId {
-  SurfaceId() : id(0) {}
-  explicit SurfaceId(uint64_t id) : id(id) {}
+class SurfaceId {
+ public:
+  SurfaceId() : id_namespace_(0), nonce_(0), local_id_(0) {}
 
-  bool is_null() const { return id == 0; }
+  SurfaceId(const SurfaceId& other)
+      : id_namespace_(other.id_namespace_),
+        nonce_(other.nonce_),
+        local_id_(other.local_id_) {}
 
-  // See SurfaceIdAllocator::GenerateId.
-  uint32_t id_namespace() const { return id >> 32; }
+  SurfaceId(uint32_t id_namespace, uint64_t nonce, uint32_t local_id)

[danakj, 2016/05/25 00:19:08]

Can you please document what is the namespace, nonce and local_id used for, so
callers can figure out what they should give here? Especially in unit tests or
other situations where they are making them up and need to make sense of them.

[Fady Samuel, 2016/05/25 00:57:45]

Done.

+      : id_namespace_(id_namespace),
+        nonce_(nonce),
+        local_id_(local_id)
 
-  uint64_t id;
-};
+  {}
 
-inline bool operator==(const SurfaceId& a, const SurfaceId& b) {
-  return a.id == b.id;
-}
+  bool is_null() const {
+    return id_namespace_ == 0 && nonce_ == 0 && local_id_ == 0;
+  }
 
-inline bool operator!=(const SurfaceId& a, const SurfaceId& b) {
-  return !(a == b);
-}
+  size_t hash() const {
+    size_t interim = base::HashInts(id_namespace_, nonce_);
+    return base::HashInts(interim, local_id_);
+  }
 
-inline bool operator<(const SurfaceId& a, const SurfaceId& b) {
-  return a.id < b.id;
-}
+  uint32_t id_namespace() const { return id_namespace_; }
 
-struct SurfaceIdHash {
-  size_t operator()(const SurfaceId& key) const {
-    return std::hash<uint64_t>()(key.id);
+  uint64_t nonce() const { return nonce_; }
+
+  uint32_t local_id() const { return local_id_; }
+
+  std::string ToString() const {
+    return base::StringPrintf("(%d, %lu, %d)", id_namespace_, nonce_,

[Tom Sepez, 2016/05/24 22:35:45]

might we ever have a FromString()?  If so, we might want to pick a
representation like x.y.z or x:y:z so that we don't get hung up on spaces as it
gets passed around as a string.

[Fady Samuel, 2016/05/25 00:04:29]

Done. Went with namespace:local:nonce

+                              local_id_);
+  }
+
+  bool operator==(const SurfaceId& b) const {
+    if (id_namespace_ != b.id_namespace_)

[Tom Sepez, 2016/05/24 22:35:45]

nit or just:
  return id_namespace == b.id_namespace &&
      nonce == b.nonce && local_id == b.local_id;

[Fady Samuel, 2016/05/25 00:04:29]

Done.

+      return false;
+
+    if (nonce_ != b.nonce_)
+      return false;
+
+    return local_id_ == b.local_id_;
   }
+
+  bool operator!=(const SurfaceId& b) const { return !(*this == b); }
+
+  bool operator<(const SurfaceId& b) const {
+    if (id_namespace_ < b.id_namespace_)

[Tom Sepez, 2016/05/24 22:35:45]

I'd sort these by namespace, then local, then nonce, if I'm looking at an
ordered dump of some sort, it might be easier to pick out a duplicated local id.

[Fady Samuel, 2016/05/25 00:04:29]

Done.

+      return true;
+
+    if (nonce_ < b.nonce_)
+      return id_namespace_ == b.id_namespace_;
+
+    if (local_id_ < b.local_id_) {
+      return id_namespace_ == b.id_namespace_ && nonce_ == b.nonce_;
+    }
+
+    return false;
+  }
+
+ private:
+  // See SurfaceIdAllocator::GenerateId.
+  uint32_t id_namespace_;

[Tom Sepez, 2016/05/24 22:35:45]

I'd order these as namespace_id_, local_id, nonce_, in case the compiler wants
to align int64's to 8-byte boundary, you'll save 8 bytes.
 

[Fady Samuel, 2016/05/25 00:04:29]

Done.

+  uint64_t nonce_;
+  uint32_t local_id_;
+};
+
+struct SurfaceIdHash {
+  size_t operator()(const SurfaceId& key) const { return key.hash(); }
 };
 
 }  // namespace cc