Make cc::SurfaceId unguessable

cc/surfaces/surface_id.h

 // 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.
 
 #ifndef CC_SURFACES_SURFACE_ID_H_
 #define CC_SURFACES_SURFACE_ID_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #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_) {}
 
+  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)
+
+  {}
+
+  bool is_null() const {
+    return id_namespace_ == 0 && nonce_ == 0 && local_id_ == 0;
+  }
+
+  size_t hash() const {
+    size_t interim = base::HashInts(id_namespace_, nonce_);
+    return base::HashInts(interim, local_id_);
+  }
+
+  uint32_t id_namespace() const { return id_namespace_; }
+
+  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() const { return id >> 32; }
-
-  uint64_t id;
+  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_;
 };
 
-inline bool operator==(const SurfaceId& a, const SurfaceId& b) {
-  return a.id == b.id;
-}
-
-inline bool operator!=(const SurfaceId& a, const SurfaceId& b) {
-  return !(a == b);
-}
-
-inline bool operator<(const SurfaceId& a, const SurfaceId& b) {
-  return a.id < b.id;
-}
-
 struct SurfaceIdHash {
-  size_t operator()(const SurfaceId& key) const {
-    return std::hash<uint64_t>()(key.id);
-  }
+  size_t operator()(const SurfaceId& key) const { return key.hash(); }
 };
 
 }  // namespace cc
 
 #endif  // CC_SURFACES_SURFACE_ID_H_