Add support for PNG representation in gfx::Image

ui/gfx/image/image.cc

Index: ui/gfx/image/image.cc
diff --git a/ui/gfx/image/image.cc b/ui/gfx/image/image.cc
index c5d30a29210fbf4db294a8f4df3189e27a2c739d..cde1e8f9a8342632622e0d71a5024f3cf6e12141 100644
--- a/ui/gfx/image/image.cc
+++ b/ui/gfx/image/image.cc
@@ -9,6 +9,7 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/size.h"
 
@@ -41,6 +42,14 @@ const ImageSkia ImageSkiaFromGdkPixbuf(GdkPixbuf* pixbuf) {
 }
 #endif
 
+const ImageSkia ImageSkiaFromPNG(const std::vector<unsigned char>& png) {
+  SkBitmap bitmap;
+  if (!gfx::PNGCodec::Decode(&png.front(), png.size(), &bitmap))
+    LOG(WARNING) << "Unable to decode PNG, returning empty bitmap";
+  return ImageSkia(bitmap);
+}
+
+class ImageRepPNG;
 class ImageRepSkia;
 class ImageRepGdk;
 class ImageRepCairo;
@@ -59,6 +68,11 @@ class ImageRep {
   virtual ~ImageRep() {}
 
   // Cast helpers ("fake RTTI").
+  ImageRepPNG* AsImageRepPNG() {
+    CHECK_EQ(type_, Image::kImageRepPNG);
+    return reinterpret_cast<ImageRepPNG*>(this);
+  }
+
   ImageRepSkia* AsImageRepSkia() {
     CHECK_EQ(type_, Image::kImageRepSkia);
     return reinterpret_cast<ImageRepSkia*>(this);
@@ -89,6 +103,32 @@ class ImageRep {
   Image::RepresentationType type_;
 };
 
+class ImageRepPNG : public ImageRep {
+ public:
+  explicit ImageRepPNG(const unsigned char* input, size_t input_size)

[Robert Sesek, 2012/07/20 21:03:01]

Don't need to mark this constructor as explicit.

[cjhopman, 2012/07/24 01:44:13]

Done.

+      : ImageRep(Image::kImageRepPNG),
+        image_(input, input + input_size) {
+  }
+
+  explicit ImageRepPNG(const SkBitmap* bitmap)
+      : ImageRep(Image::kImageRepPNG) {
+    if (!gfx::PNGCodec::EncodeBGRASkBitmap(*bitmap, false, &image_)) {
+      LOG(WARNING) << "Unable to encode bitmap returning empty PNG.";
+      image_.clear();
+    }
+  }
+
+  virtual ~ImageRepPNG() {
+  }
+
+  const std::vector<unsigned char>& image() const { return image_; }
+
+ private:
+  std::vector<unsigned char> image_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageRepPNG);
+};
+
 class ImageRepSkia : public ImageRep {
  public:
   // Takes ownership of |image|.
@@ -219,6 +259,19 @@ Image::Image() {
   // |storage_| is NULL for empty Images.
 }
 
+Image::Image(const unsigned char* png, size_t input_size)
+    : storage_(new internal::ImageStorage(Image::kImageRepPNG)) {
+  internal::ImageRepPNG* rep = new internal::ImageRepPNG(png, input_size);
+  AddRepresentation(rep);
+}
+
+Image::Image(const std::vector<unsigned char>& png)
+    : storage_(new internal::ImageStorage(Image::kImageRepPNG)) {
+  internal::ImageRepPNG* rep =
+      new internal::ImageRepPNG(&png.front(), png.size());
+  AddRepresentation(rep);
+}
+
 Image::Image(const ImageSkia& image)
     : storage_(new internal::ImageStorage(Image::kImageRepSkia)) {
   internal::ImageRepSkia* rep = new internal::ImageRepSkia(
@@ -267,6 +320,34 @@ Image& Image::operator=(const Image& other) {
 Image::~Image() {
 }
 
+const std::vector<unsigned char>* Image::ToImagePNG() const {
+  internal::ImageRep* rep = GetRepresentation(kImageRepPNG, false);
+  if (!rep) {
+    switch (DefaultRepresentationType()) {
+      // For both native types, ensure that there is an ImageRepSkia and fall
+      // through to that case

[Robert Sesek, 2012/07/20 21:03:01]

nit: full-stop

[cjhopman, 2012/07/24 01:44:13]

Done.

+#if defined(TOOLKIT_GTK)
+      case kImageRepGdk:
+        ToImageSkia();
+#elif defined(OS_MACOSX)
+      case kImageRepCocoa:
+        ToImageSkia();
+#endif
+      case kImageRepSkia: {
+        internal::ImageRepSkia* skia_rep =
+          GetRepresentation(kImageRepSkia, true)->AsImageRepSkia();

[Robert Sesek, 2012/07/20 21:03:01]

nit: continuations should be intended 4

[cjhopman, 2012/07/24 01:44:13]

Done.

+        rep = new internal::ImageRepPNG(skia_rep->image()->bitmap());
+        break;
+      }
+      default:
+        NOTREACHED();
+    }
+    CHECK(rep);
+    AddRepresentation(rep);
+  }
+  return &rep->AsImageRepPNG()->image();
+}
+
 const SkBitmap* Image::ToSkBitmap() const {
   // Possibly create and cache an intermediate ImageRepSkia.
   return ToImageSkia()->bitmap();
@@ -275,17 +356,34 @@ const SkBitmap* Image::ToSkBitmap() const {
 const ImageSkia* Image::ToImageSkia() const {
   internal::ImageRep* rep = GetRepresentation(kImageRepSkia, false);
   if (!rep) {
+    switch (DefaultRepresentationType()) {
+      case kImageRepPNG: {
+        internal::ImageRepPNG* png_rep =
+          GetRepresentation(kImageRepPNG, true)->AsImageRepPNG();

[Robert Sesek, 2012/07/20 21:03:01]

nit: indent correctly

[cjhopman, 2012/07/24 01:44:13]

Done.

+        rep = new internal::ImageRepSkia(new ImageSkia(
+            internal::ImageSkiaFromPNG(png_rep->image())));

[Robert Sesek, 2012/07/20 21:03:01]

You only call this once. Why not have this function return a pointer so that you
don't need to invoke the copy constructor?

[cjhopman, 2012/07/24 01:44:13]

Done.

+        break;
+      }
 #if defined(TOOLKIT_GTK)
-    internal::ImageRepGdk* native_rep =
-        GetRepresentation(kImageRepGdk, true)->AsImageRepGdk();
-    rep = new internal::ImageRepSkia(new ImageSkia(
-        internal::ImageSkiaFromGdkPixbuf(native_rep->pixbuf())));
+      case kImageRepGdk: {
+        internal::ImageRepGdk* native_rep =
+            GetRepresentation(kImageRepGdk, true)->AsImageRepGdk();
+        rep = new internal::ImageRepSkia(new ImageSkia(
+            internal::ImageSkiaFromGdkPixbuf(native_rep->pixbuf())));
+        break;
+      }
 #elif defined(OS_MACOSX)
-    internal::ImageRepCocoa* native_rep =
-        GetRepresentation(kImageRepCocoa, true)->AsImageRepCocoa();
-    rep = new internal::ImageRepSkia(new ImageSkia(
-        ImageSkiaFromNSImage(native_rep->image())));
+      case kImageRepCocoa: {
+        internal::ImageRepCocoa* native_rep =
+            GetRepresentation(kImageRepCocoa, true)->AsImageRepCocoa();
+        rep = new internal::ImageRepSkia(new ImageSkia(
+            ImageSkiaFromNSImage(native_rep->image())));
+        break;
+      }
 #endif
+      default:
+        NOTREACHED();
+    }
     CHECK(rep);
     AddRepresentation(rep);
   }
@@ -296,10 +394,20 @@ const ImageSkia* Image::ToImageSkia() const {
 GdkPixbuf* Image::ToGdkPixbuf() const {
   internal::ImageRep* rep = GetRepresentation(kImageRepGdk, false);
   if (!rep) {
-    internal::ImageRepSkia* skia_rep =
-        GetRepresentation(kImageRepSkia, true)->AsImageRepSkia();
-    rep = new internal::ImageRepGdk(gfx::GdkPixbufFromSkBitmap(
-        *skia_rep->image()->bitmap()));
+    switch (DefaultRepresentationType()) {
+      case kImageRepPNG:
+        // Ensure that there is an ImageRepSkia and fall through to that case.
+        ToImageSkia();

[Robert Sesek, 2012/07/20 21:03:01]

The comment at ToNSImage() applies here, too.

[cjhopman, 2012/07/24 01:44:13]

Done.

+      case kImageRepSkia: {
+        internal::ImageRepSkia* skia_rep =
+            GetRepresentation(kImageRepSkia, true)->AsImageRepSkia();
+        rep = new internal::ImageRepGdk(gfx::GdkPixbufFromSkBitmap(
+            *skia_rep->image()->bitmap()));
+        break;
+      }
+      default:
+        NOTREACHED();
+    }
     CHECK(rep);
     AddRepresentation(rep);
   }
@@ -323,11 +431,21 @@ CairoCachedSurface* const Image::ToCairo() const {
 NSImage* Image::ToNSImage() const {
   internal::ImageRep* rep = GetRepresentation(kImageRepCocoa, false);
   if (!rep) {
-    internal::ImageRepSkia* skia_rep =
-        GetRepresentation(kImageRepSkia, true)->AsImageRepSkia();
-    NSImage* image = NSImageFromImageSkia(*skia_rep->image());
-    base::mac::NSObjectRetain(image);
-    rep = new internal::ImageRepCocoa(image);
+    switch (DefaultRepresentationType()) {
+      case kImageRepPNG:
+        // Ensure that there is a ImageRepSkia and fall through to that case.
+        ToImageSkia();

[Robert Sesek, 2012/07/20 21:03:01]

This is a double-conversion. Instead you could use the platform PNG decoder to
return an NSImage with a single conversion.

The ResourceBundle already has the logic for loading raw PNG data into the
native platform type. I'm not sure how much sense it makes to unify that logic,
but I think these double conversions are unfortunate.

[cjhopman, 2012/07/24 01:44:13]

Done. Also removed the intermediate conversions in the ToImagePNG() function.

+      case kImageRepSkia: {
+        internal::ImageRepSkia* skia_rep =
+            GetRepresentation(kImageRepSkia, true)->AsImageRepSkia();
+        NSImage* image = NSImageFromImageSkia(*skia_rep->image());
+        base::mac::NSObjectRetain(image);
+        rep = new internal::ImageRepCocoa(image);
+        break;
+      }
+      default:
+        NOTREACHED();
+    }
     CHECK(rep);
     AddRepresentation(rep);
   }
@@ -335,6 +453,10 @@ NSImage* Image::ToNSImage() const {
 }
 #endif
 
+std::vector<unsigned char>* Image::CopyImagePNG() const {
+  return new std::vector<unsigned char>(*ToImagePNG());
+}
+
 ImageSkia* Image::CopyImageSkia() const {
   return new ImageSkia(*ToImageSkia());
 }