Make mojo_system static and mojo_system_impl a component, never use both

mojo/public/platform/native/system_thunks.h

+// Copyright 2013 The Chromium Authors. All rights reserved.

[viettrungluu, 2014/04/10 19:38:06]

nit: 2014

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef MOJO_PUBLIC_PLATFORM_NATIVE_SYSTEM_THUNKS_H_
+#define MOJO_PUBLIC_PLATFORM_NATIVE_SYSTEM_THUNKS_H_
+
+#include <stddef.h>
+
+#include "mojo/public/c/system/core.h"
+
+// The embedder needs to bind the basic Mojo Core functions of a DSO to those of
+// the embedder when loading a DSO that is dependent on mojo_system.
+// The typical usage would look like:
+// base::ScopedNativeLibrary app_library(
+//     base::LoadNativeLibrary(app_path_, &error));
+// typedef MojoResult (*MojoSetSystemThunksFn)(MojoSystemThunks*);
+// MojoSetSystemThunksFn mojo_set_system_thunks_fn =
+//     reinterpret_cast<MojoSetSystemThunksFn>(app_library.GetFunctionPointer(
+//         "MojoSetSystemThunks"));
+// MojoSystemThunks system_thunks = MojoMakeSystemThunks();
+// size_t expected_size = mojo_set_system_thunks_fn(&system_thunks);
+// if (expected_size > sizeof(MojoSystemThunks)) {
+//   LOG(ERROR)
+//       << "Invalid DSO. Expected MojoSystemThunks size: "
+//       << expected_size;
+//   break;
+// }
+
+// Structure used to bind the basic Mojo Core functions of a DSO to those of
+// the embedder.
+// This is the ABI between the embedder and the DSO. It can only have new
+// functions added to the end. No other changes are supported.
+#pragma pack(push, 8)
+struct MojoSystemThunks {
+  size_t size;  // Should be set to sizeof(MojoSystemThunks).
+  MojoTimeTicks (*GetTimeTicksNow)();
+  MojoResult (*Close)(MojoHandle handle);
+  MojoResult (*Wait)(MojoHandle handle,
+                     MojoWaitFlags flags,
+                     MojoDeadline deadline);
+  MojoResult (*WaitMany)(const MojoHandle* handles,
+                         const MojoWaitFlags* flags,
+                         uint32_t num_handles,
+                         MojoDeadline deadline);
+  MojoResult (*CreateMessagePipe)(MojoHandle* message_pipe_handle0,
+                                  MojoHandle* message_pipe_handle1);
+  MojoResult (*WriteMessage)(MojoHandle message_pipe_handle,
+                             const void* bytes,
+                             uint32_t num_bytes,
+                             const MojoHandle* handles,
+                             uint32_t num_handles,
+                             MojoWriteMessageFlags flags);
+  MojoResult (*ReadMessage)(MojoHandle message_pipe_handle,
+                            void* bytes,
+                            uint32_t* num_bytes,
+                            MojoHandle* handles,
+                            uint32_t* num_handles,
+                            MojoReadMessageFlags flags);
+  MojoResult (*CreateDataPipe)(const MojoCreateDataPipeOptions* options,
+                               MojoHandle* data_pipe_producer_handle,
+                               MojoHandle* data_pipe_consumer_handle);
+  MojoResult (*WriteData)(MojoHandle data_pipe_producer_handle,
+                          const void* elements,
+                          uint32_t* num_elements,
+                          MojoWriteDataFlags flags);
+  MojoResult (*BeginWriteData)(MojoHandle data_pipe_producer_handle,
+                               void** buffer,
+                               uint32_t* buffer_num_elements,
+                               MojoWriteDataFlags flags);
+  MojoResult (*EndWriteData)(MojoHandle data_pipe_producer_handle,
+                             uint32_t num_elements_written);
+  MojoResult (*ReadData)(MojoHandle data_pipe_consumer_handle,
+                         void* elements,
+                         uint32_t* num_elements,
+                         MojoReadDataFlags flags);
+  MojoResult (*BeginReadData)(MojoHandle data_pipe_consumer_handle,
+                              const void** buffer,
+                              uint32_t* buffer_num_elements,
+                              MojoReadDataFlags flags);
+  MojoResult (*EndReadData)(MojoHandle data_pipe_consumer_handle,
+                            uint32_t num_elements_read);
+  MojoResult (*CreateSharedBuffer)(
+      const MojoCreateSharedBufferOptions* options,
+      uint64_t num_bytes,
+      MojoHandle* shared_buffer_handle);
+  MojoResult (*DuplicateBufferHandle)(
+      MojoHandle buffer_handle,
+      const MojoDuplicateBufferHandleOptions* options,
+      MojoHandle* new_buffer_handle);
+  MojoResult (*MapBuffer)(MojoHandle buffer_handle,
+                          uint64_t offset,
+                          uint64_t num_bytes,
+                          void** buffer,
+                          MojoMapBufferFlags flags);
+  MojoResult (*UnmapBuffer)(void* buffer);
+};
+#pragma pack(pop)
+
+// Intended to be called from the embedder. Returns a |MojoCore| initialized
+// to contain pointers to each of the embedder's MojoCore functions.
+static inline const MojoSystemThunks MojoMakeSystemThunks() {

[viettrungluu, 2014/04/10 19:38:06]

Please remove both "static" and "const".

Alternatively, you could declare system_thunks to be "static const", and return
a pointer to it (a |const MojoSystemThunks*|).

+  MojoSystemThunks system_thunks {
+    sizeof(MojoSystemThunks),
+    MojoGetTimeTicksNow,
+    MojoClose,
+    MojoWait,
+    MojoWaitMany,
+    MojoCreateMessagePipe,
+    MojoWriteMessage,
+    MojoReadMessage,
+    MojoCreateDataPipe,
+    MojoWriteData,
+    MojoBeginWriteData,
+    MojoEndWriteData,
+    MojoReadData,
+    MojoBeginReadData,
+    MojoEndReadData,
+    MojoCreateSharedBuffer,
+    MojoDuplicateBufferHandle,
+    MojoMapBuffer,
+    MojoUnmapBuffer
+  };
+  return system_thunks;
+}
+
+typedef size_t (*MojoSetSystemThunksFn)(const MojoSystemThunks* mojo_core);
+
+// Always export this api.
+#if defined(WIN32)
+#define THUNK_EXPORT __declspec(dllexport)
+#else
+#define THUNK_EXPORT __attribute__((visibility("default")))
+#endif
+
+// Called to set the MojoCore functionality of a DSO. Returns the expected size.
+extern "C" THUNK_EXPORT size_t MojoSetSystemThunks(

[viettrungluu, 2014/04/10 19:38:06]

I keep on telling you to delete this (and maybe leave a comment about its
existence), and move it to the .cc file.... (Note that you can annotate the
function definition directly with extern "C", etc.)

The embedder shouldn't see this when it includes it (because it can't call it
directly, only by dlsym() or equivalent).

Also, you should note the lifetime of the now-misnamed |mojo_core| (further nit
-> system_thunks). I.e., does it have to copy it, or can it just save the
pointer? (As currently implemented, you have to copy it, but it'd be awfully
tempting to just save a pointer....)

+    const MojoSystemThunks* mojo_core);
+
+#endif  // MOJO_PUBLIC_PLATFORM_NATIVE_SYSTEM_THUNKS_H_