Replacing allocate bitmap IPC messages with a new Mojo interface.

content/child/child_shared_bitmap_manager.cc

Index: content/child/child_shared_bitmap_manager.cc
diff --git a/content/child/child_shared_bitmap_manager.cc b/content/child/child_shared_bitmap_manager.cc
index 00b461c7e96baad1d89e7d8717ff523cdaf132b4..26eefd412209ea0b8830091eb0a9a63121b78a03 100644
--- a/content/child/child_shared_bitmap_manager.cc
+++ b/content/child/child_shared_bitmap_manager.cc
@@ -15,6 +15,7 @@
 #include "build/build_config.h"
 #include "content/child/child_thread_impl.h"
 #include "content/common/child_process_messages.h"
+#include "mojo/public/cpp/system/platform_handle.h"
 #include "ui/gfx/geometry/size.h"
 
 namespace content {
@@ -23,27 +24,34 @@ namespace {
 
 class ChildSharedBitmap : public SharedMemoryBitmap {
  public:
-  ChildSharedBitmap(scoped_refptr<ThreadSafeSender> sender,
-                    base::SharedMemory* shared_memory,
-                    const cc::SharedBitmapId& id)
+  ChildSharedBitmap(
+      const scoped_refptr<mojom::ThreadSafeRenderMessageFilterAssociatedPtr>&
+          render_message_filter_ptr,
+      base::SharedMemory* shared_memory,
+      const cc::SharedBitmapId& id)
       : SharedMemoryBitmap(static_cast<uint8_t*>(shared_memory->memory()),
                            id,
                            shared_memory),
-        sender_(sender) {}
-
-  ChildSharedBitmap(scoped_refptr<ThreadSafeSender> sender,
-                    std::unique_ptr<base::SharedMemory> shared_memory_holder,
-                    const cc::SharedBitmapId& id)
-      : ChildSharedBitmap(sender, shared_memory_holder.get(), id) {
+        render_message_filter_ptr_(render_message_filter_ptr) {}
+
+  ChildSharedBitmap(
+      const scoped_refptr<mojom::ThreadSafeRenderMessageFilterAssociatedPtr>&
+          render_message_filter_ptr,
+      std::unique_ptr<base::SharedMemory> shared_memory_holder,
+      const cc::SharedBitmapId& id)
+      : ChildSharedBitmap(render_message_filter_ptr,
+                          shared_memory_holder.get(),
+                          id) {
     shared_memory_holder_ = std::move(shared_memory_holder);
   }
 
   ~ChildSharedBitmap() override {
-    sender_->Send(new ChildProcessHostMsg_DeletedSharedBitmap(id()));
+    (*render_message_filter_ptr_)->DeletedSharedBitmap(id());
   }
 
  private:
-  scoped_refptr<ThreadSafeSender> sender_;
+  scoped_refptr<mojom::ThreadSafeRenderMessageFilterAssociatedPtr>
+      render_message_filter_ptr_;
   std::unique_ptr<base::SharedMemory> shared_memory_holder_;
 };
 
@@ -79,25 +87,15 @@ SharedMemoryBitmap::SharedMemoryBitmap(uint8_t* pixels,
     : SharedBitmap(pixels, id), shared_memory_(shared_memory) {}
 
 ChildSharedBitmapManager::ChildSharedBitmapManager(
-    scoped_refptr<ThreadSafeSender> sender)
-    : sender_(sender) {
+    const scoped_refptr<mojom::ThreadSafeRenderMessageFilterAssociatedPtr>&
+        render_message_filter_ptr)
+    : render_message_filter_ptr_(render_message_filter_ptr) {
 }
 
 ChildSharedBitmapManager::~ChildSharedBitmapManager() {}
 
 std::unique_ptr<cc::SharedBitmap>
 ChildSharedBitmapManager::AllocateSharedBitmap(const gfx::Size& size) {
-  std::unique_ptr<SharedMemoryBitmap> bitmap = AllocateSharedMemoryBitmap(size);
-#if defined(OS_POSIX)
-  // Close file descriptor to avoid running out.
-  if (bitmap)
-    bitmap->shared_memory()->Close();
-#endif
-  return std::move(bitmap);
-}
-
-std::unique_ptr<SharedMemoryBitmap>
-ChildSharedBitmapManager::AllocateSharedMemoryBitmap(const gfx::Size& size) {
   TRACE_EVENT2("renderer",
                "ChildSharedBitmapManager::AllocateSharedMemoryBitmap", "width",
                size.width(), "height", size.height());
@@ -105,25 +103,10 @@ ChildSharedBitmapManager::AllocateSharedMemoryBitmap(const gfx::Size& size) {
   if (!cc::SharedBitmap::SizeInBytes(size, &memory_size))
     return std::unique_ptr<SharedMemoryBitmap>();
   cc::SharedBitmapId id = cc::SharedBitmap::GenerateId();
-  std::unique_ptr<base::SharedMemory> memory;
-#if defined(OS_POSIX)
-  base::SharedMemoryHandle handle;
-  bool send_success =
-      sender_->Send(new ChildProcessHostMsg_SyncAllocateSharedBitmap(
-          memory_size, id, &handle));
-  if (!send_success) {
-    // Callers of this method are not prepared to handle failures during
-    // shutdown. Exit immediately. This is expected behavior during the Fast
-    // Shutdown path, so use EXIT_SUCCESS. https://crbug.com/615121.
-    exit(EXIT_SUCCESS);
-  }
-  memory = base::MakeUnique<base::SharedMemory>(handle, false);
-  if (!memory->Map(memory_size))
-    CollectMemoryUsageAndDie(size, memory_size);
-#else
-  bool out_of_memory;
-  memory = ChildThreadImpl::AllocateSharedMemory(memory_size, sender_.get(),
-                                                 &out_of_memory);
+  bool out_of_memory = false;
+  std::unique_ptr<base::SharedMemory> memory =
+      ChildThreadImpl::AllocateSharedMemory(memory_size, nullptr,
+                                            &out_of_memory);
   if (!memory) {
     if (out_of_memory) {
       CollectMemoryUsageAndDie(size, memory_size);
@@ -135,14 +118,13 @@ ChildSharedBitmapManager::AllocateSharedMemoryBitmap(const gfx::Size& size) {
     }
   }
 
-  if (!memory->Map(memory_size))
+ if (!memory->Map(memory_size))
     CollectMemoryUsageAndDie(size, memory_size);
 
-  base::SharedMemoryHandle handle_to_send = memory->handle();
-  sender_->Send(new ChildProcessHostMsg_AllocatedSharedBitmap(
-      memory_size, handle_to_send, id));
-#endif
-  return base::MakeUnique<ChildSharedBitmap>(sender_, std::move(memory), id);
+  NotifyAllocatedSharedBitmap(memory.get(), id);
+
+  return base::MakeUnique<ChildSharedBitmap>(render_message_filter_ptr_,
+      std::move(memory), id);
 }
 
 std::unique_ptr<cc::SharedBitmap>
@@ -155,15 +137,30 @@ ChildSharedBitmapManager::GetSharedBitmapFromId(const gfx::Size&,
 std::unique_ptr<cc::SharedBitmap>
 ChildSharedBitmapManager::GetBitmapForSharedMemory(base::SharedMemory* mem) {
   cc::SharedBitmapId id = cc::SharedBitmap::GenerateId();
-  base::SharedMemoryHandle handle_to_send = mem->handle();
+  NotifyAllocatedSharedBitmap(mem, cc::SharedBitmap::GenerateId());
+  return base::MakeUnique<ChildSharedBitmap>(render_message_filter_ptr_,
+      std::move(mem), id);
+}
+
+// Notifies the browser process that a shared bitmap with the given ID was
+// allocated. Caller keeps ownership of |memory|.
+void ChildSharedBitmapManager::NotifyAllocatedSharedBitmap(
+    base::SharedMemory* memory,
+    const cc::SharedBitmapId& id) {
+  base::SharedMemoryHandle handle_to_send = memory->handle();

[Ken Rockot(use gerrit already), 2016/11/29 17:43:11]

This seems to correctly mirror the old behavior, but the old behavior seems
inconsistent across platforms. On POSIX we dup then send the handle, but on
Windows we just send the handle, leaving |memory| with an invalid handle. It's
probably fine because nobody tries using that handle after the fact.

In order to be consistent, I think we should just scrap the ShareToProcess
below, and instead call DuplicateHandle on memory->handle() directly, still
wrapping the result with WrapSharedMemoryHandle.

Sending a mojo shared buffer to another process always does the right thing with
respect to process handle ownership, so ShareToProcess usage should essentially
be phased out as IPCs transition to mojo.

[Jay Civelli, 2016/11/29 18:01:48]

Good point, done.

 #if defined(OS_POSIX)
-  if (!mem->ShareToProcess(base::GetCurrentProcessHandle(), &handle_to_send))
-    return std::unique_ptr<cc::SharedBitmap>();
+  if (!memory->ShareToProcess(base::GetCurrentProcessHandle(), &handle_to_send))
+    return;
 #endif
-  sender_->Send(new ChildProcessHostMsg_AllocatedSharedBitmap(
-      mem->mapped_size(), handle_to_send, id));
 
-  return base::MakeUnique<ChildSharedBitmap>(sender_, mem, id);
+  mojo::ScopedSharedBufferHandle buffer_handle =
+      mojo::WrapSharedMemoryHandle(
+          base::SharedMemory::DuplicateHandle(handle_to_send),
+          memory->mapped_size(),
+          true /* read_only */);
+
+  (*render_message_filter_ptr_)->
+      AllocatedSharedBitmap(std::move(buffer_handle), id);
 }
 
 }  // namespace content