ui/gfx/icon_util_unittest.cc - Issue 1406403007: Eliminate HICON leaks caused by creating icons from bitmap image.

Unified Diff: ui/gfx/icon_util_unittest.cc

Issue 1406403007: Eliminate HICON leaks caused by creating icons from bitmap image. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Use ScopedHICON instead of HICON. Created 5 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: ui/gfx/icon_util_unittest.cc

diff --git a/ui/gfx/icon_util_unittest.cc b/ui/gfx/icon_util_unittest.cc

index 5138e702a521da77e18017d93438602fcb92eb3f..c60b81e9fb8976f7cb00b4341586e9a36899a5a2 100644

--- a/ui/gfx/icon_util_unittest.cc

+++ b/ui/gfx/icon_util_unittest.cc

@@ -16,6 +16,8 @@

#include "ui/gfx/image/image.h"

#include "ui/gfx/image/image_family.h"

+using base::win::ScopedHICON;

grt (UTC plus 2) 2015/11/10 16:44:43 nit: can you move this into the public: section of

namespace {

static const char kSmallIconName[] = "icon_util/16_X_16_icon.ico";

@@ -39,15 +41,16 @@ class IconUtilTest : public testing::Test {

// Given a file name for an .ico file and an image dimensions, this

// function loads the icon and returns an HICON handle.

- HICON LoadIconFromFile(const base::FilePath& filename,

- int width, int height) {

+ ScopedHICON LoadIconFromFile(const base::FilePath& filename,

+ int width,

+ int height) {

HICON icon = static_cast<HICON>(LoadImage(NULL,

filename.value().c_str(),

IMAGE_ICON,

width,

height,

LR_LOADTRANSPARENT | LR_LOADFROMFILE));

- return icon;

+ return ScopedHICON(icon).Pass();

grt (UTC plus 2) 2015/11/10 16:44:43 can you remove .Pass() here?

}

SkBitmap CreateBlackSkBitmap(int width, int height) {

@@ -85,11 +88,10 @@ void IconUtilTest::CheckAllIconSizes(const base::FilePath& icon_filename,

}

// First, use the Windows API to load the icon, a basic validity test.

- HICON icon = LoadIconFromFile(icon_filename, kSmallIconWidth,

- kSmallIconHeight);

- EXPECT_NE(static_cast<HICON>(NULL), icon);

- if (icon != NULL)

- ::DestroyIcon(icon);

+ ScopedHICON icon =

+ LoadIconFromFile(icon_filename, kSmallIconWidth, kSmallIconHeight).Pass();

+ EXPECT_TRUE(icon.is_valid());

grt (UTC plus 2) 2015/11/10 16:44:43 EXPECT_TRUE(LoadIconFromFile(icon_filename, kSmall

+ icon.reset();

// Read the file completely into memory.

std::string icon_data;

@@ -143,14 +145,14 @@ TEST_F(IconUtilTest, TestIconToBitmapInvalidParameters) {

base::FilePath icon_filename =

test_data_directory_.AppendASCII(kSmallIconName);

gfx::Size icon_size(kSmallIconWidth, kSmallIconHeight);

- HICON icon = LoadIconFromFile(icon_filename,

- icon_size.width(),

- icon_size.height());

- ASSERT_TRUE(icon != NULL);

+ ScopedHICON icon =

grt (UTC plus 2) 2015/11/10 16:44:43 can you use: ScopedHICON icon(LoadIconFromFile(i

+ LoadIconFromFile(icon_filename, icon_size.width(), icon_size.height())

+ .Pass();

+ ASSERT_TRUE(icon.is_valid());

// Invalid size parameter.

gfx::Size invalid_icon_size(kSmallIconHeight, 0);

- EXPECT_EQ(IconUtil::CreateSkBitmapFromHICON(icon, invalid_icon_size),

+ EXPECT_EQ(IconUtil::CreateSkBitmapFromHICON(icon.get(), invalid_icon_size),

static_cast<SkBitmap*>(NULL));

// Invalid icon.

@@ -159,38 +161,37 @@ TEST_F(IconUtilTest, TestIconToBitmapInvalidParameters) {

// The following code should succeed.

scoped_ptr<SkBitmap> bitmap;

- bitmap.reset(IconUtil::CreateSkBitmapFromHICON(icon, icon_size));

+ bitmap.reset(IconUtil::CreateSkBitmapFromHICON(icon.get(), icon_size));

EXPECT_NE(bitmap.get(), static_cast<SkBitmap*>(NULL));

- ::DestroyIcon(icon);

}

// The following test case makes sure IconUtil::CreateHICONFromSkBitmap fails

// gracefully when called with invalid input parameters.

TEST_F(IconUtilTest, TestBitmapToIconInvalidParameters) {

- HICON icon = NULL;

+ ScopedHICON icon;

scoped_ptr<SkBitmap> bitmap;

// Wrong bitmap format.

bitmap.reset(new SkBitmap);

ASSERT_NE(bitmap.get(), static_cast<SkBitmap*>(NULL));

bitmap->setInfo(SkImageInfo::MakeA8(kSmallIconWidth, kSmallIconHeight));

- icon = IconUtil::CreateHICONFromSkBitmap(*bitmap);

- EXPECT_EQ(icon, static_cast<HICON>(NULL));

+ icon = IconUtil::CreateHICONFromSkBitmap(*bitmap).Pass();

+ EXPECT_FALSE(icon.is_valid());

// Invalid bitmap size.

bitmap.reset(new SkBitmap);

ASSERT_NE(bitmap.get(), static_cast<SkBitmap*>(NULL));

bitmap->setInfo(SkImageInfo::MakeN32Premul(0, 0));

- icon = IconUtil::CreateHICONFromSkBitmap(*bitmap);

- EXPECT_EQ(icon, static_cast<HICON>(NULL));

+ icon = IconUtil::CreateHICONFromSkBitmap(*bitmap).Pass();

+ EXPECT_FALSE(icon.is_valid());

// Valid bitmap configuration but no pixels allocated.

bitmap.reset(new SkBitmap);

ASSERT_NE(bitmap.get(), static_cast<SkBitmap*>(NULL));

bitmap->setInfo(SkImageInfo::MakeN32Premul(kSmallIconWidth,

kSmallIconHeight));

- icon = IconUtil::CreateHICONFromSkBitmap(*bitmap);

- EXPECT_TRUE(icon == NULL);

+ icon = IconUtil::CreateHICONFromSkBitmap(*bitmap).Pass();

+ EXPECT_FALSE(icon.is_valid());

}

// The following test case makes sure IconUtil::CreateIconFileFromImageFamily

@@ -273,30 +274,32 @@ TEST_F(IconUtilTest, TestCreateSkBitmapFromHICON) {

base::FilePath small_icon_filename = test_data_directory_.AppendASCII(

kSmallIconName);

gfx::Size small_icon_size(kSmallIconWidth, kSmallIconHeight);

- HICON small_icon = LoadIconFromFile(small_icon_filename,

- small_icon_size.width(),

- small_icon_size.height());

- ASSERT_NE(small_icon, static_cast<HICON>(NULL));

- bitmap.reset(IconUtil::CreateSkBitmapFromHICON(small_icon, small_icon_size));

+ ScopedHICON small_icon =

+ LoadIconFromFile(small_icon_filename, small_icon_size.width(),

+ small_icon_size.height())

+ .Pass();

+ ASSERT_TRUE(small_icon.is_valid());

+ bitmap.reset(

+ IconUtil::CreateSkBitmapFromHICON(small_icon.get(), small_icon_size));

ASSERT_NE(bitmap.get(), static_cast<SkBitmap*>(NULL));

EXPECT_EQ(bitmap->width(), small_icon_size.width());

EXPECT_EQ(bitmap->height(), small_icon_size.height());

EXPECT_EQ(bitmap->colorType(), kN32_SkColorType);

- ::DestroyIcon(small_icon);

base::FilePath large_icon_filename = test_data_directory_.AppendASCII(

kLargeIconName);

gfx::Size large_icon_size(kLargeIconWidth, kLargeIconHeight);

- HICON large_icon = LoadIconFromFile(large_icon_filename,

- large_icon_size.width(),

- large_icon_size.height());

- ASSERT_NE(large_icon, static_cast<HICON>(NULL));

- bitmap.reset(IconUtil::CreateSkBitmapFromHICON(large_icon, large_icon_size));

+ ScopedHICON large_icon =

+ LoadIconFromFile(large_icon_filename, large_icon_size.width(),

+ large_icon_size.height())

+ .Pass();

+ ASSERT_TRUE(large_icon.is_valid());

+ bitmap.reset(

+ IconUtil::CreateSkBitmapFromHICON(large_icon.get(), large_icon_size));

ASSERT_NE(bitmap.get(), static_cast<SkBitmap*>(NULL));

EXPECT_EQ(bitmap->width(), large_icon_size.width());

EXPECT_EQ(bitmap->height(), large_icon_size.height());

EXPECT_EQ(bitmap->colorType(), kN32_SkColorType);

- ::DestroyIcon(large_icon);

}

// This test case makes sure that when an HICON is created from an SkBitmap,

@@ -304,10 +307,10 @@ TEST_F(IconUtilTest, TestCreateSkBitmapFromHICON) {

// dimensions color depth etc.

TEST_F(IconUtilTest, TestBasicCreateHICONFromSkBitmap) {

SkBitmap bitmap = CreateBlackSkBitmap(kSmallIconWidth, kSmallIconHeight);

- HICON icon = IconUtil::CreateHICONFromSkBitmap(bitmap);

- EXPECT_NE(icon, static_cast<HICON>(NULL));

+ ScopedHICON icon = IconUtil::CreateHICONFromSkBitmap(bitmap).Pass();

grt (UTC plus 2) 2015/11/10 16:44:43 ScopedHICON icon(IconUtil::CreateHICONFromSkBitmap

+ EXPECT_TRUE(icon.is_valid());

ICONINFO icon_info;

- ASSERT_TRUE(::GetIconInfo(icon, &icon_info));

+ ASSERT_TRUE(GetIconInfo(icon.get(), &icon_info));

EXPECT_TRUE(icon_info.fIcon);

// Now that have the icon information, we should obtain the specification of

@@ -334,7 +337,6 @@ TEST_F(IconUtilTest, TestBasicCreateHICONFromSkBitmap) {

EXPECT_EQ(bitmap_info.bmiHeader.biPlanes, 1);

EXPECT_EQ(bitmap_info.bmiHeader.biBitCount, 32);

::ReleaseDC(NULL, hdc);

- ::DestroyIcon(icon);

}

// This test case makes sure that CreateIconFileFromImageFamily creates a

« ui/gfx/icon_util.cc ('K') | « ui/gfx/icon_util.cc ('k') | ui/gfx/path_win.cc » ('j') | no next file with comments »