Avoid sending notifications when the bitmap data in the history backend is replaced with bitmap data which is byte for byte equal.

Bug=168223
Test=HistoryBackendTest.*
     Manual: see comment #7 in bug

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

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

5 files changed, 72 insertions, 4 deletions

diff --git a/chrome/browser/history/history_backend.cc b/chrome/browser/history/history_backend.cc
index 34a0a30..e5a7c14 100644
--- a/chrome/browser/history/history_backend.cc
+++ b/chrome/browser/history/history_backend.cc
@@ -1931,6 +1931,13 @@ void HistoryBackend::MergeFavicon(
   bool replaced_bitmap = false;
   for (size_t i = 0; i < bitmap_id_sizes.size(); ++i) {
     if (bitmap_id_sizes[i].pixel_size == pixel_size) {
+      if (IsFaviconBitmapDataEqual(bitmap_id_sizes[i].bitmap_id, bitmap_data)) {
+        thumbnail_db_->SetFaviconBitmapLastUpdateTime(
+            bitmap_id_sizes[i].bitmap_id, base::Time::Now());
+        // Return early as merging did not alter the bitmap data.
+        ScheduleCommit();
+        return;
+      }
       thumbnail_db_->SetFaviconBitmap(bitmap_id_sizes[i].bitmap_id, bitmap_data,
           base::Time::Now());
       replaced_bitmap = true;
@@ -2316,6 +2323,20 @@ void HistoryBackend::SetFaviconSizes(FaviconID icon_id,
   thumbnail_db_->SetFaviconSizes(icon_id, favicon_sizes);
 }
 
+bool HistoryBackend::IsFaviconBitmapDataEqual(
+    FaviconBitmapID bitmap_id,
+    const scoped_refptr<base::RefCountedMemory>& new_bitmap_data) {
+  if (!new_bitmap_data.get())
+    return false;
+
+  scoped_refptr<base::RefCountedMemory> original_bitmap_data;
+  thumbnail_db_->GetFaviconBitmap(bitmap_id,
+                                  NULL,
+                                  &original_bitmap_data,
+                                  NULL);
+  return new_bitmap_data->Equals(original_bitmap_data);
+}
+
 bool HistoryBackend::GetFaviconsFromDB(
     const GURL& page_url,
     int icon_types,
diff --git a/chrome/browser/history/history_backend.h b/chrome/browser/history/history_backend.h
index 7d3c507..bab856c 100644
--- a/chrome/browser/history/history_backend.h
+++ b/chrome/browser/history/history_backend.h
@@ -704,6 +704,11 @@ class HistoryBackend : public base::RefCountedThreadSafe<HistoryBackend>,
   void SetFaviconSizes(FaviconID icon_id,
                        const FaviconSizes& favicon_sizes);
 
+  // Returns true if the bitmap data at |bitmap_id| equals |new_bitmap_data|.
+  bool IsFaviconBitmapDataEqual(
+      FaviconBitmapID bitmap_id,
+      const scoped_refptr<base::RefCountedMemory>& new_bitmap_data);
+
   // Returns true if there are favicons for |page_url| and one of the types in
   // |icon_types|.
   // |favicon_bitmap_results| is set to the favicon bitmaps which most closely
diff --git a/chrome/browser/history/history_backend_unittest.cc b/chrome/browser/history/history_backend_unittest.cc
index bc2e14a..bb57dd0 100644
--- a/chrome/browser/history/history_backend_unittest.cc
+++ b/chrome/browser/history/history_backend_unittest.cc
@@ -1792,13 +1792,39 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) {
   EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmap.bitmap_data));
   EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size);
 
-  // 1) Merge favicon of the same size.
+  EXPECT_EQ(1, num_broadcasted_notifications());
+
+  // 1) Merge identical favicon bitmap.
   std::vector<unsigned char> data;
-  data.push_back('b');
+  data.push_back('a');
   scoped_refptr<base::RefCountedBytes> bitmap_data(
       new base::RefCountedBytes(data));
   backend_->MergeFavicon(page_url, icon_url1, FAVICON, bitmap_data, kSmallSize);
 
+  // All the data should stay the same and no notifications should have been
+  // sent.
+  icon_mappings.clear();
+  EXPECT_TRUE(backend_->thumbnail_db_->GetIconMappingsForPageURL(page_url,
+      &icon_mappings));
+  EXPECT_EQ(1u, icon_mappings.size());
+  EXPECT_EQ(icon_url1, icon_mappings[0].icon_url);
+
+  favicon_sizes.clear();
+  EXPECT_TRUE(backend_->thumbnail_db_->GetFaviconHeader(
+      icon_mappings[0].icon_id, NULL, NULL, &favicon_sizes));
+  EXPECT_EQ(GetSizesSmallAndLarge(), favicon_sizes);
+
+  EXPECT_TRUE(GetOnlyFaviconBitmap(icon_mappings[0].icon_id, &favicon_bitmap));
+  EXPECT_TRUE(BitmapDataEqual('a', favicon_bitmap.bitmap_data));
+  EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size);
+
+  EXPECT_EQ(1, num_broadcasted_notifications());
+
+  // 2) Merge favicon bitmap of the same size.
+  data[0] = 'b';
+  bitmap_data = new base::RefCountedBytes(data);
+  backend_->MergeFavicon(page_url, icon_url1, FAVICON, bitmap_data, kSmallSize);
+
   // The small favicon bitmap at |icon_url1| should be overwritten and favicon
   // sizes should remain unchanged.
   icon_mappings.clear();
