Avoid sending notifications when the bitmap data in history has not changed

chrome/browser/history/history_backend.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/history/history_backend.h"
 
 #include <algorithm>
 #include <functional>
 #include <list>
 #include <map>
@@ skipping 1912 matching lines @@
   }
 
   std::vector<FaviconBitmapIDSize> bitmap_id_sizes;
   thumbnail_db_->GetFaviconBitmapIDSizes(favicon_id, &bitmap_id_sizes);
 
   // If there is already a favicon bitmap of |pixel_size| at |icon_url|,
   // replace it.
   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;
       break;
     }
   }
 
   // Create a vector of the pixel sizes of the favicon bitmaps currently at
   // |icon_url|.
   std::vector<gfx::Size> favicon_sizes;
@@ skipping 109 matching lines @@
 
   // Build map of FaviconBitmapData for each icon url.
   typedef std::map<GURL, std::vector<FaviconBitmapData> >
       BitmapDataByIconURL;
   BitmapDataByIconURL grouped_by_icon_url;
   for (size_t i = 0; i < favicon_bitmap_data.size(); ++i) {
     const GURL& icon_url = favicon_bitmap_data[i].icon_url;
     grouped_by_icon_url[icon_url].push_back(favicon_bitmap_data[i]);
   }
 
+  // Track whether the method modifies or creates any favicon bitmaps, favicons
+  // or icon mappings.
+  bool data_modified = false;
+
   std::vector<FaviconID> icon_ids;
   for (IconURLSizesMap::const_iterator it = icon_url_sizes.begin();
        it != icon_url_sizes.end(); ++it) {
     const GURL& icon_url = it->first;
     FaviconID icon_id =
         thumbnail_db_->GetFaviconIDForFaviconURL(icon_url, icon_type, NULL);
-    if (icon_id)
-      SetFaviconSizes(icon_id, it->second);
-    else
+    if (icon_id) {
+      bool favicon_bitmaps_deleted = false;
+      SetFaviconSizes(icon_id, it->second, &favicon_bitmaps_deleted);
+      data_modified |= favicon_bitmaps_deleted;
+    } else {
       icon_id = thumbnail_db_->AddFavicon(icon_url, icon_type, it->second);
+      data_modified = true;
+    }
     icon_ids.push_back(icon_id);
 
     BitmapDataByIconURL::iterator grouped_by_icon_url_it =
         grouped_by_icon_url.find(icon_url);
-    if (grouped_by_icon_url_it != grouped_by_icon_url.end())
-      SetFaviconBitmaps(icon_id, grouped_by_icon_url_it->second);
+    if (grouped_by_icon_url_it != grouped_by_icon_url.end()) {
+      if (!data_modified) {
+        bool favicon_bitmaps_changed = false;
+        SetFaviconBitmaps(icon_id,
+                          grouped_by_icon_url_it->second,
+                          &favicon_bitmaps_changed);
+        data_modified |= favicon_bitmaps_changed;
+      } else {
+        SetFaviconBitmaps(icon_id,
+                          grouped_by_icon_url_it->second,
+                          NULL);
+      }
+    }
   }
 
-  SetFaviconMappingsForPageAndRedirects(page_url, icon_type, icon_ids);
+  data_modified |=
+    SetFaviconMappingsForPageAndRedirects(page_url, icon_type, icon_ids);
 
-  // Send notification to the UI as an icon mapping, favicon, or favicon bitmap
-  // almost certainly was changed by this function. The situations where no
-  // data was changed, notably when |favicon_bitmap_data| is empty do not occur
-  // in practice.
-  SendFaviconChangedNotificationForPageAndRedirects(page_url);
+  if (data_modified) {
+    // Send notification to the UI as an icon mapping, favicon, or favicon
+    // bitmap was changed by this function.
+    SendFaviconChangedNotificationForPageAndRedirects(page_url);
+  }
   ScheduleCommit();
 }
 
 void HistoryBackend::SetFaviconsOutOfDateForPage(const GURL& page_url) {
   std::vector<IconMapping> icon_mappings;
 
   if (!thumbnail_db_.get() ||
       !thumbnail_db_->GetIconMappingsForPageURL(page_url,
                                                 &icon_mappings))
     return;
@@ skipping 139 matching lines @@
     }
   }
 
   GetFaviconBitmapResultsForBestMatch(favicon_ids, desired_size_in_dip,
       desired_scale_factors, &results->bitmap_results);
   BuildIconURLSizesMap(favicon_ids, &results->size_map);
 }
 
 void HistoryBackend::SetFaviconBitmaps(
     FaviconID icon_id,
-    const std::vector<FaviconBitmapData>& favicon_bitmap_data) {
+    const std::vector<FaviconBitmapData>& favicon_bitmap_data,
+    bool* favicon_bitmaps_changed) {
+  if (favicon_bitmaps_changed)
+    *favicon_bitmaps_changed = false;
+
   std::vector<FaviconBitmapIDSize> bitmap_id_sizes;
   thumbnail_db_->GetFaviconBitmapIDSizes(icon_id, &bitmap_id_sizes);
 
   // A nested loop is ok because in practice neither |favicon_bitmap_data| nor
   // |bitmap_id_sizes| will have many elements.
   for (size_t i = 0; i < favicon_bitmap_data.size(); ++i) {
     const FaviconBitmapData& bitmap_data_element = favicon_bitmap_data[i];
     FaviconBitmapID bitmap_id = 0;
     for (size_t j = 0; j < bitmap_id_sizes.size(); ++j) {
       if (bitmap_id_sizes[j].pixel_size == bitmap_data_element.pixel_size) {
         bitmap_id = bitmap_id_sizes[j].bitmap_id;
         break;
       }
     }
     if (bitmap_id) {
-      thumbnail_db_->SetFaviconBitmap(bitmap_id,
-          bitmap_data_element.bitmap_data, base::Time::Now());
+      if (favicon_bitmaps_changed &&
+          !*favicon_bitmaps_changed &&
+          IsFaviconBitmapDataEqual(bitmap_id,
+                                   bitmap_data_element.bitmap_data)) {
+        thumbnail_db_->SetFaviconBitmapLastUpdateTime(
+            bitmap_id, base::Time::Now());
+      } else {
+        thumbnail_db_->SetFaviconBitmap(bitmap_id,
+            bitmap_data_element.bitmap_data, base::Time::Now());
+        if (favicon_bitmaps_changed)
+          *favicon_bitmaps_changed = true;
+      }
     } else {
       thumbnail_db_->AddFaviconBitmap(icon_id, bitmap_data_element.bitmap_data,
           base::Time::Now(), bitmap_data_element.pixel_size);
+      if (favicon_bitmaps_changed)
+        *favicon_bitmaps_changed = true;
     }
   }
 }
 
 bool HistoryBackend::ValidateSetFaviconsParams(
     const std::vector<FaviconBitmapData>& favicon_bitmap_data,
     const IconURLSizesMap& icon_url_sizes) const {
   if (icon_url_sizes.size() > kMaxFaviconsPerPage)
     return false;
 
@@ skipping 15 matching lines @@
     const FaviconSizes& favicon_sizes = it->second;
     FaviconSizes::const_iterator it2 = std::find(favicon_sizes.begin(),
         favicon_sizes.end(), favicon_bitmap_data[i].pixel_size);
     if (it2 == favicon_sizes.end())
       return false;
   }
   return true;
 }
 
 void HistoryBackend::SetFaviconSizes(FaviconID icon_id,
-                                     const FaviconSizes& favicon_sizes) {
+                                     const FaviconSizes& favicon_sizes,
+                                     bool* favicon_bitmaps_deleted) {
+  *favicon_bitmaps_deleted = false;
+
   std::vector<FaviconBitmapIDSize> bitmap_id_sizes;
   thumbnail_db_->GetFaviconBitmapIDSizes(icon_id, &bitmap_id_sizes);
 
   // Remove bitmaps whose pixel size is not contained in |favicon_sizes|.
   for (size_t i = 0; i < bitmap_id_sizes.size(); ++i) {
     const gfx::Size& pixel_size = bitmap_id_sizes[i].pixel_size;
     FaviconSizes::const_iterator sizes_it = std::find(favicon_sizes.begin(),
         favicon_sizes.end(), pixel_size);
-    if (sizes_it == favicon_sizes.end())
+    if (sizes_it == favicon_sizes.end()) {
       thumbnail_db_->DeleteFaviconBitmap(bitmap_id_sizes[i].bitmap_id);
+      *favicon_bitmaps_deleted = true;
+    }
   }
 
   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 original_bitmap_data.get() &&

[sky, 2013/01/09 17:55:25]

Should this be on RefCountedMemory? Specfically and Equals method?

+         original_bitmap_data->size() == new_bitmap_data->size() &&
+         (memcmp(new_bitmap_data->front(), original_bitmap_data->front(),
+                 new_bitmap_data->size()) == 0);
+}
+
 bool HistoryBackend::GetFaviconsFromDB(
     const GURL& page_url,
     int icon_types,
     int desired_size_in_dip,
     const std::vector<ui::ScaleFactor>& desired_scale_factors,
     std::vector<FaviconBitmapResult>* favicon_bitmap_results,
     IconURLSizesMap* icon_url_sizes) {
   DCHECK(favicon_bitmap_results);
   DCHECK(icon_url_sizes);
   favicon_bitmap_results->clear();
@@ skipping 648 matching lines @@
   info.url_id = visit.url_id;
   info.time = visit.visit_time;
   info.transition = visit.transition;
   // If we don't have a delegate yet during setup or shutdown, we will drop
   // these notifications.
   if (delegate_.get())
     delegate_->NotifyVisitDBObserversOnAddVisit(info);
 }
 
 }  // namespace history