From 39f121a371d153d9beb6c3e1742f0c10bfda7c80 Mon Sep 17 00:00:00 2001 From: "pkotwicz@chromium.org" Date: Mon, 14 May 2012 00:30:31 +0000 Subject: 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 --- ui/base/resource/resource_bundle.cc | 8 ++++---- ui/gfx/image/image.cc | 7 ------- ui/gfx/image/image.h | 4 ---- ui/gfx/image/image_mac_unittest.mm | 8 ++------ ui/gfx/image/image_skia.cc | 15 --------------- ui/gfx/image/image_skia.h | 8 -------- ui/gfx/image/image_unittest.cc | 25 +++++++++++-------------- ui/gfx/image/image_unittest_util.cc | 36 ++++++++++++++++++++++++++---------- ui/gfx/image/image_unittest_util.h | 8 ++++++-- ui/gfx/image/image_util.cc | 6 +++--- 10 files changed, 52 insertions(+), 73 deletions(-) (limited to 'ui') 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 bitmap1x(gt::CreateBitmap(width1x, height1x)); - scoped_ptr 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 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 bitmap1(gt::CreateBitmap(width1x, height1x)); - scoped_ptr 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 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 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; } -- cgit v1.1