content: Pass IOSurface references using Mach IPC.

content/browser/browser_io_surface_manager_mac.cc

Index: content/browser/browser_io_surface_manager_mac.cc
diff --git a/content/browser/browser_io_surface_manager_mac.cc b/content/browser/browser_io_surface_manager_mac.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8b08fdacc62ec77d7ca5361e80b839f083064567
--- /dev/null
+++ b/content/browser/browser_io_surface_manager_mac.cc
@@ -0,0 +1,266 @@
+// Copyright 2015 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 "content/browser/browser_io_surface_manager_mac.h"
+
+#include <bsm/libbsm.h>

[Robert Sesek, 2015/05/15 21:45:05]

Unused.

[reveman, 2015/05/18 18:15:38]

Done.

+#include <servers/bootstrap.h>
+
+#include <string>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/mac/foundation_util.h"
+#include "base/mac/mach_logging.h"
+#include "base/strings/stringprintf.h"
+#include "content/browser/gpu/browser_gpu_channel_host_factory.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
+#include "content/common/mac/io_surface_manager.h"
+#include "content/common/mac/io_surface_manager_messages.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace content {
+
+// static
+BrowserIOSurfaceManager* BrowserIOSurfaceManager::GetInstance() {
+  return Singleton<BrowserIOSurfaceManager,
+                   LeakySingletonTraits<BrowserIOSurfaceManager>>::get();
+}
+
+bool BrowserIOSurfaceManager::RegisterIOSurface(int io_surface_id,
+                                                int client_id,
+                                                IOSurfaceRef io_surface) {
+  base::AutoLock lock(lock_);
+
+  IOSurfaceMapKey key(io_surface_id, client_id);
+  DCHECK(io_surfaces_.find(key) == io_surfaces_.end());

[Robert Sesek, 2015/05/15 21:45:04]

This DCHECK will continue in release mode (same bug you spotted in MachBroker).
Should this be a return false, too?

[reveman, 2015/05/18 18:15:38]

This is for in-process use only. e.g. the browser compositor. it's a browser bug
if this ever fails. A renderer process can never trigger this.

+  io_surfaces_.add(key, make_scoped_ptr(new base::mac::ScopedMachSendRight(
+                            IOSurfaceCreateMachPort(io_surface))));
+  return true;
+}
+
+void BrowserIOSurfaceManager::UnregisterIOSurface(int io_surface_id,
+                                                  int client_id) {
+  base::AutoLock lock(lock_);
+
+  IOSurfaceMapKey key(io_surface_id, client_id);
+  DCHECK(io_surfaces_.find(key) != io_surfaces_.end());

[Robert Sesek, 2015/05/15 21:45:04]

Same.

[reveman, 2015/05/18 18:15:38]

Same reply.

+  io_surfaces_.erase(key);
+}
+
+IOSurfaceRef BrowserIOSurfaceManager::AcquireIOSurface(int io_surface_id) {
+  base::AutoLock lock(lock_);
+
+  IOSurfaceMapKey key(
+      io_surface_id,
+      BrowserGpuChannelHostFactory::instance()->GetGpuChannelId());
+  auto it = io_surfaces_.find(key);
+  if (it == io_surfaces_.end()) {
+    LOG(ERROR) << "Invalid Id for IOSurface " << io_surface_id;
+    return nullptr;
+  }
+
+  return IOSurfaceLookupFromMachPort(it->second->get());
+}
+
+gpu::Mailbox BrowserIOSurfaceManager::GenerateChildProcessMailbox(
+    int client_id) {
+  base::AutoLock lock(lock_);
+
+  gpu::Mailbox mailbox = gpu::Mailbox::Generate();
+  DCHECK(mailbox.Verify());
+  child_process_ids_[mailbox] = client_id;
+  return mailbox;
+}
+
+void BrowserIOSurfaceManager::InvalidateChildProcessMailbox(
+    const gpu::Mailbox& mailbox) {
+  base::AutoLock lock(lock_);
+
+  DCHECK(child_process_ids_.find(mailbox) != child_process_ids_.end());
+  child_process_ids_.erase(mailbox);
+}
+
+gpu::Mailbox BrowserIOSurfaceManager::GetGpuProcessMailbox() const {
+  DCHECK(gpu_process_mailbox_.Verify());
+  return gpu_process_mailbox_;
+}
+
+BrowserIOSurfaceManager::BrowserIOSurfaceManager()
+    : gpu_process_mailbox_(gpu::Mailbox::Generate()) {

[Robert Sesek, 2015/05/15 21:45:04]

DCHECK(gpu_process_mailbox_.Verify()) ?

[reveman, 2015/05/18 18:15:38]

Done. Removed the DCHECK in GetGpuProcessMailbox() and moved it here instead as
it makes sense to check that we never have this service running with an invalid
mailbox.

FYI, also made gpu_process_mailbox_ const.

+  // Check in with launchd and publish the service name.
+  std::string mach_port_name = base::StringPrintf(
+      "%s.iosurfacemgr.%d", base::mac::BaseBundleID(), getpid());
+  mach_port_t port;
+  kern_return_t kr =
+      bootstrap_check_in(bootstrap_port, mach_port_name.c_str(), &port);
+  if (kr != KERN_SUCCESS) {
+    BOOTSTRAP_LOG(ERROR, kr) << "bootstrap_check_in";
+    return;
+  }
+  server_port_.reset(port);
+
+  // Start the dispatch source.
+  std::string queue_name =
+      base::StringPrintf("%s.IOSurfaceManager", base::mac::BaseBundleID());
+  dispatch_source_.reset(
+      new base::DispatchSourceMach(queue_name.c_str(), server_port_.get(), ^{
+        HandleRequest();
+      }));
+  dispatch_source_->Resume();
+}
+
+BrowserIOSurfaceManager::~BrowserIOSurfaceManager() {
+}
+
+void BrowserIOSurfaceManager::HandleRequest() {
+  struct {
+    union {
+      mach_msg_header_t header;
+      struct {
+        mach_msg_header_t header;
+        mach_msg_body_t body;
+        IOSurfaceManagerHostMsg_RegisterIOSurface msg;

[Robert Sesek, 2015/05/15 21:45:04]

This entire struct is the message. The struct declarations should have the
header, body, and mailbox fields in them.

[reveman, 2015/05/18 18:15:38]

Done by using mailbox_name[64] in the message struct instead of gpu::Mailbox.

+        gpu::Mailbox mailbox;
+      } register_io_surface;

[Robert Sesek, 2015/05/15 21:45:04]

These message structs should be in a common file, so that the client and server
can use the same declaration. It's easier to maintain that way.

[reveman, 2015/05/18 18:15:38]

Done.

[reveman, 2015/05/18 18:15:38]

Done.

+      struct {
+        mach_msg_header_t header;
+        mach_msg_body_t body;
+        IOSurfaceManagerHostMsg_UnregisterIOSurface msg;
+        gpu::Mailbox mailbox;
+      } unregister_io_surface;
+      struct {
+        mach_msg_header_t header;
+        mach_msg_body_t body;
+        IOSurfaceManagerHostMsg_AcquireIOSurface msg;
+        gpu::Mailbox mailbox;
+      } acquire_io_surface;
+    } msg;
+    mach_msg_trailer_t trailer;
+  } data = {{{0}}};
+  data.msg.header.msgh_size = sizeof(data);
+  data.msg.header.msgh_local_port = server_port_.get();
+
+  kern_return_t kr =
+      mach_msg(&data.msg.header, MACH_RCV_MSG, 0, sizeof(data), server_port_,
+               MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
+  if (kr != KERN_SUCCESS) {
+    MACH_LOG(ERROR, kr) << "mach_msg";
+    return;
+  }
+
+  switch (data.msg.header.msgh_id) {
+    case IOSurfaceManagerHostMsg_RegisterIOSurface::ID:
+      OnRegisterIOSurface(data.msg.header,
+                          data.msg.register_io_surface.msg.io_surface_id,
+                          data.msg.register_io_surface.msg.client_id,
+                          data.msg.register_io_surface.msg.io_surface_port.name,
+                          data.msg.register_io_surface.mailbox);
+      break;
+    case IOSurfaceManagerHostMsg_UnregisterIOSurface::ID:
+      OnUnregisterIOSurface(data.msg.header,
+                            data.msg.unregister_io_surface.msg.io_surface_id,
+                            data.msg.unregister_io_surface.msg.client_id,
+                            data.msg.unregister_io_surface.mailbox);
+      break;
+    case IOSurfaceManagerHostMsg_AcquireIOSurface::ID:
+      OnAcquireIOSurface(data.msg.header,
+                         data.msg.acquire_io_surface.msg.io_surface_id,
+                         data.msg.acquire_io_surface.mailbox);
+      break;
+  }
+}
+
+void BrowserIOSurfaceManager::OnRegisterIOSurface(
+    const mach_msg_header_t& header,
+    int io_surface_id,
+    int client_id,
+    mach_port_t io_surface_port,
+    const gpu::Mailbox& mailbox) {
+  base::AutoLock lock(lock_);
+
+  if (mailbox != gpu_process_mailbox_) {
+    LOG(ERROR) << "Illegal message from non-GPU process!";
+    return;
+  }
+
+  IOSurfaceMapKey key(io_surface_id, client_id);
+  io_surfaces_.add(
+      key,
+      make_scoped_ptr(new base::mac::ScopedMachSendRight(io_surface_port)));
+
+  struct {
+    mach_msg_header_t header;
+    mach_msg_body_t body;
+    IOSurfaceManagerMsg_RegisterIOSurfaceReply msg;
+  } reply = {{0}};
+  reply.header.msgh_bits = MACH_MSGH_BITS_REMOTE(header.msgh_bits);
+  reply.header.msgh_remote_port = header.msgh_remote_port;
+  reply.header.msgh_size = sizeof(reply);
+  reply.msg.result = true;
+  kern_return_t kr =
+      mach_msg(&reply.header, MACH_SEND_MSG | MACH_SEND_TIMEOUT, sizeof(reply),
+               0, MACH_PORT_NULL, 100 /*milliseconds*/, MACH_PORT_NULL);
+  if (kr != KERN_SUCCESS) {
+    MACH_LOG(ERROR, kr) << "mach_msg";
+  }
+}
+
+void BrowserIOSurfaceManager::OnUnregisterIOSurface(
+    const mach_msg_header_t& header,
+    int io_surface_id,
+    int client_id,
+    const gpu::Mailbox& mailbox) {
+  base::AutoLock lock(lock_);
+
+  if (mailbox != gpu_process_mailbox_) {
+    LOG(ERROR) << "Illegal message from non-GPU process!";
+    return;
+  }
+
+  IOSurfaceMapKey key(io_surface_id, client_id);
+  io_surfaces_.erase(key);
+}

[Robert Sesek, 2015/05/15 21:45:05]

Registration gets a reply but unregistration does not? Why?

[reveman, 2015/05/18 18:15:38]

Registration needs to be synchronous to prevent the child process from trying to
acquire the IOSurface before it reached the browser.

Unregistration happens as a result of the child process who owns the iosurface
terminating or it notifying the browser that the iosurface is no longer used
(the renderer side of it has been deleted) this notification is asynchronous in
nature and there's no guarantee for how quickly it is actually removed from the
io surface manager on the browser side.

So a reply doesn't serve any purpose here and I prefer to avoid another
round-trip for performance reasons but I'm fine adding it for consistency if you
think that makes sense?

[Robert Sesek, 2015/05/19 23:26:23]

Makes sense. It's fine to leave this as a one-way message, but I'd leave a
comment about that.

+
+void BrowserIOSurfaceManager::OnAcquireIOSurface(
+    const mach_msg_header_t& header,
+    int io_surface_id,
+    const gpu::Mailbox& mailbox) {
+  base::AutoLock lock(lock_);
+
+  auto child_process_id_it = child_process_ids_.find(mailbox);
+  if (child_process_id_it == child_process_ids_.end()) {
+    LOG(ERROR) << "Illegal message from non-child process!";
+    return;
+  }
+
+  IOSurfaceMapKey key(io_surface_id, child_process_id_it->second);
+  auto it = io_surfaces_.find(key);
+  if (it == io_surfaces_.end()) {
+    LOG(ERROR) << "Invalid Id for IOSurface " << io_surface_id;
+    return;
+  }
+
+  struct {
+    mach_msg_header_t header;
+    mach_msg_body_t body;
+    IOSurfaceManagerMsg_AcquireIOSurfaceReply msg;
+  } reply = {{0}};
+  reply.header.msgh_bits =
+      MACH_MSGH_BITS_REMOTE(header.msgh_bits) | MACH_MSGH_BITS_COMPLEX;
+  reply.header.msgh_remote_port = header.msgh_remote_port;
+  reply.header.msgh_size = sizeof(reply);
+  reply.body.msgh_descriptor_count = 1;
+  reply.msg.io_surface_port.name = it->second->get();
+  reply.msg.io_surface_port.disposition = MACH_MSG_TYPE_COPY_SEND;
+  reply.msg.io_surface_port.type = MACH_MSG_PORT_DESCRIPTOR;
+  kern_return_t kr =
+      mach_msg(&reply.header, MACH_SEND_MSG | MACH_SEND_TIMEOUT, sizeof(reply),
+               0, MACH_PORT_NULL, 100 /*milliseconds*/, MACH_PORT_NULL);
+  if (kr != KERN_SUCCESS) {
+    MACH_LOG(ERROR, kr) << "mach_msg";
+  }
+}
+
+}  // namespace content