Revert 82838 - Temporary change to test perf regression (full)This change tests which part of this review:http://codereview.chromium.org/6897013/is causing a performance regression. This change will be reverted later tonight.
[email protected]
Review URL: http://codereview.chromium.org/6895050
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@82856 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/ui/base/resource/resource_bundle.cc b/ui/base/resource/resource_bundle.cc
index 306a61e..a8d9607 100644
--- a/ui/base/resource/resource_bundle.cc
+++ b/ui/base/resource/resource_bundle.cc
@@ -131,11 +131,8 @@
if (images_.count(resource_id))
return *images_[resource_id];
- std::vector<const SkBitmap*> bitmaps;
- bitmaps.push_back(bitmap.release());
- if (large_bitmap.get())
- bitmaps.push_back(large_bitmap.release());
- gfx::Image* image = new gfx::Image(bitmaps);
+ // TODO(sail): Add the large bitmap to the image as well.
+ gfx::Image* image = new gfx::Image(bitmap.release());
images_[resource_id] = image;
return *image;
}
diff --git a/ui/gfx/image.cc b/ui/gfx/image.cc
index 23edd96..9395279 100644
--- a/ui/gfx/image.cc
+++ b/ui/gfx/image.cc
@@ -7,7 +7,6 @@
#include <algorithm>
#include "base/logging.h"
-#include "base/stl_util-inl.h"
#include "third_party/skia/include/core/SkBitmap.h"
#if defined(OS_LINUX)
@@ -27,7 +26,7 @@
#if defined(OS_MACOSX)
// This is a wrapper around gfx::NSImageToSkBitmap() because this cross-platform
// file cannot include the [square brackets] of ObjC.
-bool NSImageToSkBitmaps(NSImage* image, std::vector<const SkBitmap*>* bitmaps);
+const SkBitmap* NSImageToSkBitmap(NSImage* image);
#endif
#if defined(OS_LINUX)
@@ -85,27 +84,20 @@
class SkBitmapRep : public ImageRep {
public:
explicit SkBitmapRep(const SkBitmap* bitmap)
- : ImageRep(Image::kSkBitmapRep) {
- CHECK(bitmap);
- bitmaps_.push_back(bitmap);
- }
-
- explicit SkBitmapRep(const std::vector<const SkBitmap*>& bitmaps)
: ImageRep(Image::kSkBitmapRep),
- bitmaps_(bitmaps) {
- CHECK(!bitmaps_.empty());
+ bitmap_(bitmap) {
+ CHECK(bitmap);
}
virtual ~SkBitmapRep() {
- STLDeleteElements(&bitmaps_);
+ delete bitmap_;
+ bitmap_ = NULL;
}
- const SkBitmap* bitmap() const { return bitmaps_[0]; }
-
- const std::vector<const SkBitmap*>& bitmaps() const { return bitmaps_; }
+ const SkBitmap* bitmap() const { return bitmap_; }
private:
- std::vector<const SkBitmap*> bitmaps_;
+ const SkBitmap* bitmap_;
DISALLOW_COPY_AND_ASSIGN(SkBitmapRep);
};
@@ -201,12 +193,6 @@
AddRepresentation(rep);
}
-Image::Image(const std::vector<const SkBitmap*>& bitmaps)
- : storage_(new internal::ImageStorage(Image::kSkBitmapRep)) {
- internal::SkBitmapRep* rep = new internal::SkBitmapRep(bitmaps);
- AddRepresentation(rep);
-}
-
#if defined(OS_LINUX)
Image::Image(GdkPixbuf* pixbuf)
: storage_(new internal::ImageStorage(Image::kGdkPixbufRep)) {
@@ -303,9 +289,8 @@
#elif defined(OS_MACOSX)
if (storage_->default_representation_type() == Image::kNSImageRep) {
internal::NSImageRep* nsimage_rep = default_rep->AsNSImageRep();
- std::vector<const SkBitmap*> bitmaps;
- CHECK(internal::NSImageToSkBitmaps(nsimage_rep->image(), &bitmaps));
- rep = new internal::SkBitmapRep(bitmaps);
+ rep = new internal::SkBitmapRep(
+ internal::NSImageToSkBitmap(nsimage_rep->image()));
}
#endif
CHECK(rep);
@@ -324,7 +309,7 @@
}
#elif defined(OS_MACOSX)
if (rep_type == Image::kNSImageRep) {
- NSImage* image = gfx::SkBitmapsToNSImage(skia_rep->bitmaps());
+ NSImage* image = gfx::SkBitmapToNSImage(*(skia_rep->bitmap()));
base::mac::NSObjectRetain(image);
native_rep = new internal::NSImageRep(image);
}
@@ -342,14 +327,4 @@
storage_->representations().insert(std::make_pair(rep->type(), rep));
}
-size_t Image::GetNumberOfSkBitmaps() {
- return GetRepresentation(Image::kSkBitmapRep)->AsSkBitmapRep()->
- bitmaps().size();
-}
-
-const SkBitmap* Image::GetSkBitmapAtIndex(size_t index) {
- return GetRepresentation(Image::kSkBitmapRep)->AsSkBitmapRep()->
- bitmaps()[index];
-}
-
} // namespace gfx
diff --git a/ui/gfx/image.h b/ui/gfx/image.h
index ed9508f..518d8ac4 100644
--- a/ui/gfx/image.h
+++ b/ui/gfx/image.h
@@ -18,7 +18,6 @@
#pragma once
#include <map>
-#include <vector>
#include "base/basictypes.h"
#include "base/gtest_prod_util.h"
@@ -30,7 +29,6 @@
namespace {
class ImageTest;
-class ImageMacTest;
}
namespace gfx {
@@ -53,18 +51,11 @@
// Creates a new image with the default representation. The object will take
// ownership of the image.
explicit Image(const SkBitmap* bitmap);
-
- // To create an Image that supports multiple resolutions pass a vector
- // of bitmaps, one for each resolution.
- explicit Image(const std::vector<const SkBitmap*>& bitmaps);
-
#if defined(OS_LINUX)
// Does not increase |pixbuf|'s reference count; expects to take ownership.
explicit Image(GdkPixbuf* pixbuf);
#elif defined(OS_MACOSX)
// Does not retain |image|; expects to take ownership.
- // A single NSImage object can contain multiple bitmaps so there's no reason
- // to pass a vector of these.
explicit Image(NSImage* image);
#endif
@@ -87,16 +78,6 @@
operator NSImage*();
#endif
- // Gets the number of bitmaps in this image. This may cause a conversion
- // to a bitmap representation. Note, this function and GetSkBitmapAtIndex()
- // are primarily meant to be used by the theme provider.
- size_t GetNumberOfSkBitmaps();
-
- // Gets the bitmap at the given index. This may cause a conversion
- // to a bitmap representation. Note, the internal ordering of bitmaps is not
- // guaranteed.
- const SkBitmap* GetSkBitmapAtIndex(size_t index);
-
// Inspects the representations map to see if the given type exists.
bool HasRepresentation(RepresentationType type);
@@ -122,7 +103,6 @@
scoped_refptr<internal::ImageStorage> storage_;
friend class ::ImageTest;
- friend class ::ImageMacTest;
};
} // namespace gfx
diff --git a/ui/gfx/image_mac.mm b/ui/gfx/image_mac.mm
index 55b4aa4..b3dc977 100644
--- a/ui/gfx/image_mac.mm
+++ b/ui/gfx/image_mac.mm
@@ -4,22 +4,14 @@
#import <AppKit/AppKit.h>
-#include "base/memory/scoped_ptr.h"
#include "skia/ext/skia_utils_mac.h"
#include "third_party/skia/include/core/SkBitmap.h"
namespace gfx {
namespace internal {
-bool NSImageToSkBitmaps(NSImage* image, std::vector<const SkBitmap*>* bitmaps) {
- for (NSImageRep* imageRep in [image representations]) {
- scoped_ptr<SkBitmap> bitmap(new SkBitmap(
- gfx::NSImageRepToSkBitmap(imageRep, [imageRep size], false)));
- if (bitmap->isNull())
- return false;
- bitmaps->push_back(bitmap.release());
- }
- return true;
+const SkBitmap* NSImageToSkBitmap(NSImage* image) {
+ return new SkBitmap(::gfx::NSImageToSkBitmap(image, [image size], false));
}
} // namespace internal
diff --git a/ui/gfx/image_unittest.cc b/ui/gfx/image_unittest.cc
index aba1ed6..ce2a1f8 100644
--- a/ui/gfx/image_unittest.cc
+++ b/ui/gfx/image_unittest.cc
@@ -6,7 +6,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/image.h"
-#include "ui/gfx/image_unittest_util.h"
+#include "ui/gfx/image_unittest.h"
#if defined(OS_LINUX)
#include <gtk/gtk.h>
@@ -30,7 +30,7 @@
namespace gt = gfx::test;
TEST_F(ImageTest, SkiaToSkia) {
- gfx::Image image(gt::CreateBitmap(25, 25));
+ gfx::Image image(gt::CreateBitmap());
const SkBitmap* bitmap = static_cast<const SkBitmap*>(image);
EXPECT_TRUE(bitmap);
EXPECT_FALSE(bitmap->isNull());
@@ -48,7 +48,7 @@
}
TEST_F(ImageTest, SkiaToSkiaRef) {
- gfx::Image image(gt::CreateBitmap(25, 25));
+ gfx::Image image(gt::CreateBitmap());
const SkBitmap& bitmap = static_cast<const SkBitmap&>(image);
EXPECT_FALSE(bitmap.isNull());
@@ -64,7 +64,7 @@
}
TEST_F(ImageTest, SkiaToPlatform) {
- gfx::Image image(gt::CreateBitmap(25, 25));
+ gfx::Image image(gt::CreateBitmap());
const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U;
EXPECT_TRUE(image.HasRepresentation(gfx::Image::kSkBitmapRep));
@@ -127,7 +127,7 @@
TEST_F(ImageTest, SwapRepresentations) {
const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U;
- gfx::Image image1(gt::CreateBitmap(25, 25));
+ gfx::Image image1(gt::CreateBitmap());
const SkBitmap* bitmap1 = image1;
EXPECT_EQ(1U, image1.RepresentationCount());
@@ -148,7 +148,7 @@
TEST_F(ImageTest, Copy) {
const size_t kRepCount = kUsesSkiaNatively ? 1U : 2U;
- gfx::Image image1(gt::CreateBitmap(25, 25));
+ gfx::Image image1(gt::CreateBitmap());
gfx::Image image2(image1);
EXPECT_EQ(1U, image1.RepresentationCount());
@@ -171,41 +171,6 @@
static_cast<const SkBitmap*>(image2));
}
-TEST_F(ImageTest, MultiResolutionSkBitmap) {
- const int width1 = 10;
- const int height1 = 12;
- const int width2 = 20;
- const int height2 = 24;
-
- std::vector<const SkBitmap*> bitmaps;
- bitmaps.push_back(gt::CreateBitmap(width1, height1));
- bitmaps.push_back(gt::CreateBitmap(width2, height2));
- gfx::Image image(bitmaps);
-
- EXPECT_EQ(1u, image.RepresentationCount());
- EXPECT_EQ(2u, image.GetNumberOfSkBitmaps());
-
- const SkBitmap* bitmap1 = image.GetSkBitmapAtIndex(0);
- EXPECT_TRUE(bitmap1);
- const SkBitmap* bitmap2 = image.GetSkBitmapAtIndex(1);
- EXPECT_TRUE(bitmap2);
-
- if (bitmap1->width() == width1) {
- EXPECT_EQ(bitmap1->height(), height1);
- EXPECT_EQ(bitmap2->width(), width2);
- EXPECT_EQ(bitmap2->height(), height2);
- } else {
- EXPECT_EQ(bitmap1->width(), width2);
- EXPECT_EQ(bitmap1->height(), height2);
- EXPECT_EQ(bitmap2->width(), width1);
- EXPECT_EQ(bitmap2->height(), height1);
- }
-
- // Sanity check.
- EXPECT_EQ(1u, image.RepresentationCount());
- EXPECT_EQ(2u, image.GetNumberOfSkBitmaps());
-}
-
// Integration tests with UI toolkit frameworks require linking against the
// Views library and cannot be here (gfx_unittests doesn't include it). They
// instead live in /chrome/browser/ui/tests/ui_gfx_image_unittest.cc.
diff --git a/ui/gfx/image_unittest.h b/ui/gfx/image_unittest.h
index e69de29..09abfce 100644
--- a/ui/gfx/image_unittest.h
+++ b/ui/gfx/image_unittest.h
@@ -0,0 +1,67 @@
+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Because the unit tests for gfx::Image are spread across multiple
+// implementation files, this header contains the reusable components.
+
+#ifndef UI_GFX_IMAGE_UNITTEST_H_
+#define UI_GFX_IMAGE_UNITTEST_H_
+
+#include "base/memory/scoped_ptr.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+
+#if defined(OS_LINUX)
+#include "ui/gfx/gtk_util.h"
+#elif defined(OS_MACOSX)
+#include "base/mac/mac_util.h"
+#include "skia/ext/skia_utils_mac.h"
+#endif
+
+namespace gfx {
+namespace test {
+
+#if defined(OS_MACOSX)
+typedef NSImage* PlatformImage;
+#elif defined(OS_LINUX) && !defined(TOOLKIT_VIEWS)
+typedef GdkPixbuf* PlatformImage;
+#else
+typedef const SkBitmap* PlatformImage;
+#endif
+
+SkBitmap* CreateBitmap() {
+ SkBitmap* bitmap = new SkBitmap();
+ bitmap->setConfig(SkBitmap::kARGB_8888_Config, 25, 25);
+ bitmap->allocPixels();
+ bitmap->eraseRGB(255, 0, 0);
+ return bitmap;
+}
+
+PlatformImage CreatePlatformImage() {
+ scoped_ptr<SkBitmap> bitmap(CreateBitmap());
+#if defined(OS_MACOSX)
+ NSImage* image = gfx::SkBitmapToNSImage(*(bitmap.get()));
+ base::mac::NSObjectRetain(image);
+ return image;
+#elif defined(OS_LINUX) && !defined(TOOLKIT_VIEWS)
+ return gfx::GdkPixbufFromSkBitmap(bitmap.get());
+#else
+ return bitmap.release();
+#endif
+}
+
+gfx::Image::RepresentationType GetPlatformRepresentationType() {
+#if defined(OS_MACOSX)
+ return gfx::Image::kNSImageRep;
+#elif defined(OS_LINUX) && !defined(TOOLKIT_VIEWS)
+ return gfx::Image::kGdkPixbufRep;
+#else
+ return gfx::Image::kSkBitmapRep;
+#endif
+}
+
+} // namespace test
+} // namespace gfx
+
+#endif // UI_GFX_IMAGE_UNITTEST_H_
diff --git a/ui/ui_unittests.gypi b/ui/ui_unittests.gypi
index 2867afb..2a767a4 100644
--- a/ui/ui_unittests.gypi
+++ b/ui/ui_unittests.gypi
@@ -32,10 +32,8 @@
'gfx/codec/png_codec_unittest.cc',
'gfx/color_utils_unittest.cc',
'gfx/font_unittest.cc',
- 'gfx/image_mac_unittest.mm',
'gfx/image_unittest.cc',
- 'gfx/image_unittest_util.h',
- 'gfx/image_unittest_util.cc',
+ 'gfx/image_unittest.h',
'gfx/insets_unittest.cc',
'gfx/rect_unittest.cc',
'gfx/run_all_unittests.cc',
