Make mojo_system static and mojo_system_impl a component, never use both

mojo/system/core.cc

 // Copyright 2013 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.
 
-#include "mojo/system/core_impl.h"
+#include "mojo/system/core.h"
 
 #include <vector>
 
 #include "base/logging.h"
 #include "base/time/time.h"
+#include "mojo/public/c/system/core.h"
 #include "mojo/system/constants.h"
 #include "mojo/system/data_pipe.h"
 #include "mojo/system/data_pipe_consumer_dispatcher.h"
 #include "mojo/system/data_pipe_producer_dispatcher.h"
 #include "mojo/system/dispatcher.h"
 #include "mojo/system/local_data_pipe.h"
 #include "mojo/system/memory.h"
 #include "mojo/system/message_pipe.h"
 #include "mojo/system/message_pipe_dispatcher.h"
 #include "mojo/system/raw_shared_buffer.h"
 #include "mojo/system/shared_buffer_dispatcher.h"
 #include "mojo/system/waiter.h"
 
+static mojo::system::Core* g_core = NULL;

[viettrungluu, 2014/04/09 23:06:44]

Move this down a few lines, and get rid of the mojo::system.

[DaveMoore, 2014/04/10 18:58:58]

The static has moved to entrypoints.cc, with Set and Get functions.

