Get rid of Image::Image(SkBitmap*)

Bug=124566
Test=Compiles on CrOS,Mac

Review URL: https://chromiumcodereview.appspot.com/10378009

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

10 files changed, 52 insertions, 73 deletions

diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc
index f84e8db..9de5a4f 100644
--- a/ui/base/resource/resource_bundle.cc
+++ b/ui/base/resource/resource_bundle.cc
@@ -467,10 +467,10 @@ gfx::Image& ResourceBundle::GetEmptyImage() {
 
   if (empty_image_.IsEmpty()) {
     // The placeholder bitmap is bright red so people notice the problem.
-    SkBitmap* bitmap = new SkBitmap();
-    bitmap->setConfig(SkBitmap::kARGB_8888_Config, 32, 32);
-    bitmap->allocPixels();
-    bitmap->eraseARGB(255, 255, 0, 0);
+    SkBitmap bitmap;
+    bitmap.setConfig(SkBitmap::kARGB_8888_Config, 32, 32);
+    bitmap.allocPixels();
+    bitmap.eraseARGB(255, 255, 0, 0);
     empty_image_ = gfx::Image(bitmap);
   }
   return empty_image_;
diff --git a/ui/gfx/image/image.cc b/ui/gfx/image/image.cc
index 70324b1..a4863e7 100644
--- a/ui/gfx/image/image.cc
+++ b/ui/gfx/image/image.cc
@@ -233,13 +233,6 @@ Image::Image(const ImageSkia& image)
   AddRepresentation(rep);
 }
 
