Bitmap transport

This patch reworks bitmap transport on all platforms. Linux and Mac
are switched from serialising bitmaps over the IPC channel to using
shared memory. All platforms gain a shared memory mapping cache on the
host side.

The concept of a TransportDIB (device independent bitmap) is added to
encapsulate most of the platform specifics.

On Linux, we use SysV shared memory. This is because X shared pixmaps,
which predate POSIX SHM, can only use SysV. By using SysV between
renderer and browser, we open up the possibility to map the shared
memory directly from the renderer to the X server.

On Mac, we use POSIX shared memory. However, since this needs
filesystem access and the Mac renderer is sandboxed from the
filesystem, we add two new messages from renderer -> browser:

The first, AllocTransportDIB, synchronously creates a transport DIB in
the browser and passes a handle back to the renderer. The second,
FreeTransportDIB, asynchronously, notifies the browser that it may
close its handle to the shared memory region.

On Mac, the shared memory regions are identified by their inode
numbers on the wire. This means that the browser must keep handles
open to all the allocated shared memory regions (since an inode number
is insufficient to map the region). The alternative design is that the
renderer passes the file descriptor with each paint operation. Since
passing file descriptors is special case in the code, I felt that it
would be best to minimise their use. Creating and freeing transport
DIBs are relatively rare operations relative to paints and scrolls.

On Windows, most of the code remains the same, except that Windows now
uses the mapping cache added in this patch. This allows the browser to
maintain a shared memory mapping for a transport DIB over several
paints. Previously it mapped and unmapped for every operation, causing
lots of TLB and VM churn.

Review URL: http://codereview.chromium.org/21485


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@10071 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 446 insertions, 39 deletions

diff --git a/chrome/common/bitmap_wire_data.h b/chrome/common/bitmap_wire_data.h
deleted file mode 100644
index c4e1edd..0000000
--- a/chrome/common/bitmap_wire_data.h
+++ /dev/null
@@ -1,35 +0,0 @@
-// Copyright (c) 2006-2008 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.
-
-#ifndef CHROME_COMMON_BITMAP_WIRE_DATA_H_
-#define CHROME_COMMON_BITMAP_WIRE_DATA_H_
-
-#if defined(OS_POSIX)
-class SkBitmap;
-#endif
-
-// BitmapWireData is the type of the bitmap data which is carried from renderer
-// to browser over the wire.
-
-#if defined(OS_WIN)
-
-// On Windows, the bitmap data is carried out-of-band in a shared memory
-// segment. This is the handle to the shared memory. These handles are valid
-// only in the context of the renderer process.
-// TODO(agl): get a clarification on that last sentence. It doesn't make any
-// sense to me
-typedef HANDLE BitmapWireData;
-
-#elif defined(OS_POSIX)
-
-// On POSIX, we currently serialise the bitmap data over the wire. This will
-// change at some point when we too start using shared memory, but we wish to
-// use shared memory in a different way so this is a temporary work-around.
-// TODO(port): implement drawing with shared backing stores and replace this
-//   with an IPC no-op type.
-typedef SkBitmap BitmapWireData;
-
-#endif  // defined(OS_WIN)
-
-#endif  // CHROME_COMMON_BITMAP_WIRE_DATA_H_
diff --git a/chrome/common/common.scons b/chrome/common/common.scons
index 06a9888..b2cc936 100644
--- a/chrome/common/common.scons
+++ b/chrome/common/common.scons
@@ -275,6 +275,21 @@ if not env.Bit('posix'):
       'ipc_channel_posix.cc',
   )
 
+if env.Bit('windows'):
+  input_files.Append(
+      'transport_dib_win.cc'
+  )
+
+if env.Bit('linux'):
+  input_files.Append(
+      'transport_dib_linux.cc'
+  )
+
+if env.Bit('mac'):
+  input_files.Append(
+      'transport_dib_mac.cc'
+  )
+
 if not env.Bit('mac'):
   # TODO(port): This should be enabled for all platforms.
   env.ChromeLibrary('common', input_files)
diff --git a/chrome/common/common.vcproj b/chrome/common/common.vcproj
index 8ac8d29..67a664e 100644
--- a/chrome/common/common.vcproj
+++ b/chrome/common/common.vcproj
@@ -537,7 +537,7 @@
 			RelativePath=".\notification_details.h"
 			>
 		</File>
-                <File
+		<File
 			RelativePath=".\notification_observer.h"
 			>
 		</File>
@@ -742,6 +742,14 @@
 			>
 		</File>
 		<File
