Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(35)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: components/history/core/browser/thumbnail_database_unittest.cc

Issue 2823093002: Make FaviconService::GetRawFaviconForPageURL() select the best candidate among all the icon types (Closed)
Patch Set: Merge branch 'master' into icon_type Created 3 years, 8 months 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
« components/favicon/core/favicon_service.h ('K') | « components/history/core/browser/thumbnail_database.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: components/history/core/browser/thumbnail_database_unittest.cc
diff --git a/components/history/core/browser/thumbnail_database_unittest.cc b/components/history/core/browser/thumbnail_database_unittest.cc
index b20bbaf3f78998b31d4eba3fa3a6b3ae2d96df74..36dcd581c03b0a88c3620015cc2a3796147cab93 100644
--- a/components/history/core/browser/thumbnail_database_unittest.cc
+++ b/components/history/core/browser/thumbnail_database_unittest.cc
@@ -41,10 +41,11 @@ const GURL kPageUrl3 = GURL("http://www.google.com/");
const GURL kPageUrl4 = GURL("http://www.google.com/blank.html");
const GURL kPageUrl5 = GURL("http://www.bing.com/");
-const GURL kIconUrl1 = GURL("http://www.google.com/favicon.ico");
-const GURL kIconUrl2 = GURL("http://www.yahoo.com/favicon.ico");
-const GURL kIconUrl3 = GURL("http://www.google.com/touch.ico");
-const GURL kIconUrl5 = GURL("http://www.bing.com/favicon.ico");
+const GURL kIconUrl1 = GURL("http://www.bing.com/favicon.ico");
+const GURL kIconUrl2 = GURL("http://www.duckduckgo.com/favicon.ico");
+const GURL kIconUrl3 = GURL("http://www.google.com/favicon.ico");
+const GURL kIconUrl4 = GURL("http://www.google.com/touch.ico");
+const GURL kIconUrl5 = GURL("http://www.yahoo.com/favicon.ico");
const gfx::Size kSmallSize = gfx::Size(16, 16);
const gfx::Size kLargeSize = gfx::Size(32, 32);
@@ -76,6 +77,19 @@ void VerifyTablesAndColumns(sql::Connection* db) {
EXPECT_EQ(3u, sql::test::CountTableColumns(db, "icon_mapping"));
}
+// Adds a favicon at |icon_url| with |icon_type| with default bitmap data and
+// maps |page_url| to |icon_url|.
+void AddFaviconAndMapSimple(ThumbnailDatabase* db,
+ const GURL& page_url,
+ const GURL& icon_url,
+ favicon_base::IconType icon_type) {
+ scoped_refptr<base::RefCountedStaticMemory> data(
+ new base::RefCountedStaticMemory(kBlob1, sizeof(kBlob1)));
+ favicon_base::FaviconID favicon_id =
+ db->AddFavicon(icon_url, icon_type, data, base::Time::Now(), gfx::Size());
+ db->AddIconMapping(page_url, favicon_id);
+}
+
void VerifyDatabaseEmpty(sql::Connection* db) {
size_t rows = 0;
EXPECT_TRUE(sql::test::CountTableRows(db, "favicons", &rows));
@@ -147,6 +161,14 @@ WARN_UNUSED_RESULT bool CheckPageHasIcon(
return true;
}
+bool CompareIconMappingIconUrl(const IconMapping& a, const IconMapping& b) {
+ return a.icon_url < b.icon_url;
+}
+
+void SortMappingsByIconUrl(std::vector<IconMapping>* mappings) {
+ std::sort(mappings->begin(), mappings->end(), &CompareIconMappingIconUrl);
+}
+
} // namespace
class ThumbnailDatabaseTest : public testing::Test {
@@ -369,8 +391,8 @@ TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrls) {
kBlob2));
// The ones not retained should be missing.
- EXPECT_FALSE(db.GetFaviconIDForFaviconURL(kPageUrl2, false, NULL));
- EXPECT_FALSE(db.GetFaviconIDForFaviconURL(kPageUrl4, false, NULL));
+ EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, nullptr));
+ EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl4, nullptr));
// Schema should be the same.
EXPECT_EQ(original_schema, db.db_.GetSchema());
@@ -391,8 +413,8 @@ TEST_F(ThumbnailDatabaseTest, RetainDataForPageUrlsExpiresRetainedFavicons) {
EXPECT_TRUE(db.RetainDataForPageUrls(std::vector<GURL>(1u, kPageUrl1)));
- favicon_base::FaviconID new_favicon_id = db.GetFaviconIDForFaviconURL(
- kIconUrl1, favicon_base::FAVICON, nullptr);
+ favicon_base::FaviconID new_favicon_id =
+ db.GetFaviconIDForFaviconURL(kIconUrl1, favicon_base::FAVICON);
ASSERT_NE(0, new_favicon_id);
std::vector<FaviconBitmap> new_favicon_bitmaps;
db.GetFaviconBitmaps(new_favicon_id, &new_favicon_bitmaps);
@@ -427,124 +449,31 @@ TEST_F(ThumbnailDatabaseTest, DeleteFavicon) {
EXPECT_FALSE(db.GetFaviconBitmaps(id, NULL));
}
-TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLForReturnOrder) {
+// Test that when multiple icon types are passed to GetIconMappingsForPageURL()
+// that the results are filtered according to the passed in types.
+TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLWithIconTypes) {
ThumbnailDatabase db(NULL);
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_));
db.BeginTransaction();
- // Add a favicon
- std::vector<unsigned char> data(kBlob1, kBlob1 + sizeof(kBlob1));
- scoped_refptr<base::RefCountedBytes> favicon(new base::RefCountedBytes(data));
-
- GURL page_url("http://google.com");
- GURL icon_url("http://google.com/favicon.ico");
- base::Time time = base::Time::Now();
-
- favicon_base::FaviconID id = db.AddFavicon(
- icon_url, favicon_base::FAVICON, favicon, time, gfx::Size());
- EXPECT_NE(0, db.AddIconMapping(page_url, id));
- std::vector<IconMapping> icon_mappings;
- EXPECT_TRUE(db.GetIconMappingsForPageURL(page_url, &icon_mappings));
-
- EXPECT_EQ(page_url, icon_mappings.front().page_url);
- EXPECT_EQ(id, icon_mappings.front().icon_id);
- EXPECT_EQ(favicon_base::FAVICON, icon_mappings.front().icon_type);
- EXPECT_EQ(icon_url, icon_mappings.front().icon_url);
-
- // Add a touch icon
- std::vector<unsigned char> data2(kBlob2, kBlob2 + sizeof(kBlob2));
- scoped_refptr<base::RefCountedBytes> favicon2 =
- new base::RefCountedBytes(data);
-
- favicon_base::FaviconID id2 = db.AddFavicon(
- icon_url, favicon_base::TOUCH_ICON, favicon2, time, gfx::Size());
- EXPECT_NE(0, db.AddIconMapping(page_url, id2));
-
- icon_mappings.clear();
- EXPECT_TRUE(db.GetIconMappingsForPageURL(page_url, &icon_mappings));
-
- EXPECT_EQ(page_url, icon_mappings.front().page_url);
- EXPECT_EQ(id2, icon_mappings.front().icon_id);
- EXPECT_EQ(favicon_base::TOUCH_ICON, icon_mappings.front().icon_type);
- EXPECT_EQ(icon_url, icon_mappings.front().icon_url);
-
- // Add a touch precomposed icon
- scoped_refptr<base::RefCountedBytes> favicon3 =
- new base::RefCountedBytes(data2);
-
- favicon_base::FaviconID id3 =
- db.AddFavicon(icon_url,
- favicon_base::TOUCH_PRECOMPOSED_ICON,
- favicon3,
- time,
- gfx::Size());
- EXPECT_NE(0, db.AddIconMapping(page_url, id3));
-
- icon_mappings.clear();
- EXPECT_TRUE(db.GetIconMappingsForPageURL(page_url, &icon_mappings));
-
- EXPECT_EQ(page_url, icon_mappings.front().page_url);
- EXPECT_EQ(id3, icon_mappings.front().icon_id);
- EXPECT_EQ(favicon_base::TOUCH_PRECOMPOSED_ICON,
- icon_mappings.front().icon_type);
- EXPECT_EQ(icon_url, icon_mappings.front().icon_url);
-}
-
-// Test result of GetIconMappingsForPageURL when an icon type is passed in.
-TEST_F(ThumbnailDatabaseTest, GetIconMappingsForPageURLWithIconType) {
- ThumbnailDatabase db(NULL);
- ASSERT_EQ(sql::INIT_OK, db.Init(file_name_));
- db.BeginTransaction();
-
- GURL url("http://google.com");
- std::vector<unsigned char> data(kBlob1, kBlob1 + sizeof(kBlob1));
- scoped_refptr<base::RefCountedBytes> favicon(new base::RefCountedBytes(data));
- base::Time time = base::Time::Now();
-
- favicon_base::FaviconID id1 =
- db.AddFavicon(url, favicon_base::FAVICON, favicon, time, gfx::Size());
- EXPECT_NE(0, db.AddIconMapping(url, id1));
-
- favicon_base::FaviconID id2 =
- db.AddFavicon(url, favicon_base::TOUCH_ICON, favicon, time, gfx::Size());
- EXPECT_NE(0, db.AddIconMapping(url, id2));
-
- favicon_base::FaviconID id3 =
- db.AddFavicon(url, favicon_base::TOUCH_ICON, favicon, time, gfx::Size());
- EXPECT_NE(0, db.AddIconMapping(url, id3));
+ GURL page_url("http://www.google.com");
+ AddFaviconAndMapSimple(&db, page_url, kIconUrl1, favicon_base::FAVICON);
+ AddFaviconAndMapSimple(&db, page_url, kIconUrl2, favicon_base::TOUCH_ICON);
+ AddFaviconAndMapSimple(&db, page_url, kIconUrl3, favicon_base::TOUCH_ICON);
+ AddFaviconAndMapSimple(&db, page_url, kIconUrl4,
+ favicon_base::TOUCH_PRECOMPOSED_ICON);
- // Only the mappings for favicons of type TOUCH_ICON should be returned as
- // TOUCH_ICON is a larger icon type than FAVICON.
+ // Only the mappings for FAVICON and TOUCH_ICON should be returned.
std::vector<IconMapping> icon_mappings;
EXPECT_TRUE(db.GetIconMappingsForPageURL(
- url,
- favicon_base::FAVICON | favicon_base::TOUCH_ICON |
- favicon_base::TOUCH_PRECOMPOSED_ICON,
+ page_url, favicon_base::FAVICON | favicon_base::TOUCH_ICON,
&icon_mappings));
+ SortMappingsByIconUrl(&icon_mappings);
- EXPECT_EQ(2u, icon_mappings.size());
- if (id2 == icon_mappings[0].icon_id) {
- EXPECT_EQ(id3, icon_mappings[1].icon_id);
- } else {
- EXPECT_EQ(id3, icon_mappings[0].icon_id);
- EXPECT_EQ(id2, icon_mappings[1].icon_id);
- }
-
- icon_mappings.clear();
- EXPECT_TRUE(db.GetIconMappingsForPageURL(
- url, favicon_base::TOUCH_ICON, &icon_mappings));
- if (id2 == icon_mappings[0].icon_id) {
- EXPECT_EQ(id3, icon_mappings[1].icon_id);
- } else {
- EXPECT_EQ(id3, icon_mappings[0].icon_id);
- EXPECT_EQ(id2, icon_mappings[1].icon_id);
- }
-
- icon_mappings.clear();
- EXPECT_TRUE(
- db.GetIconMappingsForPageURL(url, favicon_base::FAVICON, &icon_mappings));
- EXPECT_EQ(1u, icon_mappings.size());
- EXPECT_EQ(id1, icon_mappings[0].icon_id);
+ EXPECT_EQ(3u, icon_mappings.size());
+ EXPECT_EQ(kIconUrl1, icon_mappings[0].icon_url);
+ EXPECT_EQ(kIconUrl2, icon_mappings[1].icon_url);
+ EXPECT_EQ(kIconUrl3, icon_mappings[2].icon_url);
}
TEST_F(ThumbnailDatabaseTest, HasMappingFor) {
@@ -646,34 +575,14 @@ TEST_F(ThumbnailDatabaseTest, Version7) {
ASSERT_TRUE(db.get() != NULL);
VerifyTablesAndColumns(&db->db_);
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl1,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
- kBlob1));
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl2,
- favicon_base::FAVICON,
- kIconUrl2,
- kLargeSize,
- sizeof(kBlob2),
- kBlob2));
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl3,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
- kBlob1));
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl3,
- favicon_base::TOUCH_ICON,
- kIconUrl3,
- kLargeSize,
- sizeof(kBlob2),
- kBlob2));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl1, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1), kBlob1));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl2, favicon_base::FAVICON,
+ kIconUrl5, kLargeSize, sizeof(kBlob2), kBlob2));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1), kBlob1));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, favicon_base::TOUCH_ICON,
+ kIconUrl4, kLargeSize, sizeof(kBlob2), kBlob2));
}
// Test loading version 8 database.
@@ -682,34 +591,14 @@ TEST_F(ThumbnailDatabaseTest, Version8) {
ASSERT_TRUE(db.get() != NULL);
VerifyTablesAndColumns(&db->db_);
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl1,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
- kBlob1));
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl2,
- favicon_base::FAVICON,
- kIconUrl2,
- kLargeSize,
- sizeof(kBlob2),
- kBlob2));
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl3,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
- kBlob1));
- EXPECT_TRUE(CheckPageHasIcon(db.get(),
- kPageUrl3,
- favicon_base::TOUCH_ICON,
- kIconUrl3,
- kLargeSize,
- sizeof(kBlob2),
- kBlob2));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl1, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1), kBlob1));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl2, favicon_base::FAVICON,
+ kIconUrl5, kLargeSize, sizeof(kBlob2), kBlob2));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1), kBlob1));
+ EXPECT_TRUE(CheckPageHasIcon(db.get(), kPageUrl3, favicon_base::TOUCH_ICON,
+ kIconUrl4, kLargeSize, sizeof(kBlob2), kBlob2));
}
TEST_F(ThumbnailDatabaseTest, Recovery) {
@@ -736,19 +625,11 @@ TEST_F(ThumbnailDatabaseTest, Recovery) {
ThumbnailDatabase db(NULL);
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_));
- EXPECT_TRUE(CheckPageHasIcon(&db,
- kPageUrl1,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
+ EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1),
kBlob1));
- EXPECT_TRUE(CheckPageHasIcon(&db,
- kPageUrl2,
- favicon_base::FAVICON,
- kIconUrl2,
- kLargeSize,
- sizeof(kBlob2),
+ EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl2, favicon_base::FAVICON,
+ kIconUrl5, kLargeSize, sizeof(kBlob2),
kBlob2));
}
@@ -807,12 +688,8 @@ TEST_F(ThumbnailDatabaseTest, Recovery) {
EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL));
// Other data was retained by recovery.
- EXPECT_TRUE(CheckPageHasIcon(&db,
- kPageUrl1,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
+ EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1),
kBlob1));
}
@@ -837,12 +714,8 @@ TEST_F(ThumbnailDatabaseTest, Recovery) {
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_));
EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL));
- EXPECT_TRUE(CheckPageHasIcon(&db,
- kPageUrl1,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
+ EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1),
kBlob1));
ASSERT_TRUE(expecter.SawExpectedErrors());
@@ -919,12 +792,8 @@ TEST_F(ThumbnailDatabaseTest, Recovery7) {
EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL));
// Other data was retained by recovery.
- EXPECT_TRUE(CheckPageHasIcon(&db,
- kPageUrl1,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
+ EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1),
kBlob1));
}
@@ -949,12 +818,8 @@ TEST_F(ThumbnailDatabaseTest, Recovery7) {
ASSERT_EQ(sql::INIT_OK, db.Init(file_name_));
EXPECT_FALSE(db.GetIconMappingsForPageURL(kPageUrl2, NULL));
- EXPECT_TRUE(CheckPageHasIcon(&db,
- kPageUrl1,
- favicon_base::FAVICON,
- kIconUrl1,
- kLargeSize,
- sizeof(kBlob1),
+ EXPECT_TRUE(CheckPageHasIcon(&db, kPageUrl1, favicon_base::FAVICON,
+ kIconUrl3, kLargeSize, sizeof(kBlob1),
kBlob1));
ASSERT_TRUE(expecter.SawExpectedErrors());
« components/favicon/core/favicon_service.h ('K') | « components/history/core/browser/thumbnail_database.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698