Changes favicon database to support storing bitmaps of different sizes for the same icon_url.

This moves the favicon bitmap data to a new table: favicon_frames.
The favicon_frame table contains columns:
icon_id
bitmap_data
last_updated
width
height

For favicons of type .ico, a single entry in the favicons table will have multiple associated frames.
For favicons of other image types, there should be a single frame for each favicon id.

Bug=138553
Test=ThumbnailDatabaseTest.*, HistoryBackendTest.* pass

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

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

chrome/browser/history/android/android_provider_backend.cc

@@ -966,11 +966,10 @@ bool AndroidProviderBackend::SimulateUpdateURL(
 
   FaviconID favicon_id = statement->statement()->ColumnInt64(4);
   if (favicon_id) {
-    scoped_refptr<base::RefCountedBytes> favicon = new base::RefCountedBytes();
-    if (!thumbnail_db_->GetFavicon(favicon_id, NULL, &favicon->data(), NULL,
-                                   NULL))
+    scoped_refptr<base::RefCountedMemory> favicon;
+    if (!thumbnail_db_->GetFavicon(favicon_id, NULL, &favicon, NULL, NULL))
       return false;
-    if (favicon->size())
+    if (favicon.get() && favicon->size())
       new_row.set_favicon(favicon);
     favicon_details->urls.insert(old_url_row.url());
     favicon_details->urls.insert(row.url());

chrome/browser/history/android/android_provider_backend_unittest.cc

@@ -1155,10 +1155,13 @@ TEST_F(AndroidProviderBackendTest, UpdateFavicon) {
   EXPECT_TRUE(thumbnail_db_.GetIconMappingForPageURL(row1.url(), FAVICON,
                                                      &icon_mapping));
   Time last_updated;
-  std::vector<unsigned char> png_icon_data;
+  scoped_refptr<base::RefCountedMemory> png_icon_data;
   EXPECT_TRUE(thumbnail_db_.GetFavicon(icon_mapping.icon_id, &last_updated,
                                        &png_icon_data, NULL, NULL));
-  EXPECT_EQ(data, png_icon_data);
+  std::string png_icon_data_as_string(png_icon_data->front(),
+      png_icon_data->front() + png_icon_data->size());
+  EXPECT_EQ(1u, png_icon_data_as_string.size());
+  EXPECT_EQ('1', png_icon_data_as_string[0]);
 
   // Remove favicon.
   HistoryAndBookmarkRow update_row2;

chrome/browser/history/android/favicon_sql_handler.cc

@@ -35,12 +35,16 @@ bool FaviconSQLHandler::Update(const HistoryAndBookmarkRow& row,
   if (row.favicon_valid()) {
     // If the image_data will be updated, it is not reasonable to find if the
     // icon is already in database, just create a new favicon.
-    favicon_id = thumbnail_db_->AddFavicon(GURL(), history::FAVICON);
-    if (!favicon_id)
-      return false;
+    // TODO(pkotwicz): Pass in real pixel size.
+    favicon_id = thumbnail_db_->AddFavicon(
+        GURL(),
+        history::FAVICON,
+        std::string("0 0"),
+        row.favicon(),
+        Time::Now(),
+        gfx::Size());
 
-    scoped_refptr<base::RefCountedMemory> image_data = row.favicon();
-    if (!thumbnail_db_->SetFavicon(favicon_id, image_data, Time::Now()))
+    if (!favicon_id)
       return false;
   }
 
@@ -110,13 +114,16 @@ bool FaviconSQLHandler::Insert(HistoryAndBookmarkRow* row) {
   DCHECK(row->is_value_set_explicitly(HistoryAndBookmarkRow::URL));
 
   // Is it a problem to give a empty URL?
-  FaviconID id = thumbnail_db_->AddFavicon(GURL(), history::FAVICON);
+  // TODO(pkotwicz): Pass in real pixel size.
+  FaviconID id = thumbnail_db_->AddFavicon(
+      GURL(),
+      history::FAVICON,
+      std::string("0 0"),
+      row->favicon(),
+      Time::Now(),
+      gfx::Size());
   if (!id)
     return false;
-
-  scoped_refptr<base::RefCountedMemory> image_data = row->favicon();
-  if (!thumbnail_db_->SetFavicon(id, image_data, Time::Now()))
-    return false;
   return thumbnail_db_->AddIconMapping(row->url(), id);
 }
 

chrome/browser/history/expire_history_backend_unittest.cc

@@ -319,11 +319,8 @@ void ExpireHistoryTest::AddExampleSourceData(const GURL& url, URLID* id) {
 bool ExpireHistoryTest::HasFavicon(FaviconID favicon_id) {
   if (!thumb_db_.get() || favicon_id == 0)
     return false;
-  Time last_updated;
-  std::vector<unsigned char> icon_data_unused;
   GURL icon_url;
-  return thumb_db_->GetFavicon(favicon_id, &last_updated, &icon_data_unused,
-                               &icon_url, NULL);
+  return thumb_db_->GetFaviconHeader(favicon_id, &icon_url, NULL, NULL);
 }
 
 FaviconID ExpireHistoryTest::GetFavicon(const GURL& page_url,

chrome/browser/history/history_backend.cc

@@ -1795,7 +1795,7 @@ void HistoryBackend::SetFaviconOutOfDateForPage(const GURL& page_url) {
 
   for (std::vector<IconMapping>::iterator m = icon_mappings.begin();
        m != icon_mappings.end(); ++m) {
-    thumbnail_db_->SetFaviconLastUpdateTime(m->icon_id, Time());
+    thumbnail_db_->SetFaviconOutOfDate(m->icon_id);
   }
   ScheduleCommit();
 }
@@ -1828,12 +1828,14 @@ void HistoryBackend::SetImportedFavicons(
         favicon_usage[i].favicon_url, history::FAVICON, NULL);
     if (!favicon_id) {
       // This favicon doesn't exist yet, so we create it using the given data.
-      favicon_id = thumbnail_db_->AddFavicon(favicon_usage[i].favicon_url,
-                                             history::FAVICON);
-      if (!favicon_id)
-        continue;  // Unable to add the favicon.
-      thumbnail_db_->SetFavicon(favicon_id,
-          new base::RefCountedBytes(favicon_usage[i].png_data), now);
+      // TODO(pkotwicz): Pass in real pixel size.
+      favicon_id = thumbnail_db_->AddFavicon(
+          favicon_usage[i].favicon_url,
+          history::FAVICON,
+          std::string("0 0"),
+          new base::RefCountedBytes(favicon_usage[i].png_data),
+          now,
+          gfx::Size());
     }
 
     // Save the mapping from all the URLs to the favicon.
@@ -1896,10 +1898,10 @@ void HistoryBackend::UpdateFaviconMappingAndFetchImpl(
         thumbnail_db_->GetFaviconIDForFaviconURL(
             icon_url, icon_types, &favicon.icon_type);
     if (favicon_id) {
-      scoped_refptr<base::RefCountedBytes> data = new base::RefCountedBytes();
+      scoped_refptr<base::RefCountedMemory> data;
       favicon.known_icon = true;
       Time last_updated;
-      if (thumbnail_db_->GetFavicon(favicon_id, &last_updated, &data->data(),
+      if (thumbnail_db_->GetFavicon(favicon_id, &last_updated, &data,
                                     NULL, NULL)) {
         favicon.expired = (Time::Now() - last_updated) >
             TimeDelta::FromDays(kFaviconRefetchDays);
@@ -1951,11 +1953,15 @@ void HistoryBackend::SetFavicon(
 
   FaviconID id = thumbnail_db_->GetFaviconIDForFaviconURL(
       icon_url, icon_type, NULL);
-  if (!id)
-    id = thumbnail_db_->AddFavicon(icon_url, icon_type);
+  if (id)
+    thumbnail_db_->DeleteFaviconBitmapsForFavicon(id);
 
-  // Set the image data.
-  thumbnail_db_->SetFavicon(id, data, Time::Now());
+  id = thumbnail_db_->AddFavicon(icon_url,
+                                 icon_type,
+                                 "0 0",
+                                 data,
+                                 Time::Now(),
+                                 gfx::Size());
 
   SetFaviconMapping(page_url, id, icon_type);
 }
@@ -2334,12 +2340,9 @@ bool HistoryBackend::ClearAllThumbnailHistory(URLRows* kept_urls) {
     return true;
   }
 
-  // Create the duplicate favicon table, this is where the favicons we want
-  // to keep will be stored.
-  if (!thumbnail_db_->InitTemporaryFaviconsTable())
-    return false;
-
-  if (!thumbnail_db_->InitTemporaryIconMappingTable())
+  // Create duplicate icon_mapping, favicon, and favicon_bitmaps tables, this
+  // is where the favicons we want to keep will be stored.
+  if (!thumbnail_db_->InitTemporaryTables())
     return false;
 
   // This maps existing favicon IDs to the ones in the temporary table.
@@ -2359,7 +2362,8 @@ bool HistoryBackend::ClearAllThumbnailHistory(URLRows* kept_urls) {
       FaviconID new_id;
       FaviconMap::const_iterator found = copied_favicons.find(old_id);
       if (found == copied_favicons.end()) {
-        new_id = thumbnail_db_->CopyToTemporaryFaviconTable(old_id);
+        new_id = thumbnail_db_->CopyFaviconAndFaviconBitmapsToTemporaryTables(
+            old_id);
         copied_favicons[old_id] = new_id;
       } else {
         // We already encountered a URL that used this favicon, use the ID we
@@ -2376,10 +2380,10 @@ bool HistoryBackend::ClearAllThumbnailHistory(URLRows* kept_urls) {
   db_->ClearAndroidURLRows();
 #endif
 
-  // Rename the duplicate favicon and icon_mapping back table and recreate the
-  // other tables. This will make the database consistent again.
-  thumbnail_db_->CommitTemporaryFaviconTable();
-  thumbnail_db_->CommitTemporaryIconMappingTable();
+  // Drop original favicon_bitmaps, favicons, and icon mapping tables and
+  // replace them with the duplicate tables. Recreate the other tables. This
+  // will make the database consistent again.
+  thumbnail_db_->CommitTemporaryTables();
 
   thumbnail_db_->RecreateThumbnailTable();
 
@@ -2466,9 +2470,9 @@ bool HistoryBackend::GetFaviconFromDB(
 bool HistoryBackend::GetFaviconFromDB(FaviconID favicon_id,
                                       FaviconData* favicon) {
   Time last_updated;
-  scoped_refptr<base::RefCountedBytes> data = new base::RefCountedBytes();
+  scoped_refptr<base::RefCountedMemory> data;
 
-  if (!thumbnail_db_->GetFavicon(favicon_id, &last_updated, &data->data(),
+  if (!thumbnail_db_->GetFavicon(favicon_id, &last_updated, &data,
                                  &favicon->icon_url, &favicon->icon_type))
     return false;
 

chrome/browser/history/history_backend_unittest.cc

@@ -45,6 +45,9 @@ namespace {
 static const unsigned char blob1[] =
     "12346102356120394751634516591348710478123649165419234519234512349134";
 
+static const gfx::Size kSmallSize = gfx::Size(16, 16);
+static const gfx::Size kLargeSize = gfx::Size(48, 48);
+
 }  // namepace
 
 namespace history {
@@ -287,10 +290,10 @@ TEST_F(HistoryBackendTest, MAYBE_Loaded) {
 TEST_F(HistoryBackendTest, DeleteAll) {
   ASSERT_TRUE(backend_.get());
 
-  // Add two favicons, use the characters '1' and '2' for the image data. Note
-  // that we do these in the opposite order. This is so the first one gets ID
-  // 2 autoassigned to the database, which will change when the other one is
-  // deleted. This way we can test that updating works properly.
+  // Add two favicons, each with two bitmaps. Note that we add favicon2 before
+  // adding favicon1. This is so that favicon1 one gets ID 2 autoassigned to
+  // the database, which will change when the other one is deleted. This way
+  // we can test that updating works properly.
   GURL favicon_url1("http://www.google.com/favicon.ico");
   GURL favicon_url2("http://news.google.com/favicon.ico");
   FaviconID favicon2 = backend_->thumbnail_db_->AddFavicon(favicon_url2,
@@ -299,13 +302,19 @@ TEST_F(HistoryBackendTest, DeleteAll) {
                                                            FAVICON);
 
   std::vector<unsigned char> data;
-  data.push_back('1');
-  EXPECT_TRUE(backend_->thumbnail_db_->SetFavicon(favicon1,
-      new base::RefCountedBytes(data), Time::Now()));
-
-  data[0] = '2';
-  EXPECT_TRUE(backend_->thumbnail_db_->SetFavicon(
-                  favicon2, new base::RefCountedBytes(data), Time::Now()));
+  data.push_back('a');
+  EXPECT_TRUE(backend_->thumbnail_db_->AddFaviconBitmap(favicon1,
+      new base::RefCountedBytes(data), Time::Now(), kSmallSize));
+  data[0] = 'b';
+  EXPECT_TRUE(backend_->thumbnail_db_->AddFaviconBitmap(favicon1,
+     new base::RefCountedBytes(data), Time::Now(), kLargeSize));
+
+  data[0] = 'c';
+  EXPECT_TRUE(backend_->thumbnail_db_->AddFaviconBitmap(favicon2,
+      new base::RefCountedBytes(data), Time::Now(), kSmallSize));
+  data[0] = 'd';
+  EXPECT_TRUE(backend_->thumbnail_db_->AddFaviconBitmap(favicon2,
+     new base::RefCountedBytes(data), Time::Now(), kLargeSize));
 
   // First visit two URLs.
   URLRow row1(GURL("http://www.google.com/"));
@@ -401,11 +410,33 @@ TEST_F(HistoryBackendTest, DeleteAll) {
                                                          &out_data));
   EXPECT_FALSE(backend_->thumbnail_db_->GetPageThumbnail(row2_id, &out_data));
 
-  // We should have a favicon for the first URL only. We look them up by favicon
-  // URL since the IDs may hav changed.
+  // We should have a favicon and favicon bitmaps for the first URL only. We
+  // look them up by favicon URL since the IDs may have changed.
   FaviconID out_favicon1 = backend_->thumbnail_db_->
       GetFaviconIDForFaviconURL(favicon_url1, FAVICON, NULL);
   EXPECT_TRUE(out_favicon1);
+
+  std::vector<FaviconBitmap> favicon_bitmaps;
+  EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconBitmaps(
+      out_favicon1, &favicon_bitmaps));
+  ASSERT_EQ(2u, favicon_bitmaps.size());
+
+  FaviconBitmap favicon_bitmap1 = favicon_bitmaps[0];
+  FaviconBitmap favicon_bitmap2 = favicon_bitmaps[1];
+
+  // Bitmaps do not need to be in particular order.
+  if (favicon_bitmap1.pixel_size == kLargeSize) {
+    FaviconBitmap tmp_favicon_bitmap = favicon_bitmap1;
+    favicon_bitmap1 = favicon_bitmap2;
+    favicon_bitmap2 = tmp_favicon_bitmap;
+  }
+
+  EXPECT_EQ('a', *favicon_bitmap1.bitmap_data->front());
+  EXPECT_EQ(kSmallSize, favicon_bitmap1.pixel_size);
+
+  EXPECT_EQ('b', *favicon_bitmap2.bitmap_data->front());
+  EXPECT_EQ(kLargeSize, favicon_bitmap2.pixel_size);
+
   FaviconID out_favicon2 = backend_->thumbnail_db_->
       GetFaviconIDForFaviconURL(favicon_url2, FAVICON, NULL);
   EXPECT_FALSE(out_favicon2) << "Favicon not deleted";
@@ -485,19 +516,25 @@ TEST_F(HistoryBackendTest, DeleteAllThenAddData) {
 TEST_F(HistoryBackendTest, URLsNoLongerBookmarked) {
   GURL favicon_url1("http://www.google.com/favicon.ico");
   GURL favicon_url2("http://news.google.com/favicon.ico");
-  FaviconID favicon2 = backend_->thumbnail_db_->AddFavicon(favicon_url2,
-                                                           FAVICON);
-  FaviconID favicon1 = backend_->thumbnail_db_->AddFavicon(favicon_url1,
-                                                           FAVICON);
 
   std::vector<unsigned char> data;
   data.push_back('1');
-  EXPECT_TRUE(backend_->thumbnail_db_->SetFavicon(
-                  favicon1, new base::RefCountedBytes(data), Time::Now()));
+  FaviconID favicon1 = backend_->thumbnail_db_->AddFavicon(
+      favicon_url1,
+      FAVICON,
+      "0 0",
+      new base::RefCountedBytes(data),
+      Time::Now(),
+      gfx::Size());
 
   data[0] = '2';
-  EXPECT_TRUE(backend_->thumbnail_db_->SetFavicon(
-                  favicon2, new base::RefCountedBytes(data), Time::Now()));
+  FaviconID favicon2 = backend_->thumbnail_db_->AddFavicon(
+      favicon_url2,
+      FAVICON,
+      "0 0",
+      new base::RefCountedBytes(data),
+      Time::Now(),
+      gfx::Size());
 
   // First visit two URLs.
   URLRow row1(GURL("http://www.google.com/"));
@@ -662,12 +699,15 @@ TEST_F(HistoryBackendTest, ImportedFaviconsTest) {
   // Setup test data - two Urls in the history, one with favicon assigned and
   // one without.
   GURL favicon_url1("http://www.google.com/favicon.ico");
-  FaviconID favicon1 = backend_->thumbnail_db_->AddFavicon(favicon_url1,
-                                                           FAVICON);
   std::vector<unsigned char> data;
   data.push_back('1');
-  EXPECT_TRUE(backend_->thumbnail_db_->SetFavicon(favicon1,
-      base::RefCountedBytes::TakeVector(&data), Time::Now()));
+  FaviconID favicon1 = backend_->thumbnail_db_->AddFavicon(
+      favicon_url1,
+      FAVICON,
+      "0 0",
+      base::RefCountedBytes::TakeVector(&data),
+      Time::Now(),
+      gfx::Size());
   URLRow row1(GURL("http://www.google.com/"));
   row1.set_visit_count(1);
   row1.set_last_visit(Time::Now());

chrome/browser/history/history_types.cc

@@ -429,6 +429,7 @@ IconMapping::IconMapping()
 
 IconMapping::~IconMapping() {}
 
+// FaviconData ----------------------------------------------------------------
 
 FaviconData::FaviconData()
   : known_icon(false),
@@ -442,4 +443,14 @@ bool FaviconData::is_valid() {
   return known_icon && image_data.get() && image_data->size();
 }
 
+// FaviconBitmap --------------------------------------------------------------
+
+FaviconBitmap::FaviconBitmap()
+    : bitmap_id(0),
+      icon_id(0) {
+}
+
+FaviconBitmap::~FaviconBitmap() {
+}
+
 }  // namespace history

chrome/browser/history/history_types.h

@@ -22,6 +22,7 @@
 #include "chrome/common/thumbnail_score.h"
 #include "content/public/common/page_transition_types.h"
 #include "googleurl/src/gurl.h"
+#include "ui/gfx/size.h"
 
 class PageUsageData;
 
@@ -42,6 +43,7 @@ typedef int64 StarID;  // Unique identifier for star entries.
 typedef int64 UIStarID;  // Identifier for star entries that come from the UI.
 typedef int64 DownloadID;   // Identifier for a download.
 typedef int64 FaviconID;  // For favicons.
+typedef int64 FaviconBitmapID; // Identifier for a bitmap in a favicon.
 typedef int64 SegmentID;  // URL segments for the most visited view.
 typedef int64 IconMappingID; // For page url and icon mapping.
 
@@ -741,6 +743,8 @@ base::Time AutocompleteAgeThreshold();
 // AutocompleteAgeThreshold() (or any other desired time in the past).
 bool RowQualifiesAsSignificant(const URLRow& row, const base::Time& threshold);
 
+// Favicons -------------------------------------------------------------------
+
 // Defines the icon types. They are also stored in icon_type field of favicons
 // table.
 enum IconType {
@@ -792,6 +796,27 @@ struct FaviconData {
   history::IconType icon_type;
 };
 
+// Defines a favicon bitmap stored in the history backend.
+struct FaviconBitmap {
+  FaviconBitmap();
+  ~FaviconBitmap();
+
+  // The unique id of the bitmap.
+  FaviconBitmapID bitmap_id;
+
+  // The id of the favicon to which the bitmap belongs to.
+  FaviconID icon_id;
+
+  // Time at which |bitmap_data| was last updated.
+  base::Time last_updated;
+
+  // The bits of the bitmap.
+  scoped_refptr<base::RefCountedMemory> bitmap_data;
+
+  // The pixel dimensions of bitmap_data.
+  gfx::Size pixel_size;
+};
+
 // Abbreviated information about a visit.
 struct BriefVisitInfo {
   URLID url_id;