-Image::Image(const SkBitmap* bitmap)
-    : storage_(new internal::ImageStorage(Image::kImageRepSkia)) {
-  internal::ImageRepSkia* rep = new internal::ImageRepSkia(
-      new ImageSkia(bitmap));
-  AddRepresentation(rep);
-}
-
 Image::Image(const SkBitmap& bitmap)
     : storage_(new internal::ImageStorage(Image::kImageRepSkia)) {
   internal::ImageRepSkia* rep =
diff --git a/ui/gfx/image/image.h b/ui/gfx/image/image.h
index b5bb3bf..2fd4be3 100644
--- a/ui/gfx/image/image.h
+++ b/ui/gfx/image/image.h
@@ -65,10 +65,6 @@ class UI_EXPORT Image {
   // Creates a new image with the default representation.
   explicit Image(const ImageSkia& image);
 
-  // Creates a new image with the default representation. The object will take
-  // ownership of the image.
-  explicit Image(const SkBitmap* bitmap);
-
   // Creates a new image by copying the bitmap for use as the default
   // representation.
   explicit Image(const SkBitmap& bitmap);
diff --git a/ui/gfx/image/image_mac_unittest.mm b/ui/gfx/image/image_mac_unittest.mm
index 1de9c64..8a2119a 100644
--- a/ui/gfx/image/image_mac_unittest.mm
+++ b/ui/gfx/image/image_mac_unittest.mm
@@ -6,7 +6,6 @@
 
 #include "base/logging.h"
 #include "base/memory/scoped_nsobject.h"
-#include "base/memory/scoped_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image.h"
@@ -79,12 +78,9 @@ TEST_F(ImageMacTest, MultiResolutionImageSkiaToNSImage) {
   const int width2x = 20;
   const int height2x = 24;
 
-  scoped_ptr<SkBitmap> bitmap1x(gt::CreateBitmap(width1x, height1x));
-  scoped_ptr<SkBitmap> bitmap2x(gt::CreateBitmap(width2x, height2x));
-
   gfx::ImageSkia image_skia;
-  image_skia.AddBitmapForScale(*bitmap1x, 1.0f);
-  image_skia.AddBitmapForScale(*bitmap2x, 2.0f);
+  image_skia.AddBitmapForScale(gt::CreateBitmap(width1x, height1x), 1.0f);
+  image_skia.AddBitmapForScale(gt::CreateBitmap(width2x, height2x), 2.0f);
 
   gfx::Image image(image_skia);
 
diff --git a/ui/gfx/image/image_skia.cc b/ui/gfx/image/image_skia.cc
index d7ad469c..8015045 100644
--- a/ui/gfx/image/image_skia.cc
+++ b/ui/gfx/image/image_skia.cc
@@ -9,8 +9,6 @@
 
 #include "base/logging.h"
 #include "ui/gfx/size.h"
-#include "base/message_loop.h"
-#include "third_party/skia/include/core/SkPixelRef.h"
 
 namespace gfx {
 
@@ -59,19 +57,6 @@ ImageSkia::ImageSkia(const SkBitmap& bitmap, float dip_scale_factor) {
   Init(bitmap, dip_scale_factor);
 }
 
-ImageSkia::ImageSkia(const SkBitmap* bitmap) {
-  Init(*bitmap);
-
-  if (MessageLoop::current()) {
-    // Use DeleteSoon such that |bitmap| is still valid if caller uses |bitmap|
-    // immediately after having called constructor.
-    MessageLoop::current()->DeleteSoon(FROM_HERE, bitmap);
-  } else {
-    // Hit in unittests.
-    delete bitmap;
-  }
-}
-
 ImageSkia::ImageSkia(const ImageSkia& other) : storage_(other.storage_) {
 }
 
diff --git a/ui/gfx/image/image_skia.h b/ui/gfx/image/image_skia.h
index 8b911cb..9a28da3 100644
--- a/ui/gfx/image/image_skia.h
+++ b/ui/gfx/image/image_skia.h
@@ -42,14 +42,6 @@ class UI_EXPORT ImageSkia {
   // DIP width and height are set based on |dip_scale_factor|.
   ImageSkia(const SkBitmap& bitmap, float dip_scale_factor);
 
-  // Takes ownership of passed in bitmap.
-  // Caller should not assume that |bitmap| will be valid after constructor
-  // is called.
-  // DIP width and height are set based on scale factor of 1x.
-  // TODO(pkotwicz): This is temporary till conversion to gfx::ImageSkia is
-  // done.
-  explicit ImageSkia(const SkBitmap* bitmap);
-
   // Copies a reference to |other|'s storage.
   ImageSkia(const ImageSkia& other);
 
diff --git a/ui/gfx/image/image_unittest.cc b/ui/gfx/image/image_unittest.cc
index 7b50be8..a00205c 100644
--- a/ui/gfx/image/image_unittest.cc
+++ b/ui/gfx/image/image_unittest.cc
@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "base/memory/scoped_ptr.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image.h"
@@ -68,8 +67,8 @@ TEST_F(ImageTest, SkiaToSkia) {
 }
 
 TEST_F(ImageTest, SkiaRefToSkia) {
-  scoped_ptr<SkBitmap> originalBitmap(gt::CreateBitmap(25, 25));
-  gfx::Image image(*originalBitmap.get());
+  SkBitmap originalBitmap(gt::CreateBitmap(25, 25));
+  gfx::Image image(originalBitmap);
   const SkBitmap* bitmap = image.ToSkBitmap();
   EXPECT_TRUE(bitmap);
   EXPECT_FALSE(bitmap->isNull());
@@ -104,7 +103,7 @@ TEST_F(ImageTest, SkiaToPlatform) {
   if (!kUsesSkiaNatively)
     EXPECT_FALSE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
 
-  EXPECT_TRUE(gt::ToPlatformType(image));
+  EXPECT_TRUE(gt::IsPlatformImageValid(gt::ToPlatformType(image)));
   EXPECT_EQ(kRepCount, image.RepresentationCount());
 
   const SkBitmap* bitmap = image.ToSkBitmap();
@@ -128,7 +127,7 @@ TEST_F(ImageTest, PlatformToSkia) {
   EXPECT_FALSE(bitmap->isNull());
   EXPECT_EQ(kRepCount, image.RepresentationCount());
 
-  EXPECT_TRUE(gt::ToPlatformType(image));
+  EXPECT_TRUE(gt::IsPlatformImageValid(gt::ToPlatformType(image)));
   EXPECT_EQ(kRepCount, image.RepresentationCount());
 
   EXPECT_TRUE(image.HasRepresentation(gfx::Image::kImageRepSkia));
@@ -136,11 +135,11 @@ TEST_F(ImageTest, PlatformToSkia) {
 
 TEST_F(ImageTest, PlatformToPlatform) {
   gfx::Image image(gt::CreatePlatformImage());
-  EXPECT_TRUE(gt::ToPlatformType(image));
+  EXPECT_TRUE(gt::IsPlatformImageValid(gt::ToPlatformType(image)));
   EXPECT_EQ(1U, image.RepresentationCount());
 
   // Make sure double conversion doesn't happen.
-  EXPECT_TRUE(gt::ToPlatformType(image));
+  EXPECT_TRUE(gt::IsPlatformImageValid(gt::ToPlatformType(image)));
   EXPECT_EQ(1U, image.RepresentationCount());
 
   EXPECT_TRUE(image.HasRepresentation(gt::GetPlatformRepresentationType()));
@@ -221,7 +220,8 @@ TEST_F(ImageTest, SwapRepresentations) {
   image1.SwapRepresentations(&image2);
 
   EXPECT_EQ(bitmap2, image1.ToSkBitmap());
-  EXPECT_EQ(platform_image, gt::ToPlatformType(image1));
+  EXPECT_TRUE(gt::PlatformImagesEqual(platform_image,
+                                      gt::ToPlatformType(image1)));
   EXPECT_EQ(bitmap1, image2.ToSkBitmap());
   EXPECT_EQ(kRepCount, image1.RepresentationCount());
   EXPECT_EQ(1U, image2.RepresentationCount());
@@ -237,7 +237,7 @@ TEST_F(ImageTest, Copy) {
   EXPECT_EQ(1U, image2.RepresentationCount());
   EXPECT_EQ(image1.ToSkBitmap(), image2.ToSkBitmap());
 
-  EXPECT_TRUE(gt::ToPlatformType(image2));
+  EXPECT_TRUE(gt::IsPlatformImageValid(gt::ToPlatformType(image2)));
   EXPECT_EQ(kRepCount, image2.RepresentationCount());
   EXPECT_EQ(kRepCount, image1.RepresentationCount());
 }
@@ -257,12 +257,9 @@ TEST_F(ImageTest, MultiResolutionImage) {
   const int width2x = 20;
   const int height2x = 24;
 
-  scoped_ptr<SkBitmap> bitmap1(gt::CreateBitmap(width1x, height1x));
-  scoped_ptr<SkBitmap> bitmap2(gt::CreateBitmap(width2x, height2x));
-
   gfx::ImageSkia image_skia;
-  image_skia.AddBitmapForScale(*bitmap1, 1.0f);
-  image_skia.AddBitmapForScale(*bitmap2, 2.0f);
+  image_skia.AddBitmapForScale(gt::CreateBitmap(width1x, height1x), 1.0f);
+  image_skia.AddBitmapForScale(gt::CreateBitmap(width2x, height2x), 2.0f);
 
   EXPECT_EQ(2u, image_skia.bitmaps().size());
 
diff --git a/ui/gfx/image/image_unittest_util.cc b/ui/gfx/image/image_unittest_util.cc
index 6a72869..141db84 100644
--- a/ui/gfx/image/image_unittest_util.cc
+++ b/ui/gfx/image/image_unittest_util.cc
@@ -20,11 +20,11 @@
 namespace gfx {
 namespace test {
 
-SkBitmap* CreateBitmap(int width, int height) {
-  SkBitmap* bitmap = new SkBitmap();
-  bitmap->setConfig(SkBitmap::kARGB_8888_Config, width, height);
-  bitmap->allocPixels();
-  bitmap->eraseRGB(255, 0, 0);
+const SkBitmap CreateBitmap(int width, int height) {
+  SkBitmap bitmap;
+  bitmap.setConfig(SkBitmap::kARGB_8888_Config, width, height);
+  bitmap.allocPixels();
+  bitmap.eraseRGB(255, 0, 0);
   return bitmap;
 }
 
@@ -65,15 +65,15 @@ bool IsEmpty(const gfx::Image& image) {
 }
 
 PlatformImage CreatePlatformImage() {
-  scoped_ptr<SkBitmap> bitmap(CreateBitmap(25, 25));
+  const SkBitmap bitmap(CreateBitmap(25, 25));
 #if defined(OS_MACOSX)
-  NSImage* image = gfx::SkBitmapToNSImage(*(bitmap.get()));
+  NSImage* image = gfx::SkBitmapToNSImage(bitmap);
   base::mac::NSObjectRetain(image);
   return image;
 #elif defined(TOOLKIT_GTK)
-  return gfx::GdkPixbufFromSkBitmap(bitmap.get());
+  return gfx::GdkPixbufFromSkBitmap(&bitmap);
 #else
-  return bitmap.release();
+  return bitmap;
 #endif
 }
 
@@ -93,7 +93,23 @@ PlatformImage ToPlatformType(const gfx::Image& image) {
 #elif defined(TOOLKIT_GTK)
   return image.ToGdkPixbuf();
 #else
-  return image.ToSkBitmap();
+  return *image.ToSkBitmap();
+#endif
+}
+
+bool IsPlatformImageValid(PlatformImage image) {
+#if defined(OS_MACOSX) || defined(TOOLKIT_GTK)
+  return image != NULL;
+#else
+  return !image.isNull();
+#endif
+}
+
+bool PlatformImagesEqual(PlatformImage image1, PlatformImage image2) {
+#if defined(OS_MACOSX) || defined(TOOLKIT_GTK)
+  return image1 == image2;
+#else
+  return image1.getPixels() == image2.getPixels();
 #endif
 }
 
diff --git a/ui/gfx/image/image_unittest_util.h b/ui/gfx/image/image_unittest_util.h
index fb471b9..e617f26 100644
--- a/ui/gfx/image/image_unittest_util.h
+++ b/ui/gfx/image/image_unittest_util.h
@@ -18,10 +18,10 @@ typedef NSImage* PlatformImage;
 #elif defined(TOOLKIT_GTK)
 typedef GdkPixbuf* PlatformImage;
 #else
-typedef const SkBitmap* PlatformImage;
+typedef const SkBitmap PlatformImage;
 #endif
 
-SkBitmap* CreateBitmap(int width, int height);
+const SkBitmap CreateBitmap(int width, int height);
 
 gfx::Image CreateImage();
 
@@ -35,6 +35,10 @@ gfx::Image::RepresentationType GetPlatformRepresentationType();
 
 PlatformImage ToPlatformType(const gfx::Image& image);
 
+bool IsPlatformImageValid(PlatformImage image);
+
+bool PlatformImagesEqual(PlatformImage image1, PlatformImage image2);
+
 }  // namespace test
 }  // namespace gfx
 
diff --git a/ui/gfx/image/image_util.cc b/ui/gfx/image/image_util.cc
index 4216629..ea870c0 100644
--- a/ui/gfx/image/image_util.cc
+++ b/ui/gfx/image/image_util.cc
@@ -13,9 +13,9 @@
 namespace gfx {
 
 Image* ImageFromPNGEncodedData(const unsigned char* input, size_t input_size) {
-  scoped_ptr<SkBitmap> favicon_bitmap(new SkBitmap());
-  if (gfx::PNGCodec::Decode(input, input_size, favicon_bitmap.get()))
-    return new Image(favicon_bitmap.release());
+  SkBitmap favicon_bitmap;
+  if (gfx::PNGCodec::Decode(input, input_size, &favicon_bitmap))
+    return new Image(favicon_bitmap);
 
   return NULL;
 }