Pass StatsTable shared memory via global descriptors on Posix rather than using named shared memory.

base/metrics/stats_table.cc

Index: base/metrics/stats_table.cc
diff --git a/base/metrics/stats_table.cc b/base/metrics/stats_table.cc
index 4a3d939cfe093c460025ff2ed26c699fbc8fdd7b..f8251fa8b5c0ce8f2faa65097addcbad219bb7dc 100644
--- a/base/metrics/stats_table.cc
+++ b/base/metrics/stats_table.cc
@@ -15,7 +15,9 @@
 #include "base/threading/thread_local_storage.h"
 
 #if defined(OS_POSIX)
+#include "base/posix/global_descriptors.h"
 #include "errno.h"
+#include "ipc/ipc_descriptors.h"
 #endif
 
 namespace base {
@@ -106,7 +108,7 @@ class StatsTable::Private {
   static Private* New(const std::string& name, int size,
                                 int max_threads, int max_counters);
 
-  SharedMemory* shared_memory() { return &shared_memory_; }
+  SharedMemory* shared_memory() { return shared_memory_.get(); }
 
   // Accessors for our header pointers
   TableHeader* table_header() const { return table_header_; }
@@ -136,8 +138,9 @@ class StatsTable::Private {
 
  private:
   // Constructor is private because you should use New() instead.
-  Private()
-      : table_header_(NULL),
+  Private(SharedMemory* shared_memory)

[jar (doing other things), 2013/08/21 01:09:46]

nit: explicit

[rmcilroy, 2013/08/21 10:37:16]

Done.

+      : shared_memory_(shared_memory),
+        table_header_(NULL),
         thread_names_table_(NULL),
         thread_tid_table_(NULL),
         thread_pid_table_(NULL),
@@ -145,6 +148,9 @@ class StatsTable::Private {
         data_table_(NULL) {
   }
 
+  // Create or open the SharedMemory used by the stats table.
+  static SharedMemory* CreateSharedMemory(int size);
+
   // Initializes the table on first access.  Sets header values
   // appropriately and zeroes all counters.
   void InitializeTable(void* memory, int size, int max_counters,
@@ -153,7 +159,7 @@ class StatsTable::Private {
   // Initializes our in-memory pointers into a pre-created StatsTable.
   void ComputeMappedPointers(void* memory);
 
-  SharedMemory shared_memory_;
+  scoped_ptr<SharedMemory> shared_memory_;
   TableHeader* table_header_;
   char* thread_names_table_;
   PlatformThreadId* thread_tid_table_;
@@ -167,12 +173,11 @@ StatsTable::Private* StatsTable::Private::New(const std::string& name,
                                               int size,
                                               int max_threads,
                                               int max_counters) {
-  scoped_ptr<Private> priv(new Private());
-  if (!priv->shared_memory_.CreateNamed(name, true, size))
-    return NULL;
-  if (!priv->shared_memory_.Map(size))
+  scoped_ptr<Private> priv(new Private(CreateSharedMemory(size)));
+
+  if (!priv->shared_memory_->Map(size))

[jar (doing other things), 2013/08/21 01:09:46]

Your code can return a null in several cases (line 203, or line 214), so you
need to handle such.

if (!priv->shared_memory || !priv->shared_memory_->Map(size))

[rmcilroy, 2013/08/21 10:37:16]

Good point, done.

     return NULL;
-  void* memory = priv->shared_memory_.memory();
+  void* memory = priv->shared_memory_->memory();
 
   TableHeader* header = static_cast<TableHeader*>(memory);
 
@@ -187,6 +192,30 @@ StatsTable::Private* StatsTable::Private::New(const std::string& name,
   return priv.release();
 }
 
+// static
+SharedMemory* StatsTable::Private::CreateSharedMemory(int size) {
+#if defined(OS_POSIX)
+  GlobalDescriptors* global_descriptors = GlobalDescriptors::GetInstance();
+  if (global_descriptors->MaybeGet(kStatsTableSharedMemFd) == -1) {
+    // If no global table exists, create it.
+    scoped_ptr<SharedMemory> shared_memory(new SharedMemory());
+    if (!shared_memory->CreateAnonymous(size))
+      return NULL;
+    return shared_memory.release();
+  } else {

[jar (doing other things), 2013/08/21 01:09:46]

nit: no need for "else" and indent, since you returned in line 204.

Less important nit: When both clauses of an if return... it is more readable to
list the shorter block one up front.

[rmcilroy, 2013/08/21 10:37:16]

Done.

+    // Otherwise open the file descriptor passed by the browser process.
+    FileDescriptor file_descriptor(
+        global_descriptors->Get(kStatsTableSharedMemFd), false);
+    return new SharedMemory(file_descriptor, false);
+  }
+#elif defined(OS_WIN)
+  scoped_ptr<SharedMemory> shared_memory(new SharedMemory());
+  if (!shared_memory.CreateNamed(name, true, size))
+    return NULL;
+  return shared_memory.release();
+#endif
+}
+
 void StatsTable::Private::InitializeTable(void* memory, int size,
                                           int max_counters,
                                           int max_threads) {
@@ -565,4 +594,10 @@ StatsTable::TLSData* StatsTable::GetTLSData() const {
   return data;
 }
 
+#if defined(OS_POSIX)
+base::SharedMemoryHandle StatsTable::GetSharedMemoryHandle() const {
+  return impl_->shared_memory()->handle();

[jar (doing other things), 2013/08/21 01:09:46]

Shouldn't you handle impl_ == NULL?

[rmcilroy, 2013/08/21 10:37:16]

Done.

+}
+#endif
+
 }  // namespace base