Allow Mojo structs as map keys

mojo/public/cpp/bindings/struct_ptr.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 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/type_converter.h"
 
 namespace mojo {
 namespace internal {
 
+constexpr size_t kHashSeed = 31;
+
 template <typename Struct>
 class StructHelper {
  public:
   template <typename Ptr>
   static void Initialize(Ptr* ptr) {
     ptr->Initialize();
   }
 };
 
 }  // namespace internal
@@ skipping 50 matching lines @@
   // type |Struct| doesn't have a Clone() method defined (which usually means
   // that it contains Mojo handles).
   StructPtr Clone() const { return is_null() ? StructPtr() : ptr_->Clone(); }
 
   bool Equals(const StructPtr& other) const {
     if (is_null() || other.is_null())
       return is_null() && other.is_null();
     return ptr_->Equals(*other.ptr_);
   }
 
+  // For use in WTF::HashMap only:
+  bool isHashTableDeletedValue() const {

[yzshen1, 2016/09/20 23:44:40]

Does it feel cleaner if we have these WTF-only functions in a separate class,
say StructPtrWTFHelper or something like that?

[yzshen1, 2016/09/21 23:17:52]

This comment is not replied yet.
(Besides, if these methods are outside of the WTF namespace, it seems more
consistent to name them with uppercase initial character.)

[tibell, 2016/09/22 05:17:23]

Done.

+    return ptr_ == reinterpret_cast<Struct*>(1u);
+  }
+
+  // For use in WTF::HashMap only:
+  static void constructDeletedValue(mojo::StructPtr<S>& slot) {
+    // Dirty trick: implant an invalid pointer in |ptr_|. Destructor isn't
+    // called for deleted buckets, so this is okay.
+    new (&slot) StructPtr();
+    slot.ptr_ = reinterpret_cast<Struct*>(1u);
+  }
+
  private:
   // TODO(dcheng): Use an explicit conversion operator.
   typedef Struct* StructPtr::*Testable;
 
  public:
   operator Testable() const { return ptr_ ? &StructPtr::ptr_ : 0; }
 
  private:
   friend class internal::StructHelper<Struct>;
 
-  // Forbid the == and != operators explicitly, otherwise StructPtr will be
-  // converted to Testable to do == or != comparison.

[yzshen1, 2016/09/20 23:44:40]

If you would like to allow == and !=, please fix the TODO on line 104. :)

[tibell, 2016/09/21 07:10:53]

Done.

-  template <typename T>
-  bool operator==(const StructPtr<T>& other) const = delete;
-  template <typename T>
-  bool operator!=(const StructPtr<T>& other) const = delete;
-
   void Initialize() {
     DCHECK(!ptr_);
     ptr_ = new Struct();
   }
 
   void Take(StructPtr* other) {
     reset();
     Swap(other);
   }
 
   Struct* ptr_;
 
   DISALLOW_COPY_AND_ASSIGN(StructPtr);
 };
 
 // Designed to be used when Struct is small and copyable.
 template <typename S>
 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() {}
 
   InlinedStructPtr& operator=(decltype(nullptr)) {
     reset();
     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;
   }
 
   template <typename U>
   U To() const {
     return TypeConverter<U, InlinedStructPtr>::Convert(*this);
   }
 
   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();
   }
   bool Equals(const InlinedStructPtr& other) const {
     if (is_null() || other.is_null())
       return is_null() && other.is_null();
     return value_.Equals(other.value_);
   }
 
+  // For use in WTF::HashMap only:
+  bool isHashTableDeletedValue() const { return state_ == DELETED; }
+
+  // For use in WTF::HashMap only:
+  static void constructDeletedValue(mojo::InlinedStructPtr<S>& slot) {
+    slot = InlinedStructPtr();

[Sam McNally, 2016/09/21 04:57:58]

Should this be constructing an InlinedStructPtr in &slot like the StructPtr
version?

[tibell, 2016/09/21 07:10:53]

Done.

+    slot.state_ = DELETED;
+  }
+
  private:
   // TODO(dcheng): Use an explicit conversion operator.
   typedef Struct InlinedStructPtr::*Testable;
 
  public:
-  operator Testable() const { return is_null_ ? 0 : &InlinedStructPtr::value_; }
+  operator Testable() const {
+    return is_null() ? 0 : &InlinedStructPtr::value_;
+  }
 
  private:
   friend class internal::StructHelper<Struct>;
 
-  // Forbid the == and != operators explicitly, otherwise InlinedStructPtr will

[yzshen1, 2016/09/20 23:44:40]

Please consider fixing TODO on line 197. :)

[tibell, 2016/09/21 07:10:53]

Done.

-  // 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,

[yzshen1, 2016/09/20 23:44:40]

I think NULL might be more commonly used.

[tibell, 2016/09/21 07:10:53]

It will collide with the standard NULL macro. :)

[yzshen1, 2016/09/21 23:17:52]

Ah, of course. :)

+    DELETED,  // For use in WTF::HashMap only
+  };
+
   mutable Struct value_;
-  bool is_null_;
+  State state_;
 
   DISALLOW_COPY_AND_ASSIGN(InlinedStructPtr);
 };
 
 }  // namespace mojo
 
+namespace std {
+
+template <typename T>
+struct hash<mojo::StructPtr<T>> {
+  size_t operator()(const mojo::StructPtr<T>& value) const {
+    return mojo::internal::Hash(mojo::internal::kHashSeed, *value.get());
+  }
+};
+
+template <typename T>
+struct hash<mojo::InlinedStructPtr<T>> {
+  size_t operator()(const mojo::InlinedStructPtr<T>& value) const {
+    return mojo::internal::Hash(mojo::internal::kHashSeed, *value.get());
+  }
+};
+
+}  // namespace std
+
 #endif  // MOJO_PUBLIC_CPP_BINDINGS_STRUCT_PTR_H_