discardable_memory: Using mojo IPC to replace Chrome IPC

components/discardable_memory/client/client_discardable_shared_memory_manager.cc

Index: components/discardable_memory/client/client_discardable_shared_memory_manager.cc
diff --git a/components/discardable_memory/client/client_discardable_shared_memory_manager.cc b/components/discardable_memory/client/client_discardable_shared_memory_manager.cc
index 22239e13869644b0243684a01e7a3c60eea03de1..4f555b6f320ad712f0fd9cb2c6c5fafc8641fc3a 100644
--- a/components/discardable_memory/client/client_discardable_shared_memory_manager.cc
+++ b/components/discardable_memory/client/client_discardable_shared_memory_manager.cc
@@ -21,9 +21,11 @@
 #include "base/process/process_metrics.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/trace_event.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 
 namespace discardable_memory {
 namespace {
@@ -82,20 +84,152 @@ class DiscardableMemoryImpl : public base::DiscardableMemory {
   DISALLOW_COPY_AND_ASSIGN(DiscardableMemoryImpl);
 };
 
-void SendDeletedDiscardableSharedMemoryMessage(
-    ClientDiscardableSharedMemoryManager::Delegate* delegate,
-    DiscardableSharedMemoryId id) {
-  delegate->DeletedDiscardableSharedMemory(id);
+}  // namespace
+
+// A wrapper for mojo interface DiscardableSharedMemroyManager. It makes
+// the mojo interface thread safe.
+class ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy
+    : public base::RefCountedThreadSafe<ThreadSafeManagerProxy> {
+ public:
+  ThreadSafeManagerProxy(mojom::DiscardableSharedMemoryManagerPtrInfo info,
+                         base::SingleThreadTaskRunner* io_task_runner);
+  std::unique_ptr<base::DiscardableSharedMemory> Allocate(size_t size,
+                                                          int32_t id);
+  void Deleted(int32_t id);
+
+ private:
+  friend class base::RefCountedThreadSafe<ThreadSafeManagerProxy>;
+
+  ~ThreadSafeManagerProxy() {}
+
+  void OnConnectionError();
+  void InitInIOThread(mojom::DiscardableSharedMemoryManagerPtrInfo info);
+  void AllocateInIOThread(
+      size_t size,
+      int32_t id,
+      std::unique_ptr<base::DiscardableSharedMemory>* memory,
+      base::WaitableEvent* event);
+  void OnAllocateCompletedInIOThread(
+      std::unique_ptr<base::DiscardableSharedMemory>* memory,
+      base::WaitableEvent* event,
+      mojo::ScopedSharedBufferHandle mojo_handle);
+  void DeletedInIOThread(int32_t id);
+
+  scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
+  mojom::DiscardableSharedMemoryManagerPtr manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThreadSafeManagerProxy);
+};
+
+ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+    ThreadSafeManagerProxy(mojom::DiscardableSharedMemoryManagerPtrInfo info,
+                           base::SingleThreadTaskRunner* io_task_runner)
+    : io_task_runner_(io_task_runner) {
+  io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&ClientDiscardableSharedMemoryManager::
+                                ThreadSafeManagerProxy::InitInIOThread,
+                            this, base::Passed(&info)));
 }
 
-}  // namespace
+void ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+    OnConnectionError() {
+  manager_.reset();
+}
+
+void ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+    InitInIOThread(mojom::DiscardableSharedMemoryManagerPtrInfo info) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  manager_.Bind(std::move(info));
+  manager_.set_connection_error_handler(
+      base::Bind(&ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+                     OnConnectionError,
+                 this));
+}
+
+std::unique_ptr<base::DiscardableSharedMemory>
+ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::Allocate(
+    size_t size,
+    int32_t id) {
+  DCHECK(!io_task_runner_->BelongsToCurrentThread());
+  std::unique_ptr<base::DiscardableSharedMemory> memory;
+  base::WaitableEvent event(base::WaitableEvent::ResetPolicy::MANUAL,
+                            base::WaitableEvent::InitialState::NOT_SIGNALED);
+  io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&ClientDiscardableSharedMemoryManager::
+                                ThreadSafeManagerProxy::AllocateInIOThread,
+                            this, size, id, &memory, &event));
+  // Waiting until IPC is finished in the IO thread.
+  event.Wait();

[reveman, 2016/11/15 22:57:15]

Why is this proxy needed when we're using a waitable event? I'm failing to see
how ClientDiscardableSharedMemoryManager could be destroyed before IO thread has
finished running this task. Can you explain how that can happen?

[Peng, 2016/11/16 19:57:03]

This function is fine, but the
ClientDiscardableSharedMemoryManager::DeletedDiscardableSharedMemory() also need
post a task to IO thread. It causes the problem.

