Mojo: Add a generator for local endpoint IDs ("LocalChannelEndpointIdGenerator").

Note: It's called "local", since I'm going to add a separate generator
for remote IDs.

R=brettw@chromium.org

Review URL: https://codereview.chromium.org/638593004

Cr-Commit-Position: refs/heads/master@{#298721}

3 files changed, 67 insertions, 32 deletions

diff --git a/mojo/edk/system/channel.cc b/mojo/edk/system/channel.cc
index 94eb7ef..50411b3 100644
--- a/mojo/edk/system/channel.cc
+++ b/mojo/edk/system/channel.cc
@@ -21,8 +21,7 @@ namespace system {
 Channel::Channel(embedder::PlatformSupport* platform_support)
     : platform_support_(platform_support),
       is_running_(false),
-      is_shutting_down_(false),
-      next_local_id_(ChannelEndpointId::GetBootstrap()) {
+      is_shutting_down_(false) {
 }
 
 bool Channel::Init(scoped_ptr<RawChannel> raw_channel) {
@@ -99,13 +98,10 @@ ChannelEndpointId Channel::AttachEndpoint(
     DLOG_IF(WARNING, is_shutting_down_)
         << "AttachEndpoint() while shutting down";
 
-    while (!next_local_id_.is_valid() ||
-           local_id_to_endpoint_map_.find(next_local_id_) !=
-               local_id_to_endpoint_map_.end())
-      next_local_id_.value++;
-
-    local_id = next_local_id_;
-    next_local_id_.value++;
+    do {
+      local_id = local_id_generator_.GetNext();
+    } while (local_id_to_endpoint_map_.find(local_id) !=
+             local_id_to_endpoint_map_.end());
     local_id_to_endpoint_map_[local_id] = endpoint;
   }
 
@@ -146,8 +142,8 @@ void Channel::RunRemoteMessagePipeEndpoint(ChannelEndpointId local_id,
     HandleLocalError(base::StringPrintf(
         "Failed to send message to run remote message pipe endpoint (local ID "
         "%u, remote ID %u)",
-        static_cast<unsigned>(local_id.value),
-        static_cast<unsigned>(remote_id.value)));
+        static_cast<unsigned>(local_id.value()),
+        static_cast<unsigned>(remote_id.value())));
   }
 }
 
@@ -206,8 +202,8 @@ void Channel::DetachEndpoint(ChannelEndpoint* endpoint,
     HandleLocalError(base::StringPrintf(
         "Failed to send message to remove remote message pipe endpoint (local "
         "ID %u, remote ID %u)",
-        static_cast<unsigned>(local_id.value),
-        static_cast<unsigned>(remote_id.value)));
+        static_cast<unsigned>(local_id.value()),
+        static_cast<unsigned>(remote_id.value())));
   }
 }
 
@@ -311,7 +307,7 @@ void Channel::OnReadMessageForDownstream(
   if (!endpoint.get()) {
     HandleRemoteError(base::StringPrintf(
         "Received a message for nonexistent local destination ID %u",
-        static_cast<unsigned>(local_id.value)));
+        static_cast<unsigned>(local_id.value())));
     // This is strongly indicative of some problem. However, it's not a fatal
     // error, since it may indicate a buggy (or hostile) remote process. Don't
     // die even for Debug builds, since handling this properly needs to be
@@ -323,7 +319,7 @@ void Channel::OnReadMessageForDownstream(
   if (!endpoint->OnReadMessage(message_view, platform_handles.Pass())) {
     HandleLocalError(
         base::StringPrintf("Failed to enqueue message to local ID %u",
-                           static_cast<unsigned>(local_id.value)));
+                           static_cast<unsigned>(local_id.value())));
     return;
   }
 }
@@ -429,8 +425,8 @@ bool Channel::OnRemoveMessagePipeEndpoint(ChannelEndpointId local_id,
     HandleLocalError(base::StringPrintf(
         "Failed to send message to remove remote message pipe endpoint ack "
         "(local ID %u, remote ID %u)",
-        static_cast<unsigned>(local_id.value),
-        static_cast<unsigned>(remote_id.value)));
+        static_cast<unsigned>(local_id.value()),
+        static_cast<unsigned>(remote_id.value())));
   }
 
   endpoint->OnDisconnect();
diff --git a/mojo/edk/system/channel.h b/mojo/edk/system/channel.h
index 41ae039..5ece5da 100644
--- a/mojo/edk/system/channel.h
+++ b/mojo/edk/system/channel.h
@@ -180,9 +180,8 @@ class MOJO_SYSTEM_IMPL_EXPORT Channel
   // Map from local IDs to endpoints. If the endpoint is null, this means that
   // we're just waiting for the remove ack before removing the entry.
   IdToEndpointMap local_id_to_endpoint_map_;
