Improve routines for verifying clipboard bitmap size/shared memory.

BUG=319477
R=aedla@chromium.org, creis@chromium.org, jln@chromium.org

Review URL: https://codereview.chromium.org/73833004

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

10 files changed, 131 insertions, 106 deletions

diff --git a/content/browser/renderer_host/clipboard_message_filter.cc b/content/browser/renderer_host/clipboard_message_filter.cc
index ebfd9e1..0b673ef 100644
--- a/content/browser/renderer_host/clipboard_message_filter.cc
+++ b/content/browser/renderer_host/clipboard_message_filter.cc
@@ -94,7 +94,9 @@ void ClipboardMessageFilter::OnWriteObjectsSync(
   DCHECK(base::SharedMemory::IsHandleValid(bitmap_handle))
       << "Bad bitmap handle";
   // Splice the shared memory handle into the clipboard data.
-  ui::Clipboard::ReplaceSharedMemHandle(&objects, bitmap_handle, PeerHandle());
+  if (!ui::Clipboard::ReplaceSharedMemHandle(
+           &objects, bitmap_handle, PeerHandle()))
+    return;
 #if defined(OS_WIN)
   // We cannot write directly from the IO thread, and cannot service the IPC
   // on the UI thread. We'll copy the relevant data and get a handle to any
diff --git a/ui/base/clipboard/clipboard.cc b/ui/base/clipboard/clipboard.cc
index d8d0788..6e6ac4e 100644
--- a/ui/base/clipboard/clipboard.cc
+++ b/ui/base/clipboard/clipboard.cc
@@ -5,39 +5,53 @@
 #include "ui/base/clipboard/clipboard.h"
 
 #include <iterator>
+#include <limits>
 
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/size.h"
 
 namespace ui {
 
 namespace {
 
-// A compromised renderer could send us bad data, so validate it.
-// This function only checks that the size parameter makes sense, the caller
-// is responsible for further validating the bitmap buffer against
-// |bitmap_bytes|.
+// Extracts the bitmap size from the passed in params. Since the renderer could
+// send us bad data, explicitly copy the parameters to make sure we go through
+// gfx::Size's sanity checks.
+bool GetBitmapSizeFromParams(const Clipboard::ObjectMapParams& params,
+                             gfx::Size* size) {
+  DCHECK(params.size() == 2);
+  if (params[1].size() != sizeof(gfx::Size))
+    return false;
+  const gfx::Size* size_from_renderer =
+      reinterpret_cast<const gfx::Size*>(&(params[1].front()));
+  size->set_width(size_from_renderer->width());
+  size->set_height(size_from_renderer->height());
+  return true;
+}
+
+// A compromised renderer could send us bad size data, so validate it to verify
+// that calculating the number of bytes in the bitmap won't overflow a uint32.
 //
-// |params| - Clipboard bitmap contents to validate.
+// |size| - Clipboard bitmap size to validate.
 // |bitmap_bytes| - On return contains the number of bytes needed to store
 // the bitmap data or -1 if the data is invalid.
 // returns: true if the bitmap size is valid, false otherwise.
-bool IsBitmapSafe(const Clipboard::ObjectMapParams& params,
-                  uint32* bitmap_bytes) {
+bool IsBitmapSizeSane(const gfx::Size& size, uint32* bitmap_bytes) {
+  DCHECK_GE(size.width(), 0);
+  DCHECK_GE(size.height(), 0);
+
   *bitmap_bytes = -1;
-  if (params[1].size() != sizeof(gfx::Size))
-    return false;
-  const gfx::Size* size =
-      reinterpret_cast<const gfx::Size*>(&(params[1].front()));
-  uint32 total_size = size->width();
-  // Using INT_MAX not SIZE_T_MAX to put a reasonable bound on things.
-  if (INT_MAX / size->width() <= size->height())
+  uint32 total_size = size.width();
+  // Use max uint32 instead of max size_t to put a reasonable bound on things.
+  if (std::numeric_limits<uint32>::max() / size.width() <=
+      static_cast<uint32>(size.height()))
     return false;
-  total_size *= size->height();
-  if (INT_MAX / total_size <= 4)
+  total_size *= size.height();
+  if (std::numeric_limits<uint32>::max() / total_size <= 4)
     return false;
   total_size *= 4;
   *bitmap_bytes = total_size;
@@ -47,9 +61,10 @@ bool IsBitmapSafe(const Clipboard::ObjectMapParams& params,
 // Validates a plain bitmap on the clipboard.
 // Returns true if the clipboard data makes sense and it's safe to access the
 // bitmap.
-bool ValidatePlainBitmap(const Clipboard::ObjectMapParams& params) {
+bool ValidatePlainBitmap(const gfx::Size& size,
+                         const Clipboard::ObjectMapParams& params) {
   uint32 bitmap_bytes = -1;
-  if (!IsBitmapSafe(params, &bitmap_bytes))
+  if (!IsBitmapSizeSane(size, &bitmap_bytes))
     return false;
   if (bitmap_bytes != params[0].size())
     return false;
@@ -59,11 +74,11 @@ bool ValidatePlainBitmap(const Clipboard::ObjectMapParams& params) {
 // Valides a shared bitmap on the clipboard.
 // Returns true if the clipboard data makes sense and it's safe to access the
 // bitmap.
-bool ValidateAndMapSharedBitmap(const Clipboard::ObjectMapParams& params,
+bool ValidateAndMapSharedBitmap(const gfx::Size& size,
                                 base::SharedMemory* bitmap_data) {
   using base::SharedMemory;
   uint32 bitmap_bytes = -1;
-  if (!IsBitmapSafe(params, &bitmap_bytes))
+  if (!IsBitmapSizeSane(size, &bitmap_bytes))
     return false;
 
   if (!bitmap_data || !SharedMemory::IsHandleValid(bitmap_data->handle()))
@@ -76,6 +91,19 @@ bool ValidateAndMapSharedBitmap(const Clipboard::ObjectMapParams& params,
   return true;
 }
 
+// Adopts a blob of bytes (assumed to be ARGB pixels) into a SkBitmap. Since the
+// pixel data is not copied, the caller must ensure that |data| is valid as long
+// as the SkBitmap is in use. Note that on little endian machines, each pixel is
+// actually BGRA in memory.
+SkBitmap AdoptBytesIntoSkBitmap(const void* data, const gfx::Size& size) {
+  SkBitmap bitmap;
+  bitmap.setConfig(SkBitmap::kARGB_8888_Config, size.width(), size.height());
+  // Guaranteed not to overflow since it's been validated by IsBitmapSizeSane().
+  DCHECK_EQ(4U * size.width(), bitmap.rowBytes());
+  bitmap.setPixels(const_cast<void*>(data));
+  return bitmap;
+}
+
 // A list of allowed threads. By default, this is empty and no thread checking
 // is done (in the unit test case), but a user (like content) can set which
 // threads are allowed to call this method.
@@ -184,12 +212,19 @@ void Clipboard::DispatchObject(ObjectType type, const ObjectMapParams& params) {
       WriteWebSmartPaste();
       break;
 
-    case CBF_BITMAP:
-      if (!ValidatePlainBitmap(params))
+    case CBF_BITMAP: {
+      gfx::Size size;
+      if (!GetBitmapSizeFromParams(params, &size))
         return;
 
-      WriteBitmap(&(params[0].front()), &(params[1].front()));
+      if (!ValidatePlainBitmap(size, params))
+        return;
+
+      const SkBitmap& bitmap = AdoptBytesIntoSkBitmap(
+          static_cast<const void*>(&params[0].front()), size);
+      WriteBitmap(bitmap);
       break;
+    }
 
     case CBF_SMBITMAP: {
       using base::SharedMemory;
@@ -206,10 +241,16 @@ void Clipboard::DispatchObject(ObjectType type, const ObjectMapParams& params) {
       scoped_ptr<SharedMemory> bitmap_data(
           *reinterpret_cast<SharedMemory**>(raw_bitmap_data));
 
-      if (!ValidateAndMapSharedBitmap(params, bitmap_data.get()))
+      gfx::Size size;
+      if (!GetBitmapSizeFromParams(params, &size))
         return;
-      WriteBitmap(static_cast<const char*>(bitmap_data->memory()),
-                  &(params[1].front()));
+
+      if (!ValidateAndMapSharedBitmap(size, bitmap_data.get()))
+        return;
+
+      const SkBitmap& bitmap = AdoptBytesIntoSkBitmap(
+          bitmap_data->memory(), size);
+      WriteBitmap(bitmap);
       break;
     }
 
@@ -227,7 +268,7 @@ void Clipboard::DispatchObject(ObjectType type, const ObjectMapParams& params) {
 }
 
 // static
-void Clipboard::ReplaceSharedMemHandle(ObjectMap* objects,
+bool Clipboard::ReplaceSharedMemHandle(ObjectMap* objects,
                                        base::SharedMemoryHandle bitmap_handle,
                                        base::ProcessHandle process) {
   using base::SharedMemory;
@@ -237,9 +278,9 @@ void Clipboard::ReplaceSharedMemHandle(ObjectMap* objects,
        ++iter) {
     if (iter->first == CBF_SMBITMAP) {
       // The code currently only accepts sending a single bitmap over this way.
-      // Fail hard if we ever encounter more than one shared bitmap structure to
-      // fill.
-      CHECK(!has_shared_bitmap);
+      // Fail if we ever encounter more than one shmem bitmap structure to fill.
+      if (has_shared_bitmap)
+        return false;
 
 #if defined(OS_WIN)
       SharedMemory* bitmap = new SharedMemory(bitmap_handle, true, process);
@@ -247,6 +288,10 @@ void Clipboard::ReplaceSharedMemHandle(ObjectMap* objects,
       SharedMemory* bitmap = new SharedMemory(bitmap_handle, true);
 #endif
 
+      // There must always be two parameters associated with each shmem bitmap.
+      if (iter->second.size() != 2)
+        return false;
+
       // We store the shared memory object pointer so it can be retrieved by the
       // UI thread (see DispatchObject()).
       iter->second[0].clear();
@@ -255,6 +300,7 @@ void Clipboard::ReplaceSharedMemHandle(ObjectMap* objects,
       has_shared_bitmap = true;
     }
   }
+  return true;
 }
 
 }  // namespace ui
diff --git a/ui/base/clipboard/clipboard.h b/ui/base/clipboard/clipboard.h
index 2a8ed7b..b8bc555 100644
--- a/ui/base/clipboard/clipboard.h
+++ b/ui/base/clipboard/clipboard.h
@@ -305,9 +305,10 @@ class UI_EXPORT Clipboard : NON_EXPORTED_BASE(public base::ThreadChecker) {
   // |objects|.  The pointer is deleted by DispatchObjects().
   //
   // On non-Windows platforms, |process| is ignored.
-  static void ReplaceSharedMemHandle(ObjectMap* objects,
+  static bool ReplaceSharedMemHandle(ObjectMap* objects,
                                      base::SharedMemoryHandle bitmap_handle,
-                                     base::ProcessHandle process);
+                                     base::ProcessHandle process)
+      WARN_UNUSED_RESULT;
 #if defined(OS_WIN)
   // Firefox text/html
   static const FormatType& GetTextHtmlFormatType();
@@ -343,7 +344,7 @@ class UI_EXPORT Clipboard : NON_EXPORTED_BASE(public base::ThreadChecker) {
 
   void WriteWebSmartPaste();
 
-  void WriteBitmap(const char* pixel_data, const char* size_data);
+  void WriteBitmap(const SkBitmap& bitmap);
 
   void WriteData(const FormatType& format,
                  const char* data_data,
diff --git a/ui/base/clipboard/clipboard_android.cc b/ui/base/clipboard/clipboard_android.cc
index 34d4a83..fc9ba00 100644
--- a/ui/base/clipboard/clipboard_android.cc
+++ b/ui/base/clipboard/clipboard_android.cc
@@ -295,14 +295,12 @@ SkBitmap Clipboard::ReadImage(ClipboardType type) const {
     DCHECK_LE(sizeof(gfx::Size), input.size());
     const gfx::Size* size = reinterpret_cast<const gfx::Size*>(input.data());
 
-    bmp.setConfig(
-        SkBitmap::kARGB_8888_Config, size->width(), size->height(), 0);
+    bmp.setConfig(SkBitmap::kARGB_8888_Config, size->width(), size->height());
     bmp.allocPixels();
 
-    int bm_size = size->width() * size->height() * 4;
-    DCHECK_EQ(sizeof(gfx::Size) + bm_size, input.size());
+    DCHECK_EQ(sizeof(gfx::Size) + bmp.getSize(), input.size());
 
-    memcpy(bmp.getPixels(), input.data() + sizeof(gfx::Size), bm_size);
+    memcpy(bmp.getPixels(), input.data() + sizeof(gfx::Size), bmp.getSize());
   }
   return bmp;
 }
@@ -407,16 +405,17 @@ void Clipboard::WriteWebSmartPaste() {
   g_map.Get().Set(kWebKitSmartPasteFormat, std::string());
 }
 
-// All platforms use gfx::Size for size data but it is passed as a const char*
-// Further, pixel_data is expected to be 32 bits per pixel
 // Note: we implement this to pass all unit tests but it is currently unclear
 // how some code would consume this.
-void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
-  const gfx::Size* size = reinterpret_cast<const gfx::Size*>(size_data);
-  int bm_size = size->width() * size->height() * 4;
-
-  std::string packed(size_data, sizeof(gfx::Size));
-  packed += std::string(pixel_data, bm_size);
+void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
+  gfx::Size size(bitmap.width(), bitmap.height());
+
+  std::string packed(reinterpret_cast<const char*>(&size), sizeof(size));
+  {
+    SkAutoLockPixels bitmap_lock(bitmap);
+    packed += std::string(static_cast<const char*>(bitmap.getPixels()),
+                          bitmap.getSize());
+  }
   g_map.Get().Set(kBitmapFormat, packed);
 }
 
diff --git a/ui/base/clipboard/clipboard_aura.cc b/ui/base/clipboard/clipboard_aura.cc
index 7d73629..2074466 100644
--- a/ui/base/clipboard/clipboard_aura.cc
+++ b/ui/base/clipboard/clipboard_aura.cc
@@ -41,8 +41,7 @@ enum AuraClipboardFormat {
 class ClipboardData {
  public:
   ClipboardData()
-      : bitmap_data_(),
-        web_smart_paste_(false),
+      : web_smart_paste_(false),
         format_(0) {}
 
   virtual ~ClipboardData() {}
@@ -86,15 +85,9 @@ class ClipboardData {
     format_ |= BOOKMARK;
   }
 
-  uint8_t* bitmap_data() const { return bitmap_data_.get(); }
-  const gfx::Size& bitmap_size() const { return bitmap_size_; }
-  void SetBitmapData(const char* pixel_data, const char* size_data) {
-    bitmap_size_ = *reinterpret_cast<const gfx::Size*>(size_data);
-
-    // We assume 4-byte pixel data.
-    size_t bitmap_data_len = 4 * bitmap_size_.width() * bitmap_size_.height();
-    bitmap_data_.reset(new uint8_t[bitmap_data_len]);
-    memcpy(bitmap_data_.get(), pixel_data, bitmap_data_len);
+  const SkBitmap& bitmap() const { return bitmap_; }
+  void SetBitmapData(const SkBitmap& bitmap) {
+    bitmap.copyTo(&bitmap_, bitmap.getConfig());
     format_ |= BITMAP;
   }
 
@@ -137,8 +130,7 @@ class ClipboardData {
   std::vector<std::string> files_;
 
   // Bitmap images.
-  scoped_ptr<uint8_t[]> bitmap_data_;
-  gfx::Size bitmap_size_;
+  SkBitmap bitmap_;
 
   // Data with custom format.
   std::string custom_data_format_;
@@ -253,13 +245,9 @@ class AuraClipboard {
     if (!HasFormat(BITMAP))
       return img;
 
-    const ClipboardData* data = GetData();
-    const gfx::Size size = data->bitmap_size();
-    uint8_t* bitmap = data->bitmap_data();
-    img.setConfig(SkBitmap::kARGB_8888_Config, size.width(), size.height(), 0);
-    img.allocPixels();
-    img.eraseARGB(0, 0, 0, 0);
-    memcpy(img.getPixels(), bitmap, size.width() * size.height() * 4);
+    // A shallow copy should be fine here, but just to be safe...
+    const SkBitmap& clipboard_bitmap = GetData()->bitmap();
+    clipboard_bitmap.copyTo(&img, clipboard_bitmap.getConfig());
     return img;
   }
 
@@ -387,9 +375,9 @@ class ClipboardDataBuilder {
     data->set_web_smart_paste(true);
   }
 
-  static void WriteBitmap(const char* pixel_data, const char* size_data) {
+  static void WriteBitmap(const SkBitmap& bitmap) {
     ClipboardData* data = GetCurrentData();
-    data->SetBitmapData(pixel_data, size_data);
+    data->SetBitmapData(bitmap);
   }
 
   static void WriteData(const std::string& format,
@@ -597,8 +585,8 @@ void Clipboard::WriteWebSmartPaste() {
   ClipboardDataBuilder::WriteWebSmartPaste();
 }
 
-void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
-  ClipboardDataBuilder::WriteBitmap(pixel_data, size_data);
+void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
+  ClipboardDataBuilder::WriteBitmap(bitmap);
 }
 
 void Clipboard::WriteData(const FormatType& format,
diff --git a/ui/base/clipboard/clipboard_aurax11.cc b/ui/base/clipboard/clipboard_aurax11.cc
index 81c3374..71a545c 100644
--- a/ui/base/clipboard/clipboard_aurax11.cc
+++ b/ui/base/clipboard/clipboard_aurax11.cc
@@ -755,15 +755,7 @@ void Clipboard::WriteWebSmartPaste() {
                                   scoped_refptr<base::RefCountedMemory>());
 }
 
-void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
-  const gfx::Size* size = reinterpret_cast<const gfx::Size*>(size_data);
-
-  // Adopt the pixels into a SkBitmap. Note that the pixel order in memory is
-  // actually BGRA.
-  SkBitmap bitmap;
-  bitmap.setConfig(SkBitmap::kARGB_8888_Config, size->width(), size->height());
-  bitmap.setPixels(const_cast<char*>(pixel_data));
-
+void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
   // Encode the bitmap as a PNG for transport.
   std::vector<unsigned char> output;
   if (gfx::PNGCodec::FastEncodeBGRASkBitmap(bitmap, false, &output)) {
diff --git a/ui/base/clipboard/clipboard_gtk.cc b/ui/base/clipboard/clipboard_gtk.cc
index 58e809d..59e2bc4 100644
--- a/ui/base/clipboard/clipboard_gtk.cc
+++ b/ui/base/clipboard/clipboard_gtk.cc
@@ -305,14 +305,7 @@ void Clipboard::WriteWebSmartPaste() {
   InsertMapping(kMimeTypeWebkitSmartPaste, NULL, 0);
 }
 
-void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
-  const gfx::Size* size = reinterpret_cast<const gfx::Size*>(size_data);
-
-  // Adopt the pixels into a SkBitmap. Note that the pixel order in memory is
-  // actually BGRA.
-  SkBitmap bitmap;
-  bitmap.setConfig(SkBitmap::kARGB_8888_Config, size->width(), size->height());
-  bitmap.setPixels(const_cast<char*>(pixel_data));
+void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
   GdkPixbuf* pixbuf = gfx::GdkPixbufFromSkBitmap(bitmap);
 
   // We store the GdkPixbuf*, and the size_t half of the pair is meaningless.
diff --git a/ui/base/clipboard/clipboard_mac.mm b/ui/base/clipboard/clipboard_mac.mm
index 7163029..9f12648 100644
--- a/ui/base/clipboard/clipboard_mac.mm
+++ b/ui/base/clipboard/clipboard_mac.mm
@@ -156,25 +156,24 @@ void Clipboard::WriteBookmark(const char* title_data,
   [pb setString:title forType:kUTTypeURLName];
 }
 
-void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
-  const gfx::Size* size = reinterpret_cast<const gfx::Size*>(size_data);
-
+void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
+  // TODO(dcheng): Just use gfx::SkBitmapToNSImageWithColorspace().
   // Safe because the image goes away before the call returns.
   base::ScopedCFTypeRef<CFDataRef> data(
       CFDataCreateWithBytesNoCopy(kCFAllocatorDefault,
-                                  reinterpret_cast<const UInt8*>(pixel_data),
-                                  size->width() * size->height() * 4,
+                                  static_cast<const UInt8*>(bitmap.getPixels()),
+                                  bitmap.getSize(),
                                   kCFAllocatorNull));
 
   base::ScopedCFTypeRef<CGDataProviderRef> data_provider(
       CGDataProviderCreateWithCFData(data));
 
   base::ScopedCFTypeRef<CGImageRef> cgimage(
-      CGImageCreate(size->width(),
-                    size->height(),
+      CGImageCreate(bitmap.width(),
+                    bitmap.height(),
                     8,
                     32,
-                    size->width() * 4,
+                    bitmap.rowBytes(),
                     base::mac::GetSRGBColorSpace(),  // TODO(avi): do better
                     kCGImageAlphaPremultipliedFirst | kCGBitmapByteOrder32Host,
                     data_provider,
@@ -186,12 +185,12 @@ void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
   data_provider.reset();
   data.reset();
 
-  base::scoped_nsobject<NSBitmapImageRep> bitmap(
+  base::scoped_nsobject<NSBitmapImageRep> bitmap_rep(
       [[NSBitmapImageRep alloc] initWithCGImage:cgimage]);
   cgimage.reset();
 
   base::scoped_nsobject<NSImage> image([[NSImage alloc] init]);
-  [image addRepresentation:bitmap];
+  [image addRepresentation:bitmap_rep];
 
   // An API to ask the NSImage to write itself to the clipboard comes in 10.6 :(
   // For now, spit out the image as a TIFF.
@@ -354,6 +353,7 @@ SkBitmap Clipboard::ReadImage(ClipboardType type) const {
   if (!image.get())
     return SkBitmap();
 
+  // TODO(dcheng): Just use gfx::NSImageToSkBitmap().
   gfx::ScopedNSGraphicsContextSaveGState scoped_state;
   [image setFlipped:YES];
   int width = [image size].width;
diff --git a/ui/base/clipboard/clipboard_unittest.cc b/ui/base/clipboard/clipboard_unittest.cc
index 7f2447a..0ce0c3d 100644
--- a/ui/base/clipboard/clipboard_unittest.cc
+++ b/ui/base/clipboard/clipboard_unittest.cc
@@ -356,7 +356,8 @@ TEST_F(ClipboardTest, SharedBitmapTest) {
 
   Clipboard::ObjectMap objects;
   objects[Clipboard::CBF_SMBITMAP] = params;
-  Clipboard::ReplaceSharedMemHandle(&objects, handle_to_share, current_process);
+  ASSERT_TRUE(Clipboard::ReplaceSharedMemHandle(
+      &objects, handle_to_share, current_process));
 
   clipboard().WriteObjects(CLIPBOARD_TYPE_COPY_PASTE,
                            objects);
diff --git a/ui/base/clipboard/clipboard_win.cc b/ui/base/clipboard/clipboard_win.cc
index 1c872cb..92632c7 100644
--- a/ui/base/clipboard/clipboard_win.cc
+++ b/ui/base/clipboard/clipboard_win.cc
@@ -273,8 +273,7 @@ void Clipboard::WriteWebSmartPaste() {
   ::SetClipboardData(GetWebKitSmartPasteFormatType().ToUINT(), NULL);
 }
 
-void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
-  const gfx::Size* size = reinterpret_cast<const gfx::Size*>(size_data);
+void Clipboard::WriteBitmap(const SkBitmap& bitmap) {
   HDC dc = ::GetDC(NULL);
 
   // This doesn't actually cost us a memcpy when the bitmap comes from the
@@ -284,8 +283,8 @@ void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
   // TODO(darin): share data in gfx/bitmap_header.cc somehow
   BITMAPINFO bm_info = {0};
   bm_info.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
-  bm_info.bmiHeader.biWidth = size->width();
-  bm_info.bmiHeader.biHeight = -size->height();  // sets vertical orientation
+  bm_info.bmiHeader.biWidth = bitmap.width();
+  bm_info.bmiHeader.biHeight = -bitmap.height();  // sets vertical orientation
   bm_info.bmiHeader.biPlanes = 1;
   bm_info.bmiHeader.biBitCount = 32;
   bm_info.bmiHeader.biCompression = BI_RGB;
@@ -298,11 +297,15 @@ void Clipboard::WriteBitmap(const char* pixel_data, const char* size_data) {
       ::CreateDIBSection(dc, &bm_info, DIB_RGB_COLORS, &bits, NULL, 0);
 
   if (bits && source_hbitmap) {
-    // Copy the bitmap out of shared memory and into GDI
-    memcpy(bits, pixel_data, 4 * size->width() * size->height());
+    {
+      SkAutoLockPixels bitmap_lock(bitmap);
+      // Copy the bitmap out of shared memory and into GDI
+      memcpy(bits, bitmap.getPixels(), bitmap.getSize());
+    }
 
     // Now we have an HBITMAP, we can write it to the clipboard
-    WriteBitmapFromHandle(source_hbitmap, *size);
+    WriteBitmapFromHandle(source_hbitmap,
+                          gfx::Size(bitmap.width(), bitmap.height()));
   }
 
   ::DeleteObject(source_hbitmap);