[Sync] Keep synced tab favicons in memory instead of in the FaviconService.

Due to the favicon service, history backend, and thumbnail db not being
prepared to expire favicons added via sync, we now keep synced tab favicons
in memory within the SessionModelAssocaitor. We do simple deduping based
on the favicon's url, and provide a GetSyncedFaviconForPageURL method that
fills a string with the bytes of a png-encoded favicon for the url.

BUG=122890
TEST=unit_tests --gtest_filter="*SessionModelAssociator*"


Review URL: http://codereview.chromium.org/10071028

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

3 files changed, 368 insertions, 34 deletions

diff --git a/chrome/browser/sync/glue/session_model_associator.cc b/chrome/browser/sync/glue/session_model_associator.cc
index fdcaac8..856b46b 100644
--- a/chrome/browser/sync/glue/session_model_associator.cc
+++ b/chrome/browser/sync/glue/session_model_associator.cc
@@ -716,6 +716,8 @@ SyncError SessionModelAssociator::DisassociateModels() {
   current_machine_tag_ = "";
   current_session_name_ = "";
   load_consumer_.CancelAllRequests();
+  synced_favicons_.clear();
+  synced_favicon_pages_.clear();
 
   // There is no local model stored with which to disassociate, just notify
   // foreign session handlers.
@@ -757,6 +759,21 @@ void SessionModelAssociator::OnSessionNameInitialized(
     current_session_name_ = name;
 }
 
+bool SessionModelAssociator::GetSyncedFaviconForPageURL(
+    const std::string& url,
+    std::string* png_favicon) const {
+  std::map<std::string, std::string>::const_iterator iter =
+      synced_favicon_pages_.find(url);
+  if (iter == synced_favicon_pages_.end())
+    return false;
+  DCHECK(synced_favicons_.find(iter->second) != synced_favicons_.end());
+  const std::string& favicon =
+      synced_favicons_.find(iter->second)->second->data;
+  png_favicon->assign(favicon);
+  DCHECK_GT(favicon.size(), 0U);
+  return true;
+}
+
 void SessionModelAssociator::InitializeCurrentSessionName() {
   DCHECK(CalledOnValidThread());
   if (setup_for_test_) {
@@ -877,8 +894,34 @@ void SessionModelAssociator::AssociateForeignSpecifics(
     SessionID::id_type tab_id = tab_s.tab_id();
     SessionTab* tab =
         synced_session_tracker_.GetTab(foreign_session_tag, tab_id);
+
+    // Figure out what the previous url for this tab was (may be empty string
+    // if this is a new tab).
+    std::string previous_url;
+    if (tab->navigations.size() > 0) {
+      int selected_index = tab->current_navigation_index;
+      selected_index = std::max(
+          0,
+          std::min(selected_index,
+                   static_cast<int>(tab->navigations.size() - 1)));
+      if (tab->navigations[selected_index].virtual_url().is_valid())
+        previous_url = tab->navigations[selected_index].virtual_url().spec();
+    }
+
+    // Update SessionTab based on protobuf.
     PopulateSessionTabFromSpecifics(tab_s, modification_time, tab);
+
+    // Loads the tab favicon, increments the usage counter, and updates
+    // synced_favicon_pages_.
     LoadForeignTabFavicon(tab_s);
+
+    // Now check to see if the favicon associated with the previous url is no
+    // longer in use. This will have no effect if the current url matches the
+    // previous url (LoadForeignTabFavicon increments, this decrements, no net
+    // change in usage), or if the previous_url was not set (new tab).
+    DecrementAndCleanFaviconForURL(previous_url);
+
+    // Update the last modified time.
     if (foreign_session->modified_time < modification_time)
       foreign_session->modified_time = modification_time;
   } else {
@@ -887,6 +930,38 @@ void SessionModelAssociator::AssociateForeignSpecifics(
   }
 }
 
+void SessionModelAssociator::DecrementAndCleanFaviconForURL(
+    const std::string& page_url) {
+  if (page_url.empty())
+    return;
+  std::map<std::string, std::string>::const_iterator iter =
+      synced_favicon_pages_.find(page_url);
+  if (iter != synced_favicon_pages_.end()) {
+    const std::string& favicon_url = iter->second;
+    DCHECK_GT(synced_favicons_[favicon_url]->usage_count, 0);
+    --(synced_favicons_[favicon_url]->usage_count);
+    if (synced_favicons_[favicon_url]->usage_count <= 0) {
+      // No more tabs using this favicon. Erase it.
+      synced_favicons_.erase(favicon_url);
+      // Erase the page mappings to the favicon url. We iterate through all
+      // page urls in case multiple pages share the same favicon.
+      std::map<std::string, std::string>::iterator page_iter;
+      for (page_iter = synced_favicon_pages_.begin();
+           page_iter != synced_favicon_pages_.end();) {
+        std::map<std::string, std::string>::iterator to_delete = page_iter;
+        ++page_iter;
+        if (to_delete->second == favicon_url) {
+          synced_favicon_pages_.erase(to_delete);
+        }
+      }
+    }
+  }
+}
+
+size_t SessionModelAssociator::NumFaviconsForTesting() const {
+  return synced_favicons_.size();
+}
+
 bool SessionModelAssociator::DisassociateForeignSession(
     const std::string& foreign_session_tag) {
   DCHECK(CalledOnValidThread());
@@ -1084,39 +1159,38 @@ void SessionModelAssociator::LoadForeignTabFavicon(
     const sync_pb::SessionTab& tab) {
   if (!tab.has_favicon() || tab.favicon().empty())
     return;
-  if (tab.favicon_type() != sync_pb::SessionTab::TYPE_WEB_FAVICON) {
+  if (!tab.has_favicon_type() ||
+      tab.favicon_type() != sync_pb::SessionTab::TYPE_WEB_FAVICON) {
     DVLOG(1) << "Ignoring non-web favicon.";
     return;
   }
-  int current_navigation_index = tab.current_navigation_index();
-  if (current_navigation_index < 0 ||
-      current_navigation_index >= tab.navigation_size()) {
+  if (tab.navigation_size() == 0)
     return;
-  }
-  GURL navigation_url(tab.navigation(current_navigation_index).virtual_url());
+  int selected_index = tab.current_navigation_index();
+  selected_index = std::max(
+      0,
+      std::min(selected_index,
+               static_cast<int>(tab.navigation_size() - 1)));
+  GURL navigation_url(tab.navigation(selected_index).virtual_url());
   if (!navigation_url.is_valid())
     return;
   GURL favicon_source(tab.favicon_source());
   if (!favicon_source.is_valid())
     return;
 
-  HistoryService* history =
-      profile_->GetHistoryService(Profile::EXPLICIT_ACCESS);
-  FaviconService* favicon_service =
-      profile_->GetFaviconService(Profile::EXPLICIT_ACCESS);
-  if (!history || !favicon_service)
-    return;
-  std::vector<unsigned char> icon_bytes;
   const std::string& favicon = tab.favicon();
-  icon_bytes.assign(reinterpret_cast<const unsigned char*>(favicon.data()),
-                    reinterpret_cast<const unsigned char*>(favicon.data() +
-                                                           favicon.length()));
   DVLOG(1) << "Storing synced favicon for url " << navigation_url.spec()
-           << " with size " << icon_bytes.size() << " bytes.";
-  favicon_service->SetFavicon(navigation_url,
-                              favicon_source,
-                              icon_bytes,
-                              history::FAVICON);
+           << " with size " << favicon.size() << " bytes.";
+  std::map<std::string, linked_ptr<SyncedFaviconInfo> >::iterator favicon_iter;
+  favicon_iter = synced_favicons_.find(favicon_source.spec());
+  if (favicon_iter == synced_favicons_.end()) {
+    synced_favicons_[favicon_source.spec()] =
+        make_linked_ptr<SyncedFaviconInfo>(new SyncedFaviconInfo(favicon));
+  } else {
+    favicon_iter->second->data = favicon;
+    ++favicon_iter->second->usage_count;
+  }
+  synced_favicon_pages_[navigation_url.spec()] = favicon_source.spec();
 }
 
 bool SessionModelAssociator::UpdateSyncModelDataFromClient(SyncError* error) {
diff --git a/chrome/browser/sync/glue/session_model_associator.h b/chrome/browser/sync/glue/session_model_associator.h
index d6029dd..50d86b1 100644
--- a/chrome/browser/sync/glue/session_model_associator.h
+++ b/chrome/browser/sync/glue/session_model_associator.h
@@ -224,7 +224,13 @@ class SessionModelAssociator
   // Callback for when the session name has been computed.
   void OnSessionNameInitialized(const std::string& name);
 
+  // If a valid favicon for the page at |url| is found, fills |png_favicon| with
+  // the png-encoded image and returns true. Else, returns false.
+  bool GetSyncedFaviconForPageURL(const std::string& url,
+                                  std::string* png_favicon) const;
+
  private:
+  friend class SyncSessionModelAssociatorTest;
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, WriteSessionToNode);
   FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest,
                            WriteFilledSessionToNode);
@@ -400,6 +406,11 @@ class SessionModelAssociator
   bool WriteTabContentsToSyncModel(TabLink* tab_link,
                                    SyncError* error);
 
+  // Decrements the favicon usage counters for the favicon used by |page_url|.
+  // Deletes the favicon and associated pages from the favicon usage maps
+  // if no page is found to be referring to the favicon anymore.
+  void DecrementAndCleanFaviconForURL(const std::string& page_url);
+
   // Load the favicon for the tab specified by |tab_link|. Will cancel any
   // outstanding request for this tab. OnFaviconDataAvailable(..) will be called
   // when the load completes.
@@ -431,8 +442,9 @@ class SessionModelAssociator
                                               const base::Time& mtime,
                                               SessionTab* session_tab);
 
-  // Helper method to take the favicon data in a foreign tab and store it
-  // into the history db.
+  // Helper method to load the favicon data from the tab specifics. If the
+  // favicon is valid, stores the favicon data and increments the usage counter
+  // in |synced_favicons_| and updates |synced_favicon_pages_| appropriately.
   void LoadForeignTabFavicon(const sync_pb::SessionTab& tab);
 
   // Used to populate a session tab from the session specifics tab provided.
@@ -456,6 +468,9 @@ class SessionModelAssociator
   bool TabHasValidEntry(const SyncedTabDelegate& tab) const;
 
   // For testing only.
+  size_t NumFaviconsForTesting() const;
+
+  // For testing only.
   void QuitLoopForSubtleTesting();
 
   // Unique client tag.
@@ -503,6 +518,32 @@ class SessionModelAssociator
   // SessionID for the tab whose favicon is being set.
   CancelableRequestConsumerTSimple<SessionID::id_type> load_consumer_;
 
+  // Synced favicon storage and tracking.
+  // Map of favicon URL -> favicon info for favicons synced from other clients.
+  // TODO(zea): if this becomes expensive memory-wise, reconsider using the
+  // favicon service instead. For now, this is simpler due to the history
+  // backend not properly supporting expiration of synced favicons.
+  // See crbug.com/122890.
+  struct SyncedFaviconInfo {
+    SyncedFaviconInfo() : usage_count(0) {}
+    explicit SyncedFaviconInfo(const std::string& data)
+        : data(data),
+          usage_count(1) {}
+    SyncedFaviconInfo(const std::string& data, int usage_count)
+        : data(data),
+          usage_count(usage_count) {}
+    // The actual favicon data, stored in png encoded bytes.
+    std::string data;
+    // The number of foreign tabs using this favicon.
+    int usage_count;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(SyncedFaviconInfo);
+  };
+  std::map<std::string, linked_ptr<SyncedFaviconInfo> > synced_favicons_;
+  // Map of page URL -> favicon url.
+  std::map<std::string, std::string> synced_favicon_pages_;
+
   DISALLOW_COPY_AND_ASSIGN(SessionModelAssociator);
 };
 
diff --git a/chrome/browser/sync/glue/session_model_associator_unittest.cc b/chrome/browser/sync/glue/session_model_associator_unittest.cc
index b1cac4a..1d14827 100644
--- a/chrome/browser/sync/glue/session_model_associator_unittest.cc
+++ b/chrome/browser/sync/glue/session_model_associator_unittest.cc
@@ -34,12 +34,24 @@ class SyncSessionModelAssociatorTest : public testing::Test {
  public:
   SyncSessionModelAssociatorTest()
       : ui_thread_(BrowserThread::UI, &message_loop_),
-        sync_service_(&profile_) {}
-  virtual void SetUp() OVERRIDE {
-    model_associator_.reset(new SessionModelAssociator(&sync_service_, true));
+        sync_service_(&profile_),
+        model_associator_(&sync_service_, true) {}
+
+  // Helper methods to avoid having to friend individual tests.
+  bool GetFavicon(std::string page_url, std::string* favicon) {
+    return model_associator_.GetSyncedFaviconForPageURL(page_url, favicon);
+  }
+
+  void LoadTabFavicon(const sync_pb::SessionTab& tab) {
+    model_associator_.LoadForeignTabFavicon(tab);
+  }
+
+  size_t NumFavicons() {
+    return model_associator_.NumFaviconsForTesting();
   }
-  virtual void TearDown() OVERRIDE {
-    model_associator_.reset();
+
+  void DecrementFavicon(std::string url) {
+    model_associator_.DecrementAndCleanFaviconForURL(url);
   }
 
  protected:
@@ -47,7 +59,7 @@ class SyncSessionModelAssociatorTest : public testing::Test {
   content::TestBrowserThread ui_thread_;
   NiceMock<ProfileMock> profile_;
   NiceMock<ProfileSyncServiceMock> sync_service_;
-  scoped_ptr<SessionModelAssociator> model_associator_;
+  SessionModelAssociator model_associator_;
 };
 
 TEST_F(SyncSessionModelAssociatorTest, SessionWindowHasNoTabsToSync) {
@@ -263,7 +275,7 @@ TEST_F(SyncSessionModelAssociatorTest, TriggerSessionRefresh) {
   SyncRefreshListener refresh_listener;
 
   EXPECT_FALSE(refresh_listener.notified_of_refresh());
-  model_associator_->AttemptSessionsDataRefresh();
+  model_associator_.AttemptSessionsDataRefresh();
   EXPECT_TRUE(refresh_listener.notified_of_refresh());
 }
 
@@ -278,7 +290,7 @@ TEST_F(SyncSessionModelAssociatorTest, ValidTabs) {
       Return((content::NavigationEntry *)NULL));
   EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(1));
   EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1));
-  EXPECT_FALSE(model_associator_->ShouldSyncTab(tab_mock));
+  EXPECT_FALSE(model_associator_.ShouldSyncTab(tab_mock));
 
   // A chrome:// entry isn't valid.
   scoped_ptr<content::NavigationEntry> entry(
@@ -289,7 +301,7 @@ TEST_F(SyncSessionModelAssociatorTest, ValidTabs) {
   EXPECT_CALL(tab_mock, GetEntryAtIndex(0)).WillRepeatedly(Return(entry.get()));
   EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(1));
   EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1));
-  EXPECT_FALSE(model_associator_->ShouldSyncTab(tab_mock));
+  EXPECT_FALSE(model_associator_.ShouldSyncTab(tab_mock));
 
   // A file:// entry isn't valid, even in addition to another entry.
   scoped_ptr<content::NavigationEntry> entry2(
@@ -302,7 +314,7 @@ TEST_F(SyncSessionModelAssociatorTest, ValidTabs) {
       Return(entry2.get()));
   EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(2));
   EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1));