-  // The next local ID to try (when allocating new local IDs). Note: It should
-  // be checked for existence before use.
-  ChannelEndpointId next_local_id_;
+  // Note: The IDs generated by this should be checked for existence before use.
+  LocalChannelEndpointIdGenerator local_id_generator_;
 
   DISALLOW_COPY_AND_ASSIGN(Channel);
 };
diff --git a/mojo/edk/system/channel_endpoint_id.h b/mojo/edk/system/channel_endpoint_id.h
index a0f518c..fc1ad60 100644
--- a/mojo/edk/system/channel_endpoint_id.h
+++ b/mojo/edk/system/channel_endpoint_id.h
@@ -10,39 +10,50 @@
 
 #include <ostream>
 
-// TODO(vtl): Remove these once we can use |std::unordered_{map,set}|.
 #include "base/containers/hash_tables.h"
+#include "base/macros.h"
 #include "build/build_config.h"
 
 namespace mojo {
 namespace system {
 
-struct ChannelEndpointId {
-  ChannelEndpointId() : value(0) {}
-  ChannelEndpointId(const ChannelEndpointId& other) : value(other.value) {}
+// ChannelEndpointId -----------------------------------------------------------
+
+class LocalChannelEndpointIdGenerator;
+
+// Represents an ID for an endpoint (i.e., one side of a message pipe) on a
+// |Channel|. This class must be POD.
+class ChannelEndpointId {
+ public:
+  ChannelEndpointId() : value_(0) {}
+  ChannelEndpointId(const ChannelEndpointId& other) : value_(other.value_) {}
 
   // Returns the local ID to use for the first message pipe endpoint on a
   // channel.
   static ChannelEndpointId GetBootstrap() {
     ChannelEndpointId rv;
-    rv.value = 1;
+    rv.value_ = 1;
     return rv;
   }
 
   bool operator==(const ChannelEndpointId& other) const {
-    return value == other.value;
+    return value_ == other.value_;
   }
   bool operator!=(const ChannelEndpointId& other) const {
     return !operator==(other);
   }
   // So that we can be used in |std::map|, etc.
   bool operator<(const ChannelEndpointId& other) const {
-    return value < other.value;
+    return value_ < other.value_;
   }
 
-  bool is_valid() const { return !!value; }
+  bool is_valid() const { return !!value_; }
+  uint32_t value() const { return value_; }
+
+ private:
+  friend class LocalChannelEndpointIdGenerator;
 
-  uint32_t value;
+  uint32_t value_;
 
   // Copying and assignment allowed.
 };
@@ -55,12 +66,41 @@ static_assert(sizeof(ChannelEndpointId) == sizeof(uint32_t),
 // So logging macros and |DCHECK_EQ()|, etc. work.
 inline std::ostream& operator<<(std::ostream& out,
                                 const ChannelEndpointId& channel_endpoint_id) {
-  return out << channel_endpoint_id.value;
+  return out << channel_endpoint_id.value();
 }
 
+// LocalChannelEndpointIdGenerator ---------------------------------------------
+
+// A simple generator for "new" local |ChannelEndpointId|s. It does not track
+// used/existing IDs; that must be done separately. (This class is not
+// thread-safe.)
+class LocalChannelEndpointIdGenerator {
+ public:
+  LocalChannelEndpointIdGenerator()
+      : next_channel_endpoint_id_(ChannelEndpointId::GetBootstrap()) {}
+
+  ChannelEndpointId GetNext() {
+    ChannelEndpointId rv = next_channel_endpoint_id_;
+    next_channel_endpoint_id_.value_++;
+    // Skip over the invalid value, in case we wrap.
+    if (!next_channel_endpoint_id_.is_valid())
+      next_channel_endpoint_id_.value_++;
+    return rv;
+  }
+
+ private:
+  ChannelEndpointId next_channel_endpoint_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(LocalChannelEndpointIdGenerator);
+};
+
 }  // namespace system
 }  // namespace mojo
 
+// Define "hash" functions for |ChannelEndpointId|s, so they can be used in hash
+// tables.
+// TODO(vtl): Once we can use |std::unordered_{map,set}|, update this (and
+// remove the base/containers/hash_tables.h and build/build_config.h includes).
 namespace BASE_HASH_NAMESPACE {
 
 #if defined(COMPILER_GCC)
@@ -68,14 +108,14 @@ namespace BASE_HASH_NAMESPACE {
 template <>
 struct hash<mojo::system::ChannelEndpointId> {
   size_t operator()(mojo::system::ChannelEndpointId channel_endpoint_id) const {
-    return static_cast<size_t>(channel_endpoint_id.value);
+    return static_cast<size_t>(channel_endpoint_id.value());
   }
 };
 
 #elif defined(COMPILER_MSVC)
 
 inline size_t hash_value(mojo::system::ChannelEndpointId channel_endpoint_id) {
-  return static_cast<size_t>(channel_endpoint_id.value);
+  return static_cast<size_t>(channel_endpoint_id.value());
 }
 #endif