+
 namespace mojo {
 namespace system {
 
 // Implementation notes
 //
-// Mojo primitives are implemented by the singleton |CoreImpl| object. Most
+// Mojo primitives are implemented by the singleton |Core| object. Most
 // calls are for a "primary" handle (the first argument).
-// |CoreImpl::GetDispatcher()| is used to look up a |Dispatcher| object for a
+// |Core::GetDispatcher()| is used to look up a |Dispatcher| object for a
 // given handle. That object implements most primitives for that object. The
-// wait primitives are not attached to objects and are implemented by |CoreImpl|
+// wait primitives are not attached to objects and are implemented by |Core|
 // itself.
 //
 // Some objects have multiple handles associated to them, e.g., message pipes
 // (which have two). In such a case, there is still a |Dispatcher| (e.g.,
 // |MessagePipeDispatcher|) for each handle, with each handle having a strong
 // reference to the common "secondary" object (e.g., |MessagePipe|). This
 // secondary object does NOT have any references to the |Dispatcher|s (even if
 // it did, it wouldn't be able to do anything with them due to lock order
 // requirements -- see below).
 //
@@ skipping 22 matching lines @@
 //   INF. |Waiter| locks
 //
 // Notes:
 //    - While holding a |Dispatcher| lock, you may not unconditionally attempt
 //      to take another |Dispatcher| lock. (This has consequences on the
 //      concurrency semantics of |MojoWriteMessage()| when passing handles.)
 //      Doing so would lead to deadlock.
 //    - Locks at the "INF" level may not have any locks taken while they are
 //      held.
 
-CoreImpl::HandleTableEntry::HandleTableEntry()
+Core::HandleTableEntry::HandleTableEntry()
     : busy(false) {
 }
 
-CoreImpl::HandleTableEntry::HandleTableEntry(
+Core::HandleTableEntry::HandleTableEntry(
     const scoped_refptr<Dispatcher>& dispatcher)
     : dispatcher(dispatcher),
       busy(false) {
 }
 
-CoreImpl::HandleTableEntry::~HandleTableEntry() {
+Core::HandleTableEntry::~HandleTableEntry() {
   DCHECK(!busy);
 }
 

[viettrungluu, 2014/04/09 23:06:44]

nit:
// static

[DaveMoore, 2014/04/10 18:58:58]

Gone
On 2014/04/09 23:06:44, viettrungluu wrote:

-CoreImpl::CoreImpl() {
+Core* Core::GetInstance() {

[viettrungluu, 2014/04/09 23:06:44]

nit: ordering doesn't match the .h file

[DaveMoore, 2014/04/10 18:58:58]

Gone
On 2014/04/09 23:06:44, viettrungluu wrote:

+  if (!g_core)
+    g_core = new Core;

[viettrungluu, 2014/04/09 23:06:44]

Please don't do this. This masks initialization errors.

Probably there should be explicit CreateInstance()/ResetInstance()/GetInstance()
static methods.

[DaveMoore, 2014/04/10 18:58:58]

Gone
On 2014/04/09 23:06:44, viettrungluu wrote:

+  return g_core;
 }
 
-CoreImpl::~CoreImpl() {
+Core::Core() {
+}
+
+Core::~Core() {
   // This should usually not be reached (the singleton lives forever), except in
   // tests.
 }
 
-MojoHandle CoreImpl::AddDispatcher(
+MojoHandle Core::AddDispatcher(
     const scoped_refptr<Dispatcher>& dispatcher) {
   base::AutoLock locker(handle_table_lock_);
   return handle_table_.AddDispatcher(dispatcher);
 }
 
-MojoTimeTicks CoreImpl::GetTimeTicksNow() {
+MojoTimeTicks Core::GetTimeTicksNow() {
   return base::TimeTicks::Now().ToInternalValue();
 }
 
-MojoResult CoreImpl::Close(MojoHandle handle) {
+MojoResult Core::Close(MojoHandle handle) {
   if (handle == MOJO_HANDLE_INVALID)
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   scoped_refptr<Dispatcher> dispatcher;
   {
     base::AutoLock locker(handle_table_lock_);
     MojoResult result = handle_table_.GetAndRemoveDispatcher(handle,
                                                              &dispatcher);
     if (result != MOJO_RESULT_OK)
       return result;
   }
 
   // The dispatcher doesn't have a say in being closed, but gets notified of it.
   // Note: This is done outside of |handle_table_lock_|. As a result, there's a
   // race condition that the dispatcher must handle; see the comment in
   // |Dispatcher| in dispatcher.h.
   return dispatcher->Close();
 }
 
-MojoResult CoreImpl::Wait(MojoHandle handle,
-                          MojoWaitFlags flags,
-                          MojoDeadline deadline) {
+MojoResult Core::Wait(MojoHandle handle,
+                      MojoWaitFlags flags,
+                      MojoDeadline deadline) {
   return WaitManyInternal(&handle, &flags, 1, deadline);
 }
 
-MojoResult CoreImpl::WaitMany(const MojoHandle* handles,
-                              const MojoWaitFlags* flags,
-                              uint32_t num_handles,
-                              MojoDeadline deadline) {
+MojoResult Core::WaitMany(const MojoHandle* handles,
+                          const MojoWaitFlags* flags,
+                          uint32_t num_handles,
+                          MojoDeadline deadline) {
   if (!VerifyUserPointer<MojoHandle>(handles, num_handles))
     return MOJO_RESULT_INVALID_ARGUMENT;
   if (!VerifyUserPointer<MojoWaitFlags>(flags, num_handles))
     return MOJO_RESULT_INVALID_ARGUMENT;
   if (num_handles < 1)
     return MOJO_RESULT_INVALID_ARGUMENT;
   if (num_handles > kMaxWaitManyNumHandles)
     return MOJO_RESULT_RESOURCE_EXHAUSTED;
   return WaitManyInternal(handles, flags, num_handles, deadline);
 }
 
-MojoResult CoreImpl::CreateMessagePipe(MojoHandle* message_pipe_handle0,
-                                       MojoHandle* message_pipe_handle1) {
+MojoResult Core::CreateMessagePipe(MojoHandle* message_pipe_handle0,
+                                   MojoHandle* message_pipe_handle1) {
   if (!VerifyUserPointer<MojoHandle>(message_pipe_handle0, 1))
     return MOJO_RESULT_INVALID_ARGUMENT;
   if (!VerifyUserPointer<MojoHandle>(message_pipe_handle1, 1))
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   scoped_refptr<MessagePipeDispatcher> dispatcher0(new MessagePipeDispatcher());
   scoped_refptr<MessagePipeDispatcher> dispatcher1(new MessagePipeDispatcher());
 
   std::pair<MojoHandle, MojoHandle> handle_pair;
   {
@@ skipping 17 matching lines @@
   return MOJO_RESULT_OK;
 }
 
 // Implementation note: To properly cancel waiters and avoid other races, this
 // does not transfer dispatchers from one handle to another, even when sending a
 // message in-process. Instead, it must transfer the "contents" of the
 // dispatcher to a new dispatcher, and then close the old dispatcher. If this
 // isn't done, in the in-process case, calls on the old handle may complete
 // after the the message has been received and a new handle created (and
 // possibly even after calls have been made on the new handle).
-MojoResult CoreImpl::WriteMessage(MojoHandle message_pipe_handle,
-                                  const void* bytes,
-                                  uint32_t num_bytes,
-                                  const MojoHandle* handles,
-                                  uint32_t num_handles,
-                                  MojoWriteMessageFlags flags) {
+MojoResult Core::WriteMessage(MojoHandle message_pipe_handle,
+                              const void* bytes,
+                              uint32_t num_bytes,
+                              const MojoHandle* handles,
+                              uint32_t num_handles,
+                              MojoWriteMessageFlags flags) {
   scoped_refptr<Dispatcher> dispatcher(GetDispatcher(message_pipe_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   // Easy case: not sending any handles.
   if (num_handles == 0)
     return dispatcher->WriteMessage(bytes, num_bytes, NULL, flags);
 
   // We have to handle |handles| here, since we have to mark them busy in the
   // global handle table. We can't delegate this to the dispatcher, since the
@@ skipping 38 matching lines @@
     base::AutoLock locker(handle_table_lock_);
     if (rv == MOJO_RESULT_OK)
       handle_table_.RemoveBusyHandles(handles, num_handles);
     else
       handle_table_.RestoreBusyHandles(handles, num_handles);
   }
 
   return rv;
 }
 
-MojoResult CoreImpl::ReadMessage(MojoHandle message_pipe_handle,
-                                 void* bytes,
-                                 uint32_t* num_bytes,
-                                 MojoHandle* handles,
-                                 uint32_t* num_handles,
-                                 MojoReadMessageFlags flags) {
+MojoResult Core::ReadMessage(MojoHandle message_pipe_handle,
+                             void* bytes,
+                             uint32_t* num_bytes,
+                             MojoHandle* handles,
+                             uint32_t* num_handles,
+                             MojoReadMessageFlags flags) {
   scoped_refptr<Dispatcher> dispatcher(GetDispatcher(message_pipe_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   if (num_handles) {
     if (!VerifyUserPointer<uint32_t>(num_handles, 1))
       return MOJO_RESULT_INVALID_ARGUMENT;
     if (!VerifyUserPointer<MojoHandle>(handles, *num_handles))
       return MOJO_RESULT_INVALID_ARGUMENT;
   }
@@ skipping 23 matching lines @@
       for (size_t i = 0; i < dispatchers.size(); i++) {
         if (dispatchers[i])
           dispatchers[i]->Close();
       }
     }
   }
 
   return rv;
 }
 
-MojoResult CoreImpl::CreateDataPipe(const MojoCreateDataPipeOptions* options,
-                                    MojoHandle* data_pipe_producer_handle,
-                                    MojoHandle* data_pipe_consumer_handle) {
+MojoResult Core::CreateDataPipe(const MojoCreateDataPipeOptions* options,
+                                MojoHandle* data_pipe_producer_handle,
+                                MojoHandle* data_pipe_consumer_handle) {
   if (options) {
     // The |struct_size| field must be valid to read.
     if (!VerifyUserPointer<uint32_t>(&options->struct_size, 1))
       return MOJO_RESULT_INVALID_ARGUMENT;
     // And then |options| must point to at least |options->struct_size| bytes.
     if (!VerifyUserPointer<void>(options, options->struct_size))
       return MOJO_RESULT_INVALID_ARGUMENT;
   }
   if (!VerifyUserPointer<MojoHandle>(data_pipe_producer_handle, 1))
     return MOJO_RESULT_INVALID_ARGUMENT;
@@ skipping 27 matching lines @@
 
   scoped_refptr<DataPipe> data_pipe(new LocalDataPipe(validated_options));
   producer_dispatcher->Init(data_pipe);
   consumer_dispatcher->Init(data_pipe);
 
   *data_pipe_producer_handle = handle_pair.first;
   *data_pipe_consumer_handle = handle_pair.second;
   return MOJO_RESULT_OK;
 }
 
-MojoResult CoreImpl::WriteData(MojoHandle data_pipe_producer_handle,
-                               const void* elements,
-                               uint32_t* num_bytes,
-                               MojoWriteDataFlags flags) {
+MojoResult Core::WriteData(MojoHandle data_pipe_producer_handle,
+                           const void* elements,
+                           uint32_t* num_bytes,
+                           MojoWriteDataFlags flags) {
   scoped_refptr<Dispatcher> dispatcher(
       GetDispatcher(data_pipe_producer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   return dispatcher->WriteData(elements, num_bytes, flags);
 }
 
-MojoResult CoreImpl::BeginWriteData(MojoHandle data_pipe_producer_handle,
-                                    void** buffer,
-                                    uint32_t* buffer_num_bytes,
-                                    MojoWriteDataFlags flags) {
+MojoResult Core::BeginWriteData(MojoHandle data_pipe_producer_handle,
+                                void** buffer,
+                                uint32_t* buffer_num_bytes,
+                                MojoWriteDataFlags flags) {
   scoped_refptr<Dispatcher> dispatcher(
       GetDispatcher(data_pipe_producer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   return dispatcher->BeginWriteData(buffer, buffer_num_bytes, flags);
 }
 
-MojoResult CoreImpl::EndWriteData(MojoHandle data_pipe_producer_handle,
-                                  uint32_t num_bytes_written) {
+MojoResult Core::EndWriteData(MojoHandle data_pipe_producer_handle,
+                              uint32_t num_bytes_written) {
   scoped_refptr<Dispatcher> dispatcher(
       GetDispatcher(data_pipe_producer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   return dispatcher->EndWriteData(num_bytes_written);
 }
 
-MojoResult CoreImpl::ReadData(MojoHandle data_pipe_consumer_handle,
-                              void* elements,
-                              uint32_t* num_bytes,
-                              MojoReadDataFlags flags) {
+MojoResult Core::ReadData(MojoHandle data_pipe_consumer_handle,
+                          void* elements,
+                          uint32_t* num_bytes,
+                          MojoReadDataFlags flags) {
   scoped_refptr<Dispatcher> dispatcher(
       GetDispatcher(data_pipe_consumer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   return dispatcher->ReadData(elements, num_bytes, flags);
 }
 
-MojoResult CoreImpl::BeginReadData(MojoHandle data_pipe_consumer_handle,
-                                   const void** buffer,
-                                   uint32_t* buffer_num_bytes,
-                                   MojoReadDataFlags flags) {
+MojoResult Core::BeginReadData(MojoHandle data_pipe_consumer_handle,
+                               const void** buffer,
+                               uint32_t* buffer_num_bytes,
+                               MojoReadDataFlags flags) {
   scoped_refptr<Dispatcher> dispatcher(
       GetDispatcher(data_pipe_consumer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   return dispatcher->BeginReadData(buffer, buffer_num_bytes, flags);
 }
 
-MojoResult CoreImpl::EndReadData(MojoHandle data_pipe_consumer_handle,
-                                 uint32_t num_bytes_read) {
+MojoResult Core::EndReadData(MojoHandle data_pipe_consumer_handle,
+                             uint32_t num_bytes_read) {
   scoped_refptr<Dispatcher> dispatcher(
       GetDispatcher(data_pipe_consumer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   return dispatcher->EndReadData(num_bytes_read);
 }
 
-MojoResult CoreImpl::CreateSharedBuffer(
+MojoResult Core::CreateSharedBuffer(
     const MojoCreateSharedBufferOptions* options,
     uint64_t num_bytes,
     MojoHandle* shared_buffer_handle) {
   if (options) {
     // The |struct_size| field must be valid to read.
     if (!VerifyUserPointer<uint32_t>(&options->struct_size, 1))
       return MOJO_RESULT_INVALID_ARGUMENT;
     // And then |options| must point to at least |options->struct_size| bytes.
     if (!VerifyUserPointer<void>(options, options->struct_size))
       return MOJO_RESULT_INVALID_ARGUMENT;
@@ skipping 19 matching lines @@
   if (h == MOJO_HANDLE_INVALID) {
     LOG(ERROR) << "Handle table full";
     dispatcher->Close();
     return MOJO_RESULT_RESOURCE_EXHAUSTED;
   }
 
   *shared_buffer_handle = h;
   return MOJO_RESULT_OK;
 }
 
-MojoResult CoreImpl::DuplicateBufferHandle(
+MojoResult Core::DuplicateBufferHandle(
     MojoHandle buffer_handle,
     const MojoDuplicateBufferHandleOptions* options,
     MojoHandle* new_buffer_handle) {
   scoped_refptr<Dispatcher> dispatcher(GetDispatcher(buffer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   // Don't verify |options| here; that's the dispatcher's job.
   if (!VerifyUserPointer<MojoHandle>(new_buffer_handle, 1))
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   scoped_refptr<Dispatcher> new_dispatcher;
   MojoResult result = dispatcher->DuplicateBufferHandle(options,
                                                         &new_dispatcher);
   if (result != MOJO_RESULT_OK)
     return result;
 
   MojoHandle new_handle = AddDispatcher(new_dispatcher);
   if (new_handle == MOJO_HANDLE_INVALID) {
     LOG(ERROR) << "Handle table full";
     dispatcher->Close();
     return MOJO_RESULT_RESOURCE_EXHAUSTED;
   }
 
   *new_buffer_handle = new_handle;
   return MOJO_RESULT_OK;
 }
 
-MojoResult CoreImpl::MapBuffer(MojoHandle buffer_handle,
-                               uint64_t offset,
-                               uint64_t num_bytes,
-                               void** buffer,
-                               MojoMapBufferFlags flags) {
+MojoResult Core::MapBuffer(MojoHandle buffer_handle,
+                           uint64_t offset,
+                           uint64_t num_bytes,
+                           void** buffer,
+                           MojoMapBufferFlags flags) {
   scoped_refptr<Dispatcher> dispatcher(GetDispatcher(buffer_handle));
   if (!dispatcher.get())
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   if (!VerifyUserPointer<void*>(buffer, 1))
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   scoped_ptr<RawSharedBufferMapping> mapping;
   MojoResult result = dispatcher->MapBuffer(offset, num_bytes, flags, &mapping);
   if (result != MOJO_RESULT_OK)
     return result;
 
   DCHECK(mapping);
   void* address = mapping->base();
   {
     base::AutoLock locker(mapping_table_lock_);
     result = mapping_table_.AddMapping(mapping.Pass());
   }
   if (result != MOJO_RESULT_OK)
     return result;
 
   *buffer = address;
   return MOJO_RESULT_OK;
 }
 
-MojoResult CoreImpl::UnmapBuffer(void* buffer) {
+MojoResult Core::UnmapBuffer(void* buffer) {
   base::AutoLock locker(mapping_table_lock_);
   return mapping_table_.RemoveMapping(buffer);
 }
 
-scoped_refptr<Dispatcher> CoreImpl::GetDispatcher(MojoHandle handle) {
+scoped_refptr<Dispatcher> Core::GetDispatcher(MojoHandle handle) {
   if (handle == MOJO_HANDLE_INVALID)
     return NULL;
 
   base::AutoLock locker(handle_table_lock_);
   return handle_table_.GetDispatcher(handle);
 }
 
 // Note: We allow |handles| to repeat the same handle multiple times, since
 // different flags may be specified.
 // TODO(vtl): This incurs a performance cost in |RemoveWaiter()|. Analyze this
 // more carefully and address it if necessary.
-MojoResult CoreImpl::WaitManyInternal(const MojoHandle* handles,
-                                      const MojoWaitFlags* flags,
-                                      uint32_t num_handles,
-                                      MojoDeadline deadline) {
+MojoResult Core::WaitManyInternal(const MojoHandle* handles,
+                                  const MojoWaitFlags* flags,
+                                  uint32_t num_handles,
+                                  MojoDeadline deadline) {
   DCHECK_GT(num_handles, 0u);
 
   std::vector<scoped_refptr<Dispatcher> > dispatchers;
   dispatchers.reserve(num_handles);
   for (uint32_t i = 0; i < num_handles; i++) {
     scoped_refptr<Dispatcher> dispatcher = GetDispatcher(handles[i]);
     if (!dispatcher.get())
       return MOJO_RESULT_INVALID_ARGUMENT;
     dispatchers.push_back(dispatcher);
   }
@@ skipping 22 matching lines @@
   // |Wait()|/|WaitMany()| call. (Only after doing this can |waiter| be
   // destroyed, but this would still be required if the waiter were in TLS.)
   for (i = 0; i < num_added; i++)
     dispatchers[i]->RemoveWaiter(&waiter);
 
   return rv;
 }
 
 }  // namespace system
 }  // namespace mojo
+

[viettrungluu, 2014/04/09 23:06:44]

nit: Please don't add trailing blank lines.

[DaveMoore, 2014/04/10 18:58:58]

Done.