+			RelativePath=".\transport_dib.h"
+			>
+		</File>
+		<File
+			RelativePath=".\transport_dib_win.cc"
+			>
+		</File>
+		<File
 			RelativePath=".\unzip.cc"
 			>
 		</File>
diff --git a/chrome/common/ipc_maybe.h b/chrome/common/ipc_maybe.h
new file mode 100644
index 0000000..de45dc0
--- /dev/null
+++ b/chrome/common/ipc_maybe.h
@@ -0,0 +1,19 @@
+#ifndef CHROME_COMMON_MAYBE_H_
+#define CHROME_COMMON_MAYBE_H_
+
+namespace IPC {
+
+// The Maybe type can be used to avoid serialising a type when it's invalid.
+// This is most useful in conjunction with FileDescriptor, as there's no
+// possible invalid value which can be serialised (one can type to use -1, but
+// the IPC channel will fail). It may also be useful if the invalid value is
+// otherwise expensive to serialise.
+template<typename A>
+struct Maybe {
+  bool valid;
+  A value;
+};
+
+}  // namespace IPC
+
+#endif  // CHROME_COMMON_MAYBE_H_
diff --git a/chrome/common/ipc_message_utils.h b/chrome/common/ipc_message_utils.h
index ae42bb0..428c889 100644
--- a/chrome/common/ipc_message_utils.h
+++ b/chrome/common/ipc_message_utils.h
@@ -15,8 +15,10 @@
 #if defined(OS_POSIX)
 #include "chrome/common/file_descriptor_set_posix.h"
 #endif
+#include "chrome/common/ipc_maybe.h"
 #include "chrome/common/ipc_sync_message.h"
 #include "chrome/common/thumbnail_score.h"
+#include "chrome/common/transport_dib.h"
 #include "webkit/glue/cache_manager.h"
 #include "webkit/glue/console_message_level.h"
 #include "webkit/glue/find_in_page_request.h"
@@ -1045,6 +1047,55 @@ struct ParamTraits< Tuple6<A, B, C, D, E, F> > {
   }
 };
 