@@ -1816,7 +1842,7 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) {
   EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmap.bitmap_data));
   EXPECT_EQ(kSmallSize, favicon_bitmap.pixel_size);
 
-  // 2) Merge favicon for the same icon URL, but a pixel size for which there is
+  // 3) Merge favicon for the same icon URL, but a pixel size for which there is
   // no favicon bitmap.
   data[0] = 'c';
   bitmap_data = new base::RefCountedBytes(data);
@@ -1843,7 +1869,7 @@ TEST_F(HistoryBackendTest, MergeFaviconPageURLInDB) {
   EXPECT_TRUE(BitmapDataEqual('b', favicon_bitmaps[1].bitmap_data));
   EXPECT_EQ(kSmallSize, favicon_bitmaps[1].pixel_size);
 
-  // 3) Merge favicon for an icon URL different from the icon URLs already
+  // 4) Merge favicon for an icon URL different from the icon URLs already
   // mapped to page URL.
   data[0] = 'd';
   bitmap_data = new base::RefCountedBytes(data);
diff --git a/chrome/browser/history/thumbnail_database.cc b/chrome/browser/history/thumbnail_database.cc
index ae7c82e..a5dde8c 100644
--- a/chrome/browser/history/thumbnail_database.cc
+++ b/chrome/browser/history/thumbnail_database.cc
@@ -597,6 +597,17 @@ bool ThumbnailDatabase::SetFaviconBitmap(
   return statement.Run();
 }
 
+bool ThumbnailDatabase::SetFaviconBitmapLastUpdateTime(
+    FaviconBitmapID bitmap_id,
+    base::Time time) {
+  DCHECK(bitmap_id);
+  sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
+      "UPDATE favicon_bitmaps SET last_updated=? WHERE id=?"));
+  statement.BindInt64(0, time.ToInternalValue());
+  statement.BindInt64(1, bitmap_id);
+  return statement.Run();
+}
+
 bool ThumbnailDatabase::DeleteFaviconBitmapsForFavicon(FaviconID icon_id) {
   sql::Statement statement(db_.GetCachedStatement(SQL_FROM_HERE,
       "DELETE FROM favicon_bitmaps WHERE icon_id=?"));
diff --git a/chrome/browser/history/thumbnail_database.h b/chrome/browser/history/thumbnail_database.h
index dffdc90..9ee17c7 100644
--- a/chrome/browser/history/thumbnail_database.h
+++ b/chrome/browser/history/thumbnail_database.h
@@ -142,6 +142,11 @@ class ThumbnailDatabase {
                         scoped_refptr<base::RefCountedMemory> bitmap_data,
                         base::Time time);
 
+  // Sets the last updated time for the favicon bitmap at |bitmap_id|.
+  // Returns true if successful.
+  bool SetFaviconBitmapLastUpdateTime(FaviconBitmapID bitmap_id,
+                                      base::Time time);
+
   // Deletes the favicon bitmaps for the favicon with with |icon_id|.
   // Returns true if successful.
   bool DeleteFaviconBitmapsForFavicon(FaviconID icon_id);