summaryrefslogtreecommitdiffstats
path: root/chrome/browser/sync/glue
diff options
context:
space:
mode:
authorzea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-19 20:09:24 +0000
committerzea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-03-19 20:09:24 +0000
commit38f2b0095999ba43e7b5d175a3d66ca65b6a1c1f (patch)
tree20b962344b51f9180ebe38f14eb7a5a8beac5a64 /chrome/browser/sync/glue
parentab5f4a9ec6d1a0d2d4b42a489c6537e48eb2ce83 (diff)
downloadchromium_src-38f2b0095999ba43e7b5d175a3d66ca65b6a1c1f.zip
chromium_src-38f2b0095999ba43e7b5d175a3d66ca65b6a1c1f.tar.gz
chromium_src-38f2b0095999ba43e7b5d175a3d66ca65b6a1c1f.tar.bz2
[Sync] Plumb local favicon deletions to sync favicons datatype
If the user deletes all the local visits to a url, triggering a favicon expiration, we now delete the favicon in sync too. Similarly, if the user clears all their browsing history, we delete all synced favicons. This involves tracking which favicon icon urls are expired during history deletes and including that information in the URLsDeletedDetails. BUG=154886 Review URL: https://codereview.chromium.org/12779025 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@189076 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync/glue')
-rw-r--r--chrome/browser/sync/glue/favicon_cache.cc117
-rw-r--r--chrome/browser/sync/glue/favicon_cache.h28
-rw-r--r--chrome/browser/sync/glue/favicon_cache_unittest.cc126
3 files changed, 245 insertions, 26 deletions
diff --git a/chrome/browser/sync/glue/favicon_cache.cc b/chrome/browser/sync/glue/favicon_cache.cc
index 244bd54..a1dc8fa 100644
--- a/chrome/browser/sync/glue/favicon_cache.cc
+++ b/chrome/browser/sync/glue/favicon_cache.cc
@@ -7,7 +7,11 @@
#include "base/metrics/histogram.h"
#include "chrome/browser/favicon/favicon_service.h"
#include "chrome/browser/favicon/favicon_service_factory.h"
+#include "chrome/browser/history/history_notifications.h"
#include "chrome/browser/history/history_types.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "content/public/browser/notification_details.h"
+#include "content/public/browser/notification_source.h"
#include "sync/api/time.h"
#include "sync/protocol/favicon_image_specifics.pb.h"
#include "sync/protocol/favicon_tracking_specifics.pb.h"
@@ -190,7 +194,11 @@ FaviconCache::FaviconCache(Profile* profile, int max_sync_favicon_limit)
: profile_(profile),
weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)),
legacy_delegate_(NULL),
- max_sync_favicon_limit_(max_sync_favicon_limit) {}
+ max_sync_favicon_limit_(max_sync_favicon_limit) {
+ notification_registrar_.Add(this,
+ chrome::NOTIFICATION_HISTORY_URLS_DELETED,
+ content::Source<Profile>(profile_));
+}
FaviconCache::~FaviconCache() {}
@@ -248,8 +256,7 @@ syncer::SyncMergeResult FaviconCache::MergeDataAndStartSyncing(
FaviconMap::iterator favicon_iter = synced_favicons_.find(*iter);
DVLOG(1) << "Dropping local favicon "
<< favicon_iter->second->favicon_url.spec();
- recent_favicons_.erase(favicon_iter->second);
- synced_favicons_.erase(favicon_iter);
+ DropSyncedFavicon(favicon_iter);
merge_result.set_num_items_deleted(merge_result.num_items_deleted() + 1);
}
}
@@ -316,8 +323,7 @@ syncer::SyncError FaviconCache::ProcessSyncChanges(
continue;
} else {
DVLOG(1) << "Deleting favicon at " << favicon_url.spec();
- recent_favicons_.erase(favicon_iter->second);
- synced_favicons_.erase(favicon_iter);
+ DropSyncedFavicon(favicon_iter);
// TODO(zea): it's possible that we'll receive a deletion for an image,
// but not a tracking data, or vice versa, resulting in an orphan
// favicon node in one of the types. We should track how often this
@@ -489,6 +495,41 @@ void FaviconCache::RemoveLegacyDelegate() {
legacy_delegate_ = NULL;
}
+void FaviconCache::Observe(int type,
+ const content::NotificationSource& source,
+ const content::NotificationDetails& details) {
+ DCHECK_EQ(type, chrome::NOTIFICATION_HISTORY_URLS_DELETED);
+
+ content::Details<history::URLsDeletedDetails> deleted_details(details);
+
+ // We only care about actual user (or sync) deletions.
+ if (deleted_details->archived)
+ return;
+
+ if (!deleted_details->all_history) {
+ DeleteSyncedFavicons(deleted_details->favicon_urls);
+ return;
+ }
+
+ // All history was cleared: just delete all favicons.
+ DVLOG(1) << "History clear detected, deleting all synced favicons.";
+ syncer::SyncChangeList image_deletions, tracking_deletions;
+ for (FaviconMap::iterator iter = synced_favicons_.begin();
+ iter != synced_favicons_.end();) {
+ iter = DeleteSyncedFavicon(iter,
+ &image_deletions,
+ &tracking_deletions);
+ }
+
+ if (favicon_images_sync_processor_.get() &&
+ favicon_tracking_sync_processor_.get()) {
+ favicon_images_sync_processor_->ProcessSyncChanges(FROM_HERE,
+ image_deletions);
+ favicon_tracking_sync_processor_->ProcessSyncChanges(FROM_HERE,
+ tracking_deletions);
+ }
+}
+
bool FaviconCache::FaviconRecencyFunctor::operator()(
const linked_ptr<SyncedFaviconInfo>& lhs,
const linked_ptr<SyncedFaviconInfo>& rhs) const {
@@ -665,20 +706,9 @@ void FaviconCache::ExpireFaviconsIfNecessary(
while (recent_favicons_.size() > max_sync_favicon_limit_) {
linked_ptr<SyncedFaviconInfo> candidate = *recent_favicons_.begin();
DVLOG(1) << "Expiring favicon " << candidate->favicon_url.spec();
- recent_favicons_.erase(recent_favicons_.begin());
- synced_favicons_.erase(candidate->favicon_url);
- image_changes->push_back(
- syncer::SyncChange(FROM_HERE,
- syncer::SyncChange::ACTION_DELETE,
- syncer::SyncData::CreateLocalDelete(
- candidate->favicon_url.spec(),
- syncer::FAVICON_IMAGES)));
- tracking_changes->push_back(
- syncer::SyncChange(FROM_HERE,
- syncer::SyncChange::ACTION_DELETE,
- syncer::SyncData::CreateLocalDelete(
- candidate->favicon_url.spec(),
- syncer::FAVICON_TRACKING)));
+ DeleteSyncedFavicon(synced_favicons_.find(candidate->favicon_url),
+ image_changes,
+ tracking_changes);
}
DCHECK_EQ(recent_favicons_.size(), synced_favicons_.size());
}
@@ -830,6 +860,55 @@ syncer::SyncData FaviconCache::CreateSyncDataFromLocalFavicon(
return data;
}
+void FaviconCache::DeleteSyncedFavicons(const std::set<GURL>& favicon_urls) {
+ syncer::SyncChangeList image_deletions, tracking_deletions;
+ for (std::set<GURL>::const_iterator iter = favicon_urls.begin();
+ iter != favicon_urls.end(); ++iter) {
+ FaviconMap::iterator favicon_iter = synced_favicons_.find(*iter);
+ if (favicon_iter == synced_favicons_.end())
+ continue;
+ DeleteSyncedFavicon(favicon_iter,
+ &image_deletions,
+ &tracking_deletions);
+ }
+ DVLOG(1) << "Deleting " << image_deletions.size() << " synced favicons.";
+ if (favicon_images_sync_processor_.get() &&
+ favicon_tracking_sync_processor_.get()) {
+ favicon_images_sync_processor_->ProcessSyncChanges(FROM_HERE,
+ image_deletions);
+ favicon_tracking_sync_processor_->ProcessSyncChanges(FROM_HERE,
+ tracking_deletions);
+ }
+}
+
+FaviconCache::FaviconMap::iterator FaviconCache::DeleteSyncedFavicon(
+ FaviconMap::iterator favicon_iter,
+ syncer::SyncChangeList* image_changes,
+ syncer::SyncChangeList* tracking_changes) {
+ linked_ptr<SyncedFaviconInfo> favicon_info = favicon_iter->second;
+ image_changes->push_back(
+ syncer::SyncChange(FROM_HERE,
+ syncer::SyncChange::ACTION_DELETE,
+ syncer::SyncData::CreateLocalDelete(
+ favicon_info->favicon_url.spec(),
+ syncer::FAVICON_IMAGES)));
+ tracking_changes->push_back(
+ syncer::SyncChange(FROM_HERE,
+ syncer::SyncChange::ACTION_DELETE,
+ syncer::SyncData::CreateLocalDelete(
+ favicon_info->favicon_url.spec(),
+ syncer::FAVICON_TRACKING)));
+ FaviconMap::iterator next = favicon_iter;
+ next++;
+ DropSyncedFavicon(favicon_iter);
+ return next;
+}
+
+void FaviconCache::DropSyncedFavicon(FaviconMap::iterator favicon_iter) {
+ recent_favicons_.erase(favicon_iter->second);
+ synced_favicons_.erase(favicon_iter);
+}
+
size_t FaviconCache::NumFaviconsForTest() const {
return synced_favicons_.size();
}
diff --git a/chrome/browser/sync/glue/favicon_cache.h b/chrome/browser/sync/glue/favicon_cache.h
index fdf97ea..b7c8f7c 100644
--- a/chrome/browser/sync/glue/favicon_cache.h
+++ b/chrome/browser/sync/glue/favicon_cache.h
@@ -18,6 +18,8 @@
#include "chrome/browser/history/history_types.h"
#include "chrome/browser/sessions/session_id.h"
#include "chrome/common/cancelable_task_tracker.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
#include "googleurl/src/gurl.h"
#include "sync/api/sync_change.h"
#include "sync/api/sync_error_factory.h"
@@ -52,7 +54,8 @@ class FaviconCacheObserver {
// Encapsulates the logic for loading and storing synced favicons.
// TODO(zea): make this a ProfileKeyedService.
-class FaviconCache : public syncer::SyncableService {
+class FaviconCache : public syncer::SyncableService,
+ public content::NotificationObserver {
public:
FaviconCache(Profile* profile, int max_sync_favicon_limit);
virtual ~FaviconCache();
@@ -106,6 +109,11 @@ class FaviconCache : public syncer::SyncableService {
void SetLegacyDelegate(FaviconCacheObserver* observer);
void RemoveLegacyDelegate();
+ // NotificationObserver implementation.
+ virtual void Observe(int type,
+ const content::NotificationSource& source,
+ const content::NotificationDetails& details) OVERRIDE;
+
private:
friend class SyncFaviconCacheTest;
@@ -179,6 +187,21 @@ class FaviconCache : public syncer::SyncableService {
syncer::ModelType type,
const GURL& favicon_url) const;
+ // Deletes all synced favicons corresponding with |favicon_urls| and pushes
+ // the deletions to sync.
+ void DeleteSyncedFavicons(const std::set<GURL>& favicon_urls);
+
+ // Deletes the favicon pointed to by |favicon_iter| and appends the necessary
+ // sync deletions to |image_changes| and |tracking_changes|.
+ // Returns an iterator to the favicon after |favicon_iter|.
+ FaviconMap::iterator DeleteSyncedFavicon(
+ FaviconMap::iterator favicon_iter,
+ syncer::SyncChangeList* image_changes,
+ syncer::SyncChangeList* tracking_changes);
+
+ // Locally drops the favicon pointed to by |favicon_iter|.
+ void DropSyncedFavicon(FaviconMap::iterator favicon_iter);
+
// For testing only.
size_t NumFaviconsForTest() const;
size_t NumTasksForTest() const;
@@ -214,6 +237,9 @@ class FaviconCache : public syncer::SyncableService {
scoped_ptr<syncer::SyncChangeProcessor> favicon_images_sync_processor_;
scoped_ptr<syncer::SyncChangeProcessor> favicon_tracking_sync_processor_;
+ // For listening to history deletions.
+ content::NotificationRegistrar notification_registrar_;
+
// Maximum number of favicons to sync. 0 means no limit.
const size_t max_sync_favicon_limit_;
diff --git a/chrome/browser/sync/glue/favicon_cache_unittest.cc b/chrome/browser/sync/glue/favicon_cache_unittest.cc
index 9eac341..f7c146b 100644
--- a/chrome/browser/sync/glue/favicon_cache_unittest.cc
+++ b/chrome/browser/sync/glue/favicon_cache_unittest.cc
@@ -5,6 +5,9 @@
#include "chrome/browser/sync/glue/favicon_cache.h"
#include "base/stringprintf.h"
+#include "chrome/browser/history/history_notifications.h"
+#include "chrome/common/chrome_notification_types.h"
+#include "content/public/browser/notification_service.h"
#include "sync/api/sync_error_factory_mock.h"
#include "sync/protocol/favicon_image_specifics.pb.h"
#include "sync/protocol/favicon_tracking_specifics.pb.h"
@@ -238,12 +241,17 @@ testing::AssertionResult VerifyChanges(
return testing::AssertionFailure() << "Change datatype doesn't match.";
if (change_list[i].change_type() != expected_change_types[i])
return testing::AssertionFailure() << "Change type doesn't match.";
- testing::AssertionResult compare_result =
- CompareFaviconDataToSpecifics(
- BuildFaviconData(expected_icons[i]),
- change_list[i].sync_data().GetSpecifics());
- if (compare_result.failure_message())
- return compare_result;
+ if (change_list[i].change_type() == syncer::SyncChange::ACTION_DELETE) {
+ if (change_list[i].sync_data().GetTag() != data.icon_url.spec())
+ return testing::AssertionFailure() << "Deletion url does not match.";
+ } else {
+ testing::AssertionResult compare_result =
+ CompareFaviconDataToSpecifics(
+ data,
+ change_list[i].sync_data().GetSpecifics());
+ if (!compare_result)
+ return compare_result;
+ }
}
return testing::AssertionSuccess();
}
@@ -1281,4 +1289,110 @@ TEST_F(SyncFaviconCacheTest, ExpireOnFaviconVisited) {
EXPECT_EQ((unsigned long)kMaxSyncFavicons, GetFaviconCount());
}
+// A full history clear notification should result in all synced favicons being
+// deleted.
+TEST_F(SyncFaviconCacheTest, HistoryFullClear) {
+ syncer::SyncDataList initial_image_data, initial_tracking_data;
+ std::vector<int> expected_icons;
+ std::vector<syncer::SyncChange::SyncChangeType> expected_deletions;
+ for (int i = 0; i < kFaviconBatchSize; ++i) {
+ expected_icons.push_back(i);
+ expected_deletions.push_back(syncer::SyncChange::ACTION_DELETE);
+ TestFaviconData test_data = BuildFaviconData(i);
+ sync_pb::EntitySpecifics image_specifics, tracking_specifics;
+ FillImageSpecifics(test_data,
+ image_specifics.mutable_favicon_image());
+ initial_image_data.push_back(
+ syncer::SyncData::CreateRemoteData(1,
+ image_specifics));
+ FillTrackingSpecifics(BuildFaviconData(i),
+ tracking_specifics.mutable_favicon_tracking());
+ initial_tracking_data.push_back(
+ syncer::SyncData::CreateRemoteData(1,
+ tracking_specifics));
+ }
+
+ SetUpInitialSync(initial_image_data, initial_tracking_data);
+ syncer::SyncChangeList changes = processor()->GetAndResetChangeList();
+ EXPECT_TRUE(changes.empty());
+
+ history::URLsDeletedDetails deletions;
+ deletions.all_history = true;
+ EXPECT_EQ((unsigned long)kFaviconBatchSize, GetFaviconCount());
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_HISTORY_URLS_DELETED,
+ content::Source<Profile>(NULL),
+ content::Details<history::URLsDeletedDetails>(&deletions));
+ EXPECT_EQ(0U, GetFaviconCount());
+ changes = processor()->GetAndResetChangeList();
+ ASSERT_EQ(changes.size(), (unsigned long)kFaviconBatchSize*2);
+ syncer::SyncChangeList changes_1, changes_2;
+ for (int i = 0; i < kFaviconBatchSize; ++i) {
+ changes_1.push_back(changes[i]);
+ changes_2.push_back(changes[i + kFaviconBatchSize]);
+ }
+ VerifyChanges(syncer::FAVICON_IMAGES,
+ expected_deletions,
+ expected_icons,
+ changes_1);
+ VerifyChanges(syncer::FAVICON_TRACKING,
+ expected_deletions,
+ expected_icons,
+ changes_2);
+}
+
+// A partial history clear notification should result in the expired favicons
+// also being deleted from sync.
+TEST_F(SyncFaviconCacheTest, HistorySubsetClear) {
+ syncer::SyncDataList initial_image_data, initial_tracking_data;
+ std::vector<int> expected_icons;
+ std::vector<syncer::SyncChange::SyncChangeType> expected_deletions;
+ history::URLsDeletedDetails deletions;
+ for (int i = 0; i < kFaviconBatchSize; ++i) {
+ TestFaviconData test_data = BuildFaviconData(i);
+ if (i < kFaviconBatchSize/2) {
+ expected_icons.push_back(i);
+ expected_deletions.push_back(syncer::SyncChange::ACTION_DELETE);
+ deletions.favicon_urls.insert(test_data.icon_url);
+ }
+ sync_pb::EntitySpecifics image_specifics, tracking_specifics;
+ FillImageSpecifics(test_data,
+ image_specifics.mutable_favicon_image());
+ initial_image_data.push_back(
+ syncer::SyncData::CreateRemoteData(1,
+ image_specifics));
+ FillTrackingSpecifics(BuildFaviconData(i),
+ tracking_specifics.mutable_favicon_tracking());
+ initial_tracking_data.push_back(
+ syncer::SyncData::CreateRemoteData(1,
+ tracking_specifics));
+ }
+
+ SetUpInitialSync(initial_image_data, initial_tracking_data);
+ syncer::SyncChangeList changes = processor()->GetAndResetChangeList();
+ EXPECT_TRUE(changes.empty());
+
+ EXPECT_EQ((unsigned long)kFaviconBatchSize, GetFaviconCount());
+ content::NotificationService::current()->Notify(
+ chrome::NOTIFICATION_HISTORY_URLS_DELETED,
+ content::Source<Profile>(NULL),
+ content::Details<history::URLsDeletedDetails>(&deletions));
+ EXPECT_EQ((unsigned long)kFaviconBatchSize/2, GetFaviconCount());
+ changes = processor()->GetAndResetChangeList();
+ ASSERT_EQ(changes.size(), (unsigned long)kFaviconBatchSize);
+ syncer::SyncChangeList changes_1, changes_2;
+ for (size_t i = 0; i < kFaviconBatchSize/2; ++i) {
+ changes_1.push_back(changes[i]);
+ changes_2.push_back(changes[i + kFaviconBatchSize/2]);
+ }
+ VerifyChanges(syncer::FAVICON_IMAGES,
+ expected_deletions,
+ expected_icons,
+ changes_1);
+ VerifyChanges(syncer::FAVICON_TRACKING,
+ expected_deletions,
+ expected_icons,
+ changes_2);
+}
+
} // namespace browser_sync