Allow Mojo structs as map keys

mojo/public/cpp/bindings/struct_ptr.h

Index: mojo/public/cpp/bindings/struct_ptr.h
diff --git a/mojo/public/cpp/bindings/struct_ptr.h b/mojo/public/cpp/bindings/struct_ptr.h
index 92f2728a3a08470541c0985e5d12e713e7363dbb..f04f3a4c26f966cbee69a3a0f536f247b4a4cdbf 100644
--- a/mojo/public/cpp/bindings/struct_ptr.h
+++ b/mojo/public/cpp/bindings/struct_ptr.h
@@ -5,15 +5,19 @@
 #ifndef MOJO_PUBLIC_CPP_BINDINGS_STRUCT_PTR_H_
 #define MOJO_PUBLIC_CPP_BINDINGS_STRUCT_PTR_H_
 
+#include <functional>
 #include <new>
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "mojo/public/cpp/bindings/lib/hash_util.h"
 #include "mojo/public/cpp/bindings/type_converter.h"
 
 namespace mojo {
 namespace internal {
 
+constexpr size_t kHashSeed = 31;
+
 template <typename Struct>
 class StructHelper {
  public:
@@ -23,6 +27,12 @@ class StructHelper {
   }
 };
 
+template <typename Struct>
+class StructPtrWTFHelper;
+
+template <typename Struct>
+class InlinedStructPtrWTFHelper;
+
 }  // namespace internal
 
 // Smart pointer wrapping a mojom structure with move-only semantics.
@@ -78,28 +88,35 @@ class StructPtr {
   // that it contains Mojo handles).
   StructPtr Clone() const { return is_null() ? StructPtr() : ptr_->Clone(); }
 
+  // Compares the pointees (which might both be null).
   bool Equals(const StructPtr& other) const {
     if (is_null() || other.is_null())
       return is_null() && other.is_null();
     return ptr_->Equals(*other.ptr_);
   }
 
- private:
-  // TODO(dcheng): Use an explicit conversion operator.
-  typedef Struct* StructPtr::*Testable;
+  // Hashes based on the pointee (which might be null).
+  size_t Hash(size_t seed) const {
+    if (is_null())
+      return internal::HashCombine(seed, 0);
+    return ptr_->Hash(seed);
+  }
 
- public:
-  operator Testable() const { return ptr_ ? &StructPtr::ptr_ : 0; }
+  // TODO(tibell): Get rid of Equals in favor of the operator. Same for Hash.
+  template <typename T>
+  bool operator==(const StructPtr<T>& other) const {

[Sam McNally, 2016/09/22 05:40:51]

How about an out of line
template <typename T>
bool operator==(const StructPtr<T>&, const StructPtr<T>&)?

[tibell, 2016/09/22 07:18:33]

Done.

+    return this->Equals(other);
+  }
+  template <typename T>
+  bool operator!=(const StructPtr<T>& other) const {
+    return !(this->Equals(other));
+  }
+
+  explicit operator bool() const { return !is_null(); }
 
  private:
   friend class internal::StructHelper<Struct>;
-
-  // Forbid the == and != operators explicitly, otherwise StructPtr will be
-  // converted to Testable to do == or != comparison.
-  template <typename T>
-  bool operator==(const StructPtr<T>& other) const = delete;
-  template <typename T>
-  bool operator!=(const StructPtr<T>& other) const = delete;
+  friend class internal::StructPtrWTFHelper<Struct>;
 
   void Initialize() {
     DCHECK(!ptr_);
@@ -122,8 +139,8 @@ class InlinedStructPtr {
  public:
   using Struct = S;
 
-  InlinedStructPtr() : is_null_(true) {}
-  InlinedStructPtr(decltype(nullptr)) : is_null_(true) {}
+  InlinedStructPtr() : state_(NIL) {}
+  InlinedStructPtr(decltype(nullptr)) : state_(NIL) {}
 
   ~InlinedStructPtr() {}
 
@@ -132,7 +149,7 @@ class InlinedStructPtr {
     return *this;
   }
 
-  InlinedStructPtr(InlinedStructPtr&& other) : is_null_(true) { Take(&other); }
+  InlinedStructPtr(InlinedStructPtr&& other) : state_(NIL) { Take(&other); }
   InlinedStructPtr& operator=(InlinedStructPtr&& other) {
     Take(&other);
     return *this;
@@ -144,67 +161,128 @@ class InlinedStructPtr {
   }
 
   void reset() {
-    is_null_ = true;
+    state_ = NIL;
     value_. ~Struct();
     new (&value_) Struct();
   }
 
-  bool is_null() const { return is_null_; }
+  bool is_null() const { return state_ == NIL; }
 
   Struct& operator*() const {
-    DCHECK(!is_null_);
+    DCHECK(state_ == VALID);
     return value_;
   }
   Struct* operator->() const {
-    DCHECK(!is_null_);
+    DCHECK(state_ == VALID);
     return &value_;
   }
   Struct* get() const { return &value_; }
 
   void Swap(InlinedStructPtr* other) {
     std::swap(value_, other->value_);
-    std::swap(is_null_, other->is_null_);
+    std::swap(state_, other->state_);
   }
 
   InlinedStructPtr Clone() const {
     return is_null() ? InlinedStructPtr() : value_.Clone();
   }
+
+  // Compares the pointees (which might both be null).
   bool Equals(const InlinedStructPtr& other) const {
     if (is_null() || other.is_null())
       return is_null() && other.is_null();
     return value_.Equals(other.value_);
   }
 
- private:
-  // TODO(dcheng): Use an explicit conversion operator.
-  typedef Struct InlinedStructPtr::*Testable;
+  // Hashes based on the pointee (which might be null).
+  size_t Hash(size_t seed) const {
+    if (is_null())
+      return internal::HashCombine(seed, 0);
+    return value_.Hash(seed);
+  }
 
- public:
-  operator Testable() const { return is_null_ ? 0 : &InlinedStructPtr::value_; }
+  template <typename T>
+  bool operator==(const InlinedStructPtr<T>& other) const {
+    return this->Equals(other);
+  }
+  template <typename T>
+  bool operator!=(const InlinedStructPtr<T>& other) const {
+    return !(this->Equals(other));
+  }
+
+  explicit operator bool() const { return !is_null(); }
 
  private:
   friend class internal::StructHelper<Struct>;
+  friend class internal::StructPtrWTFHelper<Struct>;
 
-  // Forbid the == and != operators explicitly, otherwise InlinedStructPtr will
-  // be converted to Testable to do == or != comparison.
-  template <typename T>
-  bool operator==(const InlinedStructPtr<T>& other) const = delete;
-  template <typename T>
-  bool operator!=(const InlinedStructPtr<T>& other) const = delete;
-
-  void Initialize() { is_null_ = false; }
+  void Initialize() { state_ = VALID; }
 
   void Take(InlinedStructPtr* other) {
     reset();
     Swap(other);
   }
 
+  enum State {
+    VALID,
+    NIL,
+    DELETED,  // For use in WTF::HashMap only
+  };
+
   mutable Struct value_;
-  bool is_null_;
+  State state_;
 
   DISALLOW_COPY_AND_ASSIGN(InlinedStructPtr);
 };
 
+namespace internal {
+
+template <typename Struct>
+class StructPtrWTFHelper {
+  static bool IsHashTableDeletedValue(const StructPtr<Struct>& value) {
+    return value.ptr_ == reinterpret_cast<Struct*>(1u);
+  }
+
+  static void ConstructDeletedValue(mojo::StructPtr<Struct>& slot) {
+    // Dirty trick: implant an invalid pointer in |ptr_|. Destructor isn't
+    // called for deleted buckets, so this is okay.
+    new (&slot) StructPtr<Struct>();
+    slot.ptr_ = reinterpret_cast<Struct*>(1u);
+  }
+};
+
+template <typename Struct>
+class InlinedStructPtrWTFHelper {
+  static bool IsHashTableDeletedValue(const InlinedStructPtr<Struct>& value) {
+    return value.state_ == InlinedStructPtr<Struct>::DELETED;
+  }
+
+  static void ConstructDeletedValue(mojo::InlinedStructPtr<Struct>& slot) {
+    new (&slot) InlinedStructPtr<Struct>();
+    slot.state_ = InlinedStructPtr<Struct>::DELETEDc;

[Sam McNally, 2016/09/22 05:40:51]

DELETEDc?

[tibell, 2016/09/22 07:18:33]

Done.

+  }
+};
+
+}  // namespace internal
+
 }  // namespace mojo
 
+namespace std {
+
+template <typename T>
+struct hash<mojo::StructPtr<T>> {
+  size_t operator()(const mojo::StructPtr<T>& value) const {
+    return value.Hash(mojo::internal::kHashSeed);
+  }
+};
+
+template <typename T>
+struct hash<mojo::InlinedStructPtr<T>> {
+  size_t operator()(const mojo::InlinedStructPtr<T>& value) const {
+    return value.Hash(mojo::internal::kHashSeed);
+  }
+};
+
+}  // namespace std
+
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_STRUCT_PTR_H_