+  return memory;
+}
+
+void ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::Deleted(
+    int32_t id) {
+  DCHECK(!io_task_runner_->BelongsToCurrentThread());
+  io_task_runner_->PostTask(
+      FROM_HERE, base::Bind(&ClientDiscardableSharedMemoryManager::
+                                ThreadSafeManagerProxy::DeletedInIOThread,
+                            this, id));
+}
+
+void ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+    AllocateInIOThread(size_t size,
+                       int32_t id,
+                       std::unique_ptr<base::DiscardableSharedMemory>* memory,
+                       base::WaitableEvent* event) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  if (!manager_)
+    return;
+  manager_->AllocateLockedDiscardableSharedMemory(
+      static_cast<uint32_t>(size), id,
+      base::Bind(&ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+                     OnAllocateCompletedInIOThread,
+                 this, memory, event));
+}
+
+void ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+    OnAllocateCompletedInIOThread(
+        std::unique_ptr<base::DiscardableSharedMemory>* memory,
+        base::WaitableEvent* event,
+        mojo::ScopedSharedBufferHandle mojo_handle) {
+  DCHECK(io_task_runner_->BelongsToCurrentThread());
+  do {
+    if (!mojo_handle.is_valid())
+      break;
+    auto handle = base::SharedMemory::NULLHandle();
+    size_t memory_size = 0;
+    bool read_only = false;
+    auto result = mojo::UnwrapSharedMemoryHandle(
+        std::move(mojo_handle), &handle, &memory_size, &read_only);
+    if (result != MOJO_RESULT_OK)
+      break;
+    auto discardable_shared_memory =
+        base::MakeUnique<base::DiscardableSharedMemory>(handle);
+    if (!discardable_shared_memory->Map(memory_size)) {
+      base::TerminateBecauseOutOfMemory(memory_size);
+      break;
+    }
+    *memory = std::move(discardable_shared_memory);
+  } while (false);
+  event->Signal();
+}
+
+void ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy::
+    DeletedInIOThread(int32_t id) {
+  if (!manager_)
+    return;
+  manager_->DeletedDiscardableSharedMemory(id);
+}
 
 ClientDiscardableSharedMemoryManager::ClientDiscardableSharedMemoryManager(
-    Delegate* delegate)
-    : heap_(base::GetPageSize()), delegate_(delegate) {
+    mojom::DiscardableSharedMemoryManagerPtrInfo info,
+    base::SingleThreadTaskRunner* io_task_runner)
+    : heap_(base::GetPageSize()) {
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
       this, "ClientDiscardableSharedMemoryManager",
       base::ThreadTaskRunnerHandle::Get());
+  manager_ = new ClientDiscardableSharedMemoryManager::ThreadSafeManagerProxy(
+      std::move(info), io_task_runner);
 }
 
 ClientDiscardableSharedMemoryManager::~ClientDiscardableSharedMemoryManager() {
@@ -181,18 +315,18 @@ ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(
       std::max(kAllocationSize / base::GetPageSize(), pages);
   size_t allocation_size_in_bytes = pages_to_allocate * base::GetPageSize();
 
-  DiscardableSharedMemoryId new_id =
-      g_next_discardable_shared_memory_id.GetNext();
+  int32_t new_id = g_next_discardable_shared_memory_id.GetNext();
 
   // Ask parent process to allocate a new discardable shared memory segment.
-  std::unique_ptr<base::DiscardableSharedMemory> shared_memory(
-      AllocateLockedDiscardableSharedMemory(allocation_size_in_bytes, new_id));
+  auto shared_memory =
+      AllocateLockedDiscardableSharedMemory(allocation_size_in_bytes, new_id);
 
   // Create span for allocated memory.
-  std::unique_ptr<DiscardableSharedMemoryHeap::Span> new_span(
-      heap_.Grow(std::move(shared_memory), allocation_size_in_bytes, new_id,
-                 base::Bind(&SendDeletedDiscardableSharedMemoryMessage,
-                            delegate_, new_id)));
+  std::unique_ptr<DiscardableSharedMemoryHeap::Span> new_span(heap_.Grow(
+      std::move(shared_memory), allocation_size_in_bytes, new_id,
+      base::Bind(
+          &ClientDiscardableSharedMemoryManager::DeletedDiscardableSharedMemory,
+          base::Unretained(this), new_id)));
   new_span->set_is_locked(true);
 
   // Unlock and insert any left over memory into free lists.
@@ -324,19 +458,17 @@ ClientDiscardableSharedMemoryManager::CreateMemoryAllocatorDump(
 std::unique_ptr<base::DiscardableSharedMemory>
 ClientDiscardableSharedMemoryManager::AllocateLockedDiscardableSharedMemory(
     size_t size,
-    DiscardableSharedMemoryId id) {
+    int32_t id) {
   TRACE_EVENT2("renderer",
                "ClientDiscardableSharedMemoryManager::"
                "AllocateLockedDiscardableSharedMemory",
                "size", size, "id", id);
+  return manager_->Allocate(size, id);
+}
 
-  base::SharedMemoryHandle handle = base::SharedMemory::NULLHandle();
-  delegate_->AllocateLockedDiscardableSharedMemory(size, id, &handle);
-  std::unique_ptr<base::DiscardableSharedMemory> memory(
-      new base::DiscardableSharedMemory(handle));
-  if (!memory->Map(size))
-    base::TerminateBecauseOutOfMemory(size);
-  return memory;
+void ClientDiscardableSharedMemoryManager::DeletedDiscardableSharedMemory(
+    int32_t id) {
+  manager_->Deleted(id);
 }
 
 void ClientDiscardableSharedMemoryManager::MemoryUsageChanged(