+#if defined(OS_WIN)
+template<>
+struct ParamTraits<TransportDIB::Id> {
+  typedef TransportDIB::Id param_type;
+  static void Write(Message* m, const param_type& p) {
+    WriteParam(m, p.handle);
+    WriteParam(m, p.sequence_num);
+  }
+  static bool Read(const Message* m, void** iter, param_type* r) {
+    return (ReadParam(m, iter, &r->handle) &&
+            ReadParam(m, iter, &r->sequence_num));
+  }
+  static void Log(const param_type& p, std::wstring* l) {
+    l->append(L"TransportDIB(");
+    LogParam(p.handle, l);
+    l->append(L", ");
+    LogParam(p.sequence_num, l);
+    l->append(L")");
+  }
+};
+#endif
+
+template<typename A>
+struct ParamTraits<Maybe<A> > {
+  typedef struct Maybe<A> param_type;
+  static void Write(Message* m, const param_type& p) {
+    WriteParam(m, p.valid);
+    if (p.valid)
+      WriteParam(m, p.value);
+  }
+  static bool Read(const Message* m, void** iter, param_type* r) {
+    if (!ReadParam(m, iter, &r->valid))
+      return false;
+
+    if (r->valid)
+      return ReadParam(m, iter, &r->value);
+    return true;
+  }
+  static void Log(const param_type& p, std::wstring* l) {
+    if (p.valid) {
+      l->append(L"Just ");
+      ParamTraits<A>::Log(p.value, l);
+    } else {
+      l->append(L"Nothing");
+    }
+
+  }
+};
+
 template <>
 struct ParamTraits<webkit_glue::WebApplicationInfo> {
   typedef webkit_glue::WebApplicationInfo param_type;
diff --git a/chrome/common/render_messages.h b/chrome/common/render_messages.h
index 8c91097..e78970e 100644
--- a/chrome/common/render_messages.h
+++ b/chrome/common/render_messages.h
@@ -15,11 +15,11 @@
 #include "base/shared_memory.h"
 #include "chrome/browser/renderer_host/resource_handler.h"
 #include "chrome/common/accessibility.h"
-#include "chrome/common/bitmap_wire_data.h"
 #include "chrome/common/filter_policy.h"
 #include "chrome/common/ipc_message_utils.h"
 #include "chrome/common/modal_dialog_event.h"
 #include "chrome/common/page_transition_types.h"
+#include "chrome/common/transport_dib.h"
 #include "googleurl/src/gurl.h"
 #include "media/audio/audio_output.h"
 #include "net/base/upload_data.h"
@@ -150,7 +150,7 @@ struct ViewHostMsg_PaintRect_Flags {
 
 struct ViewHostMsg_PaintRect_Params {
   // The bitmap to be painted into the rect given by bitmap_rect.
-  BitmapWireData bitmap;
+  TransportDIB::Id bitmap;
 
   // The position and size of the bitmap.
   gfx::Rect bitmap_rect;
@@ -183,7 +183,7 @@ struct ViewHostMsg_PaintRect_Params {
 // parameters to be reasonably put in a predefined IPC message.
 struct ViewHostMsg_ScrollRect_Params {
   // The bitmap to be painted into the rect exposed by scrolling.
-  BitmapWireData bitmap;
+  TransportDIB::Id bitmap;
 
   // The position and size of the bitmap.
   gfx::Rect bitmap_rect;
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index 02a9a43..f02f5cf 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -16,6 +16,8 @@
 #include "base/gfx/native_widget_types.h"
 #include "base/shared_memory.h"
 #include "chrome/common/ipc_message_macros.h"
+#include "chrome/common/ipc_maybe.h"
+#include "chrome/common/transport_dib.h"
 #include "skia/include/SkBitmap.h"
 #include "webkit/glue/console_message_level.h"
 #include "webkit/glue/dom_operations.h"
@@ -1168,4 +1170,19 @@ IPC_BEGIN_MESSAGES(ViewHost)
                       double /* left_channel */,
                       double /* right_channel */)
 
+#if defined(OS_MACOSX)
+  // On OSX, we cannot allocated shared memory from within the sandbox, so
+  // this call exists for the renderer to ask the browser to allocate memory
+  // on its behalf. We return a file descriptor to the POSIX shared memory.
+  IPC_SYNC_MESSAGE_CONTROL1_1(ViewHostMsg_AllocTransportDIB,
+                              size_t, /* bytes requested */
+                              IPC::Maybe<TransportDIB::Handle> /* DIB */)
+
+  // Since the browser keeps handles to the allocated transport DIBs, this
+  // message is sent to tell the browser that it may release them when the
+  // renderer is finished with them.
+  IPC_MESSAGE_CONTROL1(ViewHostMsg_FreeTransportDIB,
+                       TransportDIB::Id /* DIB id */)
+#endif
+
 IPC_END_MESSAGES(ViewHost)
diff --git a/chrome/common/transport_dib.h b/chrome/common/transport_dib.h
new file mode 100644
index 0000000..e5fc009
--- /dev/null
+++ b/chrome/common/transport_dib.h
@@ -0,0 +1,112 @@
+// Copyright (c) 2006-2009 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.
+
+#ifndef CHROME_COMMON_TRANSPORT_DIB_H_
+#define CHROME_COMMON_TRANSPORT_DIB_H_
+
+#include "base/basictypes.h"
+
+#if defined(OS_WIN) || defined(OS_MACOSX)
+#include "base/shared_memory.h"
+#endif
+
+#if defined(OS_WIN)
+#include <windows.h>
+#endif
+
+// -----------------------------------------------------------------------------
+// A TransportDIB is a block of memory that is used to transport pixels
+// from the renderer process to the browser.
+// -----------------------------------------------------------------------------
+class TransportDIB {
+ public:
+  ~TransportDIB();
+
+  // Two typedefs are defined. A Handle is the type which can be sent over
+  // the wire so that the remote side can map the transport DIB. The Id typedef
+  // is sufficient to identify the transport DIB when you know that the remote
+  // side already may have it mapped.
+#if defined(OS_WIN)
+  typedef HANDLE Handle;
+  // On Windows, the Id type includes a sequence number (epoch) to solve an ABA
+  // issue:
+  //   1) Process A creates a transport DIB with HANDLE=1 and sends to B.
+  //   2) Process B maps the transport DIB and caches 1 -> DIB.
+  //   3) Process A closes the transport DIB and creates a new one. The new DIB
+  //      is also assigned HANDLE=1.
+  //   4) Process A sends the Handle to B, but B incorrectly believes that it
+  //      already has it cached.
+  struct HandleAndSequenceNum {
+    HandleAndSequenceNum()
+        : handle(NULL),
+          sequence_num(0) {
+    }
+
+    HandleAndSequenceNum(HANDLE h, uint32 seq_num)
+        : handle(h),
+          sequence_num(seq_num) {
+    }
+
+    bool operator< (const HandleAndSequenceNum& other) const {
+      if (other.handle < handle)
+        return true;
+      if (other.sequence_num < sequence_num)
+        return true;
+      return false;
+    }
+
+    HANDLE handle;
+    uint32 sequence_num;
+  };
+  typedef HandleAndSequenceNum Id;
+#elif defined(OS_MACOSX)
+  typedef base::SharedMemoryHandle Handle;
+  // On Mac, the inode number of the backing file is used as an id.
+  typedef base::SharedMemoryId Id;
+#elif defined(OS_LINUX)
+  typedef int Handle;  // These two ints are SysV IPC shared memory keys
+  typedef int Id;
+#endif
+
+  // Create a new TransportDIB
+  //   size: the minimum size, in bytes
+  //   epoch: Windows only: a global counter. See comment above.
+  //   returns: NULL on failure
+  static TransportDIB* Create(size_t size, uint32 sequence_num);
+
+  // Map the referenced transport DIB. Returns NULL on failure.
+  static TransportDIB* Map(Handle transport_dib);
+
+  // Return a pointer to the shared memory
+  void* memory() const;
+
+  // Return the maximum size of the shared memory. This is not the amount of
+  // data which is valid, you have to know that via other means, this is simply
+  // the maximum amount that /could/ be valid.
+  size_t size() const { return size_; }
+
+  // Return the identifier which can be used to refer to this shared memory
+  // on the wire.
+  Id id() const;
+
+  // Return a handle to the underlying shared memory. This can be sent over the
+  // wire to give this transport DIB to another process.
+  Handle handle() const;
+
+ private:
+  TransportDIB();
+#if defined(OS_WIN) || defined(OS_MACOSX)
+  explicit TransportDIB(base::SharedMemoryHandle dib);
+  base::SharedMemory shared_memory_;
+  uint32 sequence_num_;
+#elif defined(OS_LINUX)
+  int key_;  // SysV shared memory id
+  void* address_;  // mapped address
+#endif
+  size_t size_;  // length, in bytes
+};
+
+class MessageLoop;
+
+#endif  // CHROME_COMMON_TRANSPORT_DIB_H_
diff --git a/chrome/common/transport_dib_linux.cc b/chrome/common/transport_dib_linux.cc
new file mode 100644
index 0000000..adecf1d
--- /dev/null
+++ b/chrome/common/transport_dib_linux.cc
@@ -0,0 +1,85 @@
+// Copyright (c) 2009 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 <errno.h>
+#include <stdlib.h>
+#include <sys/ipc.h>
+#include <sys/shm.h>
+
+#include "base/logging.h"
+#include "chrome/common/transport_dib.h"
+
+// The shmat system call uses this as it's invalid return address
+static void *const kInvalidAddress = (void*) -1;
+
+TransportDIB::TransportDIB()
+    : key_(-1),
+      address_(kInvalidAddress),
+      size_(0) {
+}
+
+TransportDIB::~TransportDIB() {
+  if (address_ != kInvalidAddress) {
+    shmdt(address_);
+    address_ = kInvalidAddress;
+  }
+}
+
+// static
+TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) {
+  // We use a mode of 0666 since the X server won't attach to memory which is
+  // 0600 since it can't know if it (as a root process) is being asked to map
+  // someone else's private shared memory region.
+  const int shmkey = shmget(IPC_PRIVATE, size, 0666);
+  if (shmkey == -1) {
+    DLOG(ERROR) << "Failed to create SysV shared memory region"
+                << " errno:" << errno;
+    return false;
+  }
+
+  void* address = shmat(shmkey, NULL /* desired address */, 0 /* flags */);
+  // Here we mark the shared memory for deletion. Since we attached it in the
+  // line above, it doesn't actually get deleted but, if we crash, this means
+  // that the kernel will automatically clean it up for us.
+  shmctl(shmkey, IPC_RMID, 0);
+  if (address == kInvalidAddress)
+    return false;
+
+  TransportDIB* dib = new TransportDIB;
+
+  dib->key_ = shmkey;
+  dib->address_ = address;
+  dib->size_ = size;
+  return dib;
+}
+
+TransportDIB* TransportDIB::Map(Handle shmkey) {
+  struct shmid_ds shmst;
+  if (shmctl(shmkey, IPC_STAT, &shmst) == -1)
+    return NULL;
+
+  void* address = shmat(shmkey, NULL /* desired address */, 0 /* flags */);
+  if (address == kInvalidAddress)
+    return NULL;
+
+  TransportDIB* dib = new TransportDIB;
+
+  dib->address_ = address;
+  dib->size_ = shmst.shm_segsz;
+  dib->key_ = shmkey;
+  return dib;
+}
+
+void* TransportDIB::memory() const {
+  DCHECK_NE(address_, kInvalidAddress);
+  return address_;
+}
+
+TransportDIB::Id TransportDIB::id() const {
+  return key_;
+}
+
+TransportDIB::Handle TransportDIB::handle() const {
+  return key_;
+}
diff --git a/chrome/common/transport_dib_mac.cc b/chrome/common/transport_dib_mac.cc
new file mode 100644
index 0000000..4fb67a7
--- /dev/null
+++ b/chrome/common/transport_dib_mac.cc
@@ -0,0 +1,63 @@
+// Copyright (c) 2009 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 "chrome/common/transport_dib.h"
+
+#include <unistd.h>
+#include <sys/stat.h>
+#include "base/shared_memory.h"
+
+TransportDIB::TransportDIB()
+    : size_(0) {
+}
+
+TransportDIB::TransportDIB(TransportDIB::Handle dib)
+    : shared_memory_(dib, false /* read write */),
+      size_(0) {
+}
+
+TransportDIB::~TransportDIB() {
+}
+
+// static
+TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) {
+  TransportDIB* dib = new TransportDIB;
+  if (!dib->shared_memory_.Create(L"", false /* read write */,
+                                  false /* do not open existing */, size)) {
+    delete dib;
+    return NULL;
+  }
+
+  dib->size_ = size;
+  return dib;
+}
+
+// static
+TransportDIB* TransportDIB::Map(TransportDIB::Handle handle) {
+  TransportDIB* dib = new TransportDIB(handle);
+  struct stat st;
+  fstat(handle.fd, &st);
+
+  if (!dib->shared_memory_.Map(st.st_size)) {
+    delete dib;
+    close(handle.fd);
+    return false;
+  }
+
+  dib->size_ = st.st_size;
+
+  return dib;
+}
+
+void* TransportDIB::memory() const {
+  return shared_memory_.memory();
+}
+
+TransportDIB::Id TransportDIB::id() const {
+  return false;
+}
+
+TransportDIB::Handle TransportDIB::handle() const {
+  return shared_memory_.handle();
+}
diff --git a/chrome/common/transport_dib_win.cc b/chrome/common/transport_dib_win.cc
new file mode 100644
index 0000000..e9ab424
--- /dev/null
+++ b/chrome/common/transport_dib_win.cc
@@ -0,0 +1,72 @@
+// Copyright (c) 2009 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 <limits>
+#include <windows.h>
+
+#include "base/logging.h"
+#include "base/sys_info.h"
+#include "chrome/common/transport_dib.h"
+
+TransportDIB::TransportDIB() {
+}
+
+TransportDIB::~TransportDIB() {
+}
+
+TransportDIB::TransportDIB(HANDLE handle)
+    : shared_memory_(handle, false /* read write */) {
+}
+
+// static
+TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) {
+  size_t allocation_granularity = base::SysInfo::VMAllocationGranularity();
+  size = size / allocation_granularity + 1;
+  size = size * allocation_granularity;
+
+  TransportDIB* dib = new TransportDIB;
+
+  if (!dib->shared_memory_.Create(L"", false /* read write */,
+                                  true /* open existing */, size)) {
+    delete dib;
+    return NULL;
+  }
+
+  dib->size_ = size;
+  dib->sequence_num_ = sequence_num;
+
+  return dib;
+}
+
+// static
+TransportDIB* TransportDIB::Map(TransportDIB::Handle handle) {
+  TransportDIB* dib = new TransportDIB(handle);
+  if (!dib->shared_memory_.Map(0 /* map whole shared memory segment */)) {
+    DLOG(ERROR) << "Failed to map transport DIB"
+                << " handle:" << handle
+                << " error:" << GetLastError();
+    delete dib;
+    return NULL;
+  }
+
+  // There doesn't seem to be any way to find the size of the shared memory
+  // region! GetFileSize indicates that the handle is invalid. Thus, we
+  // conservatively set the size to the maximum and hope that the renderer
+  // isn't about to ask us to read off the end of the array.
+  dib->size_ = std::numeric_limits<size_t>::max();
+
+  return dib;
+}
+
+void* TransportDIB::memory() const {
+  return shared_memory_.memory();
+}
+
+TransportDIB::Handle TransportDIB::handle() const {
+  return shared_memory_.handle();
+}
+
+TransportDIB::Id TransportDIB::id() const {
+  return Id(shared_memory_.handle(), sequence_num_);
+}