Add heuristic for fixing the alpha channel when reading clipboard images on Windows

ui/base/clipboard/clipboard_win.cc

Index: ui/base/clipboard/clipboard_win.cc
diff --git a/ui/base/clipboard/clipboard_win.cc b/ui/base/clipboard/clipboard_win.cc
index cec06012fbf50e9540fe27d69155a15eba6122e2..d28c3fdc555167afb5ab11a8e88e7fe5fd4dd135 100644
--- a/ui/base/clipboard/clipboard_win.cc
+++ b/ui/base/clipboard/clipboard_win.cc
@@ -430,6 +430,27 @@ void Clipboard::ReadHTML(Clipboard::Buffer buffer, string16* markup,
     markup->assign(UTF8ToWide(markup_utf8));
 }
 
+static bool BitmapHasInvalidPremultipliedColors(const SkBitmap& bitmap) {

[tony, 2011/09/22 01:26:58]

Nit: Chromium style normally puts static functions in the anonymous namespace at
the top of the file.

+  for (int x = 0; x < bitmap.width(); ++x) {
+    for (int y = 0; y < bitmap.height(); ++y) {
+      uint32_t pixel = *bitmap.getAddr32(x, y);
+      if (SkColorGetR(pixel) > SkColorGetA(pixel) ||
+          SkColorGetG(pixel) > SkColorGetA(pixel) ||
+          SkColorGetB(pixel) > SkColorGetA(pixel))
+        return true;
+    }
+  }
+  return false;
+}
+
+static void MakeBitmapOpaque(const SkBitmap& bitmap) {
+  for (int x = 0; x < bitmap.width(); ++x) {
+    for (int y = 0; y < bitmap.height(); ++y) {
+      *bitmap.getAddr32(x, y) = SkColorSetA(*bitmap.getAddr32(x, y), 0xFF);
+    }
+  }
+}
+
 SkBitmap Clipboard::ReadImage(Buffer buffer) const {
   DCHECK_EQ(buffer, BUFFER_STANDARD);
 
@@ -476,18 +497,24 @@ SkBitmap Clipboard::ReadImage(Buffer buffer) const {
                         bitmap->bmiHeader.biHeight, bitmap_bits, bitmap,
                         DIB_RGB_COLORS);
   }
-  // SetDIBitsToDevice doesn't properly set alpha values for bitmaps with
-  // depth < 32bpp so manually fix it up.
-  if (bitmap->bmiHeader.biBitCount < 32) {
-    const SkBitmap& device_bitmap = canvas.getDevice()->accessBitmap(true);
+  // Windows doesn't really handle alpha channels well in many situations. When

[Wez, 2011/09/22 10:55:41]

nit: Technically it's applications which handle DIBV5 image data on the
clipboard poorly.

+  // the source image is < 32 bpp, we force the bitmap to be opaque. When the
+  // source image is 32 bpp, the alpha channel might still contain garbage data.

[Wez, 2011/09/22 10:55:41]

Isn't it rather that some applications put images with alpha channels on the
clipboard but fail to set the alpha mask correctly in the bitmap header, so we
have to check for valid alpha channel if the mask is zero?

+  // Since Windows uses premultiplied alpha, we scan for instances where
+  // (R, G, B) > A. If there are any invalid premultiplied colors in the image,
+  // we assume the alpha channel contains garbage and force the bitmap to be
+  // opaque as well. Note that this  heuristic will fail on a transparent bitmap
+  // containing only black pixels...
+  const SkBitmap& device_bitmap = canvas.getDevice()->accessBitmap(true);
+  {
     SkAutoLockPixels lock(device_bitmap);
-    for (int x = 0; x < device_bitmap.width(); ++x) {
-      for (int y = 0; y < device_bitmap.height(); ++y) {
-        *device_bitmap.getAddr32(x, y) =
-            SkColorSetA(*device_bitmap.getAddr32(x, y), 0xFF);
-      }
+    bool has_invalid_alpha_channel = bitmap->bmiHeader.biBitCount < 32 ||
+        BitmapHasInvalidPremultipliedColors(device_bitmap);
+    if (has_invalid_alpha_channel) {
+      MakeBitmapOpaque(device_bitmap);
     }
   }
+
   return canvas.ExtractBitmap();
 }