-  EXPECT_FALSE(model_associator_->ShouldSyncTab(tab_mock));
+  EXPECT_FALSE(model_associator_.ShouldSyncTab(tab_mock));
 
   // Add a valid scheme entry to tab, making the tab valid.
   scoped_ptr<content::NavigationEntry> entry3(
@@ -318,7 +330,214 @@ TEST_F(SyncSessionModelAssociatorTest, ValidTabs) {
       Return(entry3.get()));
   EXPECT_CALL(tab_mock, GetEntryCount()).WillRepeatedly(Return(3));
   EXPECT_CALL(tab_mock, GetPendingEntryIndex()).WillRepeatedly(Return(-1));
-  EXPECT_TRUE(model_associator_->ShouldSyncTab(tab_mock));
+  EXPECT_TRUE(model_associator_.ShouldSyncTab(tab_mock));
+}
+
+// Create tab specifics with an empty favicon. Ensure it gets ignored and not
+// stored into the synced favicon lookups.
+TEST_F(SyncSessionModelAssociatorTest, LoadEmptyFavicon) {
+  std::string favicon = "";
+  std::string favicon_url = "http://www.faviconurl.com/favicon.ico";
+  std::string page_url = "http://www.faviconurl.com/page.html";
+  sync_pb::SessionTab tab;
+  tab.set_favicon(favicon);
+  tab.set_favicon_source(favicon_url);
+  tab.set_favicon_type(sync_pb::SessionTab::TYPE_WEB_FAVICON);
+  sync_pb::TabNavigation* navigation = tab.add_navigation();
+  navigation->set_index(0);
+  navigation->set_virtual_url(page_url);
+  tab.set_current_navigation_index(0);
+
+  std::string synced_favicon;
+  EXPECT_FALSE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  LoadTabFavicon(tab);
+  EXPECT_FALSE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+}
+
+// Create tab specifics with a non-web favicon. Ensure it gets ignored and not
+// stored into the synced favicon lookups.
+TEST_F(SyncSessionModelAssociatorTest, LoadNonWebFavicon) {
+  std::string favicon = "these are icon synced_favicon";
+  std::string favicon_url = "http://www.faviconurl.com/favicon.ico";
+  std::string page_url = "http://www.faviconurl.com/page.html";
+  sync_pb::SessionTab tab;
+  tab.set_favicon(favicon);
+  tab.set_favicon_source(favicon_url);
+  // Set favicon type to an unsupported value (1 == WEB_FAVICON).
+   tab.mutable_unknown_fields()->AddVarint(9, 2);
+  sync_pb::TabNavigation* navigation = tab.add_navigation();
+  navigation->set_index(0);
+  navigation->set_virtual_url(page_url);
+  tab.set_current_navigation_index(0);
+
+  std::string synced_favicon;
+  EXPECT_FALSE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  LoadTabFavicon(tab);
+  EXPECT_FALSE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+}
+
+// Create tab specifics with a valid favicon. Ensure it gets stored in the
+// synced favicon lookups and is accessible by the page url.
+TEST_F(SyncSessionModelAssociatorTest, LoadValidFavicon) {
+  std::string favicon = "these are icon synced_favicon";
+  std::string favicon_url = "http://www.faviconurl.com/favicon.ico";
+  std::string page_url = "http://www.faviconurl.com/page.html";
+  sync_pb::SessionTab tab;
+  tab.set_favicon(favicon);
+  tab.set_favicon_source(favicon_url);
+  tab.set_favicon_type(sync_pb::SessionTab::TYPE_WEB_FAVICON);
+  sync_pb::TabNavigation* navigation = tab.add_navigation();
+  navigation->set_index(0);
+  navigation->set_virtual_url(page_url);
+  tab.set_current_navigation_index(0);
+
+  std::string synced_favicon;
+  EXPECT_FALSE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  LoadTabFavicon(tab);
+  EXPECT_TRUE(GetFavicon(page_url, &synced_favicon));
+  ASSERT_FALSE(synced_favicon.empty());
+  EXPECT_EQ(favicon, synced_favicon);
+}
+
+// Create tab specifics with a valid favicon, load it, then load tab specifics
+// with a new favicon for the same favicon source but different page. Ensure the
+// new favicon overwrites the old favicon for both page urls.
+TEST_F(SyncSessionModelAssociatorTest, UpdateValidFavicon) {
+  std::string favicon_url = "http://www.faviconurl.com/favicon.ico";
+
+  std::string favicon = "these are icon synced_favicon";
+  std::string page_url = "http://www.faviconurl.com/page.html";
+  sync_pb::SessionTab tab;
+  tab.set_favicon(favicon);
+  tab.set_favicon_source(favicon_url);
+  tab.set_favicon_type(sync_pb::SessionTab::TYPE_WEB_FAVICON);
+  sync_pb::TabNavigation* navigation = tab.add_navigation();
+  navigation->set_index(0);
+  navigation->set_virtual_url(page_url);
+  tab.set_current_navigation_index(0);
+
+  std::string synced_favicon;
+  EXPECT_FALSE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  LoadTabFavicon(tab);
+  EXPECT_TRUE(GetFavicon(page_url, &synced_favicon));
+  ASSERT_FALSE(synced_favicon.empty());
+  EXPECT_EQ(favicon, synced_favicon);
+
+  // Now have a new page with same favicon source but newer favicon data.
+  std::string favicon2 = "these are new icon synced_favicon";
+  std::string page_url2 = "http://www.faviconurl.com/page2.html";
+  sync_pb::SessionTab tab2;
+  tab2.set_favicon(favicon2);
+  tab2.set_favicon_source(favicon_url);
+  tab2.set_favicon_type(sync_pb::SessionTab::TYPE_WEB_FAVICON);
+  sync_pb::TabNavigation* navigation2 = tab2.add_navigation();
+  navigation2->set_index(0);
+  navigation2->set_virtual_url(page_url2);
+  tab2.set_current_navigation_index(0);
+
+  // Verify the favicons for both pages match the newest favicon.
+  synced_favicon.clear();
+  EXPECT_FALSE(GetFavicon(page_url2, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  LoadTabFavicon(tab2);
+  EXPECT_TRUE(GetFavicon(page_url2, &synced_favicon));
+  ASSERT_FALSE(synced_favicon.empty());
+  EXPECT_EQ(favicon2, synced_favicon);
+  EXPECT_NE(favicon, synced_favicon);
+  synced_favicon.clear();
+  EXPECT_TRUE(GetFavicon(page_url, &synced_favicon));
+  ASSERT_FALSE(synced_favicon.empty());
+  EXPECT_EQ(favicon2, synced_favicon);
+  EXPECT_NE(favicon, synced_favicon);
+}
+
+// Ensure that favicon cleanup cleans up favicons no longer being used and
+// doesn't touch those favicons still in use.
+TEST_F(SyncSessionModelAssociatorTest, FaviconCleanup) {
+  EXPECT_EQ(NumFavicons(), 0U);
+
+  std::string double_favicon = "these are icon synced_favicon";
+  std::string double_favicon_url = "http://www.faviconurl.com/favicon.ico";
+  std::string page_url = "http://www.faviconurl.com/page.html";
+  sync_pb::SessionTab tab;
+  tab.set_favicon(double_favicon);
+  tab.set_favicon_source(double_favicon_url);
+  tab.set_favicon_type(sync_pb::SessionTab::TYPE_WEB_FAVICON);
+  sync_pb::TabNavigation* navigation = tab.add_navigation();
+  navigation->set_index(0);
+  navigation->set_virtual_url(page_url);
+  tab.set_current_navigation_index(0);
+  LoadTabFavicon(tab);
+  EXPECT_EQ(1U, NumFavicons());
+
+  // Add another page using the first favicon.
+  std::string page_url2 = "http://www.faviconurl.com/page2.html";
+  tab.mutable_navigation(0)->set_virtual_url(page_url2);
+  LoadTabFavicon(tab);
+  EXPECT_EQ(1U, NumFavicons());
+
+  // Add a favicon with a single user.
+  std::string single_favicon = "different favicon synced_favicon";
+  std::string single_favicon_url = "http://www.single_favicon_page.com/x.ico";
+  std::string single_favicon_page = "http://www.single_favicon_page.com/x.html";
+  tab.set_favicon(single_favicon);
+  tab.set_favicon_source(single_favicon_url);
+  tab.mutable_navigation(0)->set_virtual_url(single_favicon_page);
+  LoadTabFavicon(tab);
+  EXPECT_EQ(2U, NumFavicons());
+
+  // Decrementing a favicon used by one page should remove it.
+  std::string synced_favicon;
+  EXPECT_TRUE(GetFavicon(single_favicon_page, &synced_favicon));
+  EXPECT_EQ(synced_favicon, single_favicon);
+  DecrementFavicon(single_favicon_page);
+  synced_favicon.clear();
+  EXPECT_FALSE(GetFavicon(single_favicon_page, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  EXPECT_EQ(1U, NumFavicons());
+
+  // Decrementing a favicon used by two pages shouldn't remove it.
+  synced_favicon.clear();
+  EXPECT_TRUE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_EQ(synced_favicon, double_favicon);
+  synced_favicon.clear();
+  EXPECT_TRUE(GetFavicon(page_url2, &synced_favicon));
+  EXPECT_EQ(synced_favicon, double_favicon);
+  DecrementFavicon(page_url);
+  EXPECT_EQ(1U, NumFavicons());
+  synced_favicon.clear();
+  EXPECT_TRUE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_EQ(synced_favicon, double_favicon);
+  synced_favicon.clear();
+  EXPECT_TRUE(GetFavicon(page_url2, &synced_favicon));
+  EXPECT_EQ(synced_favicon, double_favicon);
+  EXPECT_EQ(1U, NumFavicons());
+
+  // Attempting to decrement a page that's already removed should do nothing.
+  DecrementFavicon(single_favicon_page);
+  EXPECT_EQ(1U, NumFavicons());
+
+  // Attempting to decrement an empty url should do nothing.
+  DecrementFavicon("");
+  EXPECT_EQ(1U, NumFavicons());
+
+  // Decrementing the second and only remaining page should remove the favicon.
+  // Both pages that referred to it should now fail to look up their favicon.
+  DecrementFavicon(page_url2);
+  synced_favicon.clear();
+  EXPECT_FALSE(GetFavicon(page_url, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  EXPECT_EQ(0U, NumFavicons());
+  synced_favicon.clear();
+  EXPECT_FALSE(GetFavicon(page_url2, &synced_favicon));
+  EXPECT_TRUE(synced_favicon.empty());
+  EXPECT_EQ(0U, NumFavicons());
 }
 
 }  // namespace browser_sync