Retry. I believe the code (with your width() * bbp fix) is correct.

The UMR errors occur when the source image contain alpha. I believe the issue comes from webkit glue image decoder, and the reason this only started triggering valgrind errors is that the old ones were masked by the change detailed in bug 12640

Also adds valgrind suppression for the new code.

BUG=12891,12640
TEST=none

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

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

2 files changed, 47 insertions, 5 deletions

diff --git a/base/gfx/png_encoder.cc b/base/gfx/png_encoder.cc
index 5021721..9ff4d61 100644
--- a/base/gfx/png_encoder.cc
+++ b/base/gfx/png_encoder.cc
@@ -5,6 +5,7 @@
 #include "base/basictypes.h"
 #include "base/gfx/png_encoder.h"
 #include "base/logging.h"
+#include "base/scoped_ptr.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
 extern "C" {
@@ -197,9 +198,40 @@ bool PNGEncoder::Encode(const unsigned char* input, ColorFormat format,
 bool PNGEncoder::EncodeBGRASkBitmap(const SkBitmap& input,
                                     bool discard_transparency,
                                     std::vector<unsigned char>* output) {
-  SkAutoLockPixels input_lock(input);
-  DCHECK(input.empty() || input.bytesPerPixel() == 4);
-  return Encode(static_cast<unsigned char*>(input.getPixels()),
-                PNGEncoder::FORMAT_BGRA, input.width(), input.height(),
-                input.rowBytes(), discard_transparency, output);
+  static const int bbp = 4;
+
+  SkAutoLockPixels lock_input(input);
+  CHECK(!input.empty());
+  CHECK(input.bytesPerPixel() == bbp);
+  CHECK(input.getConfig() == SkBitmap::kARGB_8888_Config);
+  CHECK(input.width() > 1 && input.height() > 1);
+
+  // SkBitmaps are premultiplied, we need to unpremultiply them.
+  scoped_array<unsigned char> divided(
+      new unsigned char[input.width() * input.height() * bbp]);
+
+  int i = 0;
+  for (int y = 0; y < input.height(); y++) {
+    for (int x = 0; x < input.width(); x++) {
+      uint32 pixel = input.getAddr32(0, y)[x];
+
+      int alpha = SkColorGetA(pixel);
+      if (alpha != 0 && alpha != 255) {
+        divided[i + 0] = (SkColorGetR(pixel) << 8) / alpha;
+        divided[i + 1] = (SkColorGetG(pixel) << 8) / alpha;
+        divided[i + 2] = (SkColorGetB(pixel) << 8) / alpha;
+        divided[i + 3] = alpha;
+      } else {
+        divided[i + 0] = SkColorGetR(pixel);
+        divided[i + 1] = SkColorGetG(pixel);
+        divided[i + 2] = SkColorGetB(pixel);
+        divided[i + 3] = alpha;
+      }
+      i += bbp;
+    }
+  }
+
+  return Encode(divided.get(),
+                PNGEncoder::FORMAT_RGBA, input.width(), input.height(),
+                input.width() * bbp, discard_transparency, output);
 }
diff --git a/tools/valgrind/suppressions_mac.txt b/tools/valgrind/suppressions_mac.txt
index 1db729f..66bc8c2 100644
--- a/tools/valgrind/suppressions_mac.txt
+++ b/tools/valgrind/suppressions_mac.txt
@@ -220,3 +220,13 @@
    fun:write$UNIX2003
    fun:_ZN24ExtensionsServiceBackend19OnExtensionUnpackedERK8FilePathS2_SsbRK15DictionaryValueRKSt6vectorI6Tuple2I8SkBitmapS0_ESaIS9_EE
 }
+{
+   bug_12640d
+   Memcheck:Cond
+   fun:_ZN10PNGEncoder18EncodeBGRASkBitmapERK8SkBitmapbPSt6vectorIhSaIhEE
+   fun:_ZN24ExtensionsServiceBackend19OnExtensionUnpackedERK8FilePathS2_SsbRK15DictionaryValueRKSt6vectorI6Tuple2I8SkBitmapS0_ESaIS9_EE
+   fun:_ZN24ExtensionsServiceBackend14UnpackerClient26OnUnpackExtensionSucceededERK15DictionaryValueRKSt6vectorI6Tuple2I8SkBitmap8FilePathESaIS8_EE
+   ...
+   fun:_ZN11MessageLoop13RunAllPendingEv
+}
+