discardable_memory: Using mojo IPC to replace Chrome IPC

components/discardable_memory/service/discardable_shared_memory_manager.cc

 // Copyright 2014 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 "components/discardable_memory/service/discardable_shared_memory_manager.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/atomic_sequence_num.h"
@@ skipping 11 matching lines @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/memory_allocator_dump.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "components/discardable_memory/common/discardable_shared_memory_heap.h"
+#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 
 #if defined(OS_LINUX)
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/metrics/histogram_macros.h"
 #endif
 
 namespace discardable_memory {
 namespace {
 
 const char kSingleProcess[] = "single-process";
 
 const int kInvalidUniqueClientID = -1;
 
 const uint64_t kBrowserTracingProcessId = std::numeric_limits<uint64_t>::max();
 
 uint64_t ClientProcessUniqueIdToTracingProcessId(int client_id) {
   // TODO(penghuang): Move this function to right place.
   // https://crbug.com/661257
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(kSingleProcess))
     return kBrowserTracingProcessId;
   // The hash value is incremented so that the tracing id is never equal to
   // MemoryDumpManager::kInvalidTracingProcessId.
   return static_cast<uint64_t>(base::Hash(
              reinterpret_cast<const char*>(&client_id), sizeof(client_id))) +
          1;
 }
 
+class DiscardableSharedMemoryManagerProxy

[reveman, 2016/11/23 17:07:44]

Please add a comment above this class describing what it's used for? The "Proxy"
suffix has confused me a couple of times when reviewing this code. It's
typically used on the client side while here it's used on the service side
instead. What do you think about calling this DiscardableSharedMemoryManagerHost
or HostDiscardableSharedMemoryManager instead?

[Peng, 2016/11/24 15:04:37]

How about MojoDiscardableSharedMemoryManagerImpl?

Done

+    : public mojom::DiscardableSharedMemoryManager {
+ public:
+  DiscardableSharedMemoryManagerProxy(
+      int32_t client_id,
+      ::discardable_memory::DiscardableSharedMemoryManager* manager)
+      : client_id_(client_id), manager_(manager) {}
+
+  ~DiscardableSharedMemoryManagerProxy() override {
+    manager_->ClientRemoved(client_id_);
+  }
+
+  // mojom::DiscardableSharedMemoryManager overrides:
+  void AllocateLockedDiscardableSharedMemory(
+      uint32_t size,
+      int32_t id,
+      const AllocateLockedDiscardableSharedMemoryCallback& callback) override {
+    base::SharedMemoryHandle handle;
+    manager_->AllocateLockedDiscardableSharedMemoryForClient(client_id_, size,
+                                                             id, &handle);
+    mojo::ScopedSharedBufferHandle memory;
+    if (handle != base::SharedMemory::NULLHandle()) {

[dcheng, 2016/11/25 00:07:08]

Is it necessary to check this condition? I would expect WrapSharedMemoryHandle()
to Just Work.

[Peng, 2016/11/25 16:41:53]

Done.

+      memory =
+          mojo::WrapSharedMemoryHandle(handle, size, false /* ready_only */);
+    }
+    return callback.Run(std::move(memory));
+  }
+
+  void DeletedDiscardableSharedMemory(int32_t id) override {
+    manager_->ClientDeletedDiscardableSharedMemory(id, client_id_);
+  }
+
+ private:
+  const int32_t client_id_;
+  ::discardable_memory::DiscardableSharedMemoryManager* const manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(DiscardableSharedMemoryManagerProxy);
+};
+
 class DiscardableMemoryImpl : public base::DiscardableMemory {
  public:
   DiscardableMemoryImpl(
       std::unique_ptr<base::DiscardableSharedMemory> shared_memory,
       const base::Closure& deleted_callback)
       : shared_memory_(std::move(shared_memory)),
         deleted_callback_(deleted_callback),
         is_locked_(true) {}
 
   ~DiscardableMemoryImpl() override {
@@ skipping 103 matching lines @@
 
 }  // namespace
 
 DiscardableSharedMemoryManager::MemorySegment::MemorySegment(
     std::unique_ptr<base::DiscardableSharedMemory> memory)
     : memory_(std::move(memory)) {}
 
 DiscardableSharedMemoryManager::MemorySegment::~MemorySegment() {}
 
 DiscardableSharedMemoryManager::DiscardableSharedMemoryManager()
-    : default_memory_limit_(GetDefaultMemoryLimit()),
+    : last_client_id_(0),
+      default_memory_limit_(GetDefaultMemoryLimit()),
       memory_limit_(default_memory_limit_),
       bytes_allocated_(0),
       memory_pressure_listener_(new base::MemoryPressureListener(
           base::Bind(&DiscardableSharedMemoryManager::OnMemoryPressure,
                      base::Unretained(this)))),
       // Current thread might not have a task runner in tests.
       enforce_memory_policy_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       enforce_memory_policy_pending_(false),
       weak_ptr_factory_(this) {
   DCHECK_NE(memory_limit_, 0u);
   enforce_memory_policy_callback_ =
       base::Bind(&DiscardableSharedMemoryManager::EnforceMemoryPolicy,
                  weak_ptr_factory_.GetWeakPtr());
   base::trace_event::MemoryDumpManager::GetInstance()->RegisterDumpProvider(
       this, "DiscardableSharedMemoryManager",
       base::ThreadTaskRunnerHandle::Get());
   base::MemoryCoordinatorClientRegistry::GetInstance()->Register(this);
 }
 
 DiscardableSharedMemoryManager::~DiscardableSharedMemoryManager() {
   base::trace_event::MemoryDumpManager::GetInstance()->UnregisterDumpProvider(
       this);
 }
 
+// static
+DiscardableSharedMemoryManager*
+DiscardableSharedMemoryManager::CreateInstance() {
+  DCHECK(g_discardable_shared_memory_manager == nullptr);

[reveman, 2016/11/23 17:07:44]

nit: DCHECK(!g_discardable_shared_memory_manager)

[Peng, 2016/11/24 15:04:37]

It doesn't work. Because base::LazyInstance only overrides operator==.

+  return g_discardable_shared_memory_manager.Pointer();
+}
+
+// static
 DiscardableSharedMemoryManager* DiscardableSharedMemoryManager::current() {
+  DCHECK(!(g_discardable_shared_memory_manager == nullptr));

[reveman, 2016/11/23 17:07:44]

nit: DCHECK(g_discardable_shared_memory_manager)

[Peng, 2016/11/24 15:04:37]

It doesn't work. Because base::LazyInstance only overrides operator==.

   return g_discardable_shared_memory_manager.Pointer();
 }
 
+// static
+void DiscardableSharedMemoryManager::Bind(
+    mojom::DiscardableSharedMemoryManagerRequest request) {
+  auto* manager = current();
+  mojo::MakeStrongBinding(base::MakeUnique<DiscardableSharedMemoryManagerProxy>(
+                              ++manager->last_client_id_, manager),
+                          std::move(request));
+}
+
 std::unique_ptr<base::DiscardableMemory>
 DiscardableSharedMemoryManager::AllocateLockedDiscardableMemory(size_t size) {
   DCHECK_NE(size, 0u);
 
-  DiscardableSharedMemoryId new_id =
-      g_next_discardable_shared_memory_id.GetNext();
-  base::ProcessHandle current_process_handle = base::GetCurrentProcessHandle();
+  int32_t new_id = g_next_discardable_shared_memory_id.GetNext();
 
   // Note: Use DiscardableSharedMemoryHeap for in-process allocation
   // of discardable memory if the cost of each allocation is too high.
   base::SharedMemoryHandle handle;
-  AllocateLockedDiscardableSharedMemory(
-      current_process_handle, kInvalidUniqueClientID, size, new_id, &handle);
+  AllocateLockedDiscardableSharedMemory(kInvalidUniqueClientID, size, new_id,
+                                        &handle);
   std::unique_ptr<base::DiscardableSharedMemory> memory(
       new base::DiscardableSharedMemory(handle));
   if (!memory->Map(size))
     base::TerminateBecauseOutOfMemory(size);
   // Close file descriptor to avoid running out.
   memory->Close();
   return base::MakeUnique<DiscardableMemoryImpl>(
       std::move(memory),
       base::Bind(
           &DiscardableSharedMemoryManager::DeletedDiscardableSharedMemory,
@@ skipping 66 matching lines @@
                         static_cast<uint64_t>(resident_size));
       }
 #endif  // defined(COUNT_RESIDENT_BYTES_SUPPORTED)
     }
   }
   return true;
 }
 
 void DiscardableSharedMemoryManager::
     AllocateLockedDiscardableSharedMemoryForClient(
-        base::ProcessHandle process_handle,
         int client_id,
         size_t size,
-        DiscardableSharedMemoryId id,
+        int32_t id,
         base::SharedMemoryHandle* shared_memory_handle) {
-  AllocateLockedDiscardableSharedMemory(process_handle, client_id, size, id,
+  AllocateLockedDiscardableSharedMemory(client_id, size, id,
                                         shared_memory_handle);
 }
 
 void DiscardableSharedMemoryManager::ClientDeletedDiscardableSharedMemory(
-    DiscardableSharedMemoryId id,
+    int32_t id,
     int client_id) {
   DeletedDiscardableSharedMemory(id, client_id);
 }
 
 void DiscardableSharedMemoryManager::ClientRemoved(int client_id) {
   base::AutoLock lock(lock_);
 
   auto it = clients_.find(client_id);
   if (it == clients_.end())
     return;
@@ skipping 41 matching lines @@
     case base::MemoryState::SUSPENDED:
     // Note that SUSPENDED never occurs in the main browser process so far.
     // Fall through.
     case base::MemoryState::UNKNOWN:
       NOTREACHED();
       break;
   }
 }
 
 void DiscardableSharedMemoryManager::AllocateLockedDiscardableSharedMemory(
-    base::ProcessHandle process_handle,
     int client_id,
     size_t size,
-    DiscardableSharedMemoryId id,
+    int32_t id,
     base::SharedMemoryHandle* shared_memory_handle) {
   base::AutoLock lock(lock_);
 
   // Make sure |id| is not already in use.
   MemorySegmentMap& client_segments = clients_[client_id];
   if (client_segments.find(id) != client_segments.end()) {
     LOG(ERROR) << "Invalid discardable shared memory ID";
     *shared_memory_handle = base::SharedMemory::NULLHandle();
     return;
   }
@@ skipping 12 matching lines @@
   if (bytes_allocated_ > limit)
     ReduceMemoryUsageUntilWithinLimit(limit);
 
   std::unique_ptr<base::DiscardableSharedMemory> memory(
       new base::DiscardableSharedMemory);
   if (!memory->CreateAndMap(size)) {
     *shared_memory_handle = base::SharedMemory::NULLHandle();
     return;
   }
 
-  if (!memory->ShareToProcess(process_handle, shared_memory_handle)) {
-    LOG(ERROR) << "Cannot share discardable memory segment";
-    *shared_memory_handle = base::SharedMemory::NULLHandle();
-    return;
-  }
+  *shared_memory_handle = base::SharedMemory::DuplicateHandle(memory->handle());

[dcheng, 2016/11/25 00:07:08]

Nit: I would just DuplicateHandle up in the Proxy (then we don't need to be very
careful about our handle management here)

[Peng, 2016/11/25 16:41:53]

Because the handle inside of memory will be closed (line478) before returning
from this function. So we have to dup it here.

[dcheng, 2016/11/28 18:26:54]

Hmm... I was hoping we could wrap this in sort of scoper to be a bit safer about
closing the handle when it's needed...

Can we just move the DuplicateHandle() to line 483 so we don't need the extra
CloseHandle() call at least?

[Peng, 2016/11/29 15:20:13]

Done.

 
   // Close file descriptor to avoid running out.
   memory->Close();
 
   base::CheckedNumeric<size_t> checked_bytes_allocated = bytes_allocated_;
   checked_bytes_allocated += memory->mapped_size();
   if (!checked_bytes_allocated.IsValid()) {
+    base::SharedMemory::CloseHandle(*shared_memory_handle);
     *shared_memory_handle = base::SharedMemory::NULLHandle();
     return;
   }
 
   bytes_allocated_ = checked_bytes_allocated.ValueOrDie();
   BytesAllocatedChanged(bytes_allocated_);
 
   scoped_refptr<MemorySegment> segment(new MemorySegment(std::move(memory)));
   client_segments[id] = segment.get();
   segments_.push_back(segment.get());
   std::push_heap(segments_.begin(), segments_.end(), CompareMemoryUsageTime);
 
   if (bytes_allocated_ > memory_limit_)
     ScheduleEnforceMemoryPolicy();
 }
 
 void DiscardableSharedMemoryManager::DeletedDiscardableSharedMemory(
-    DiscardableSharedMemoryId id,
+    int32_t id,
     int client_id) {
   base::AutoLock lock(lock_);
 
   MemorySegmentMap& client_segments = clients_[client_id];
 
   MemorySegmentMap::iterator segment_it = client_segments.find(id);
   if (segment_it == client_segments.end()) {
     LOG(ERROR) << "Invalid discardable shared memory ID";
     return;
   }
@@ skipping 122 matching lines @@
     return;
 
   enforce_memory_policy_pending_ = true;
   DCHECK(enforce_memory_policy_task_runner_);
   enforce_memory_policy_task_runner_->PostDelayedTask(
       FROM_HERE, enforce_memory_policy_callback_,
       base::TimeDelta::FromMilliseconds(kEnforceMemoryPolicyDelayMs));
 }
 
 }  // namespace discardable_memory