diff options
author | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-15 04:56:52 +0000 |
---|---|---|
committer | zea@chromium.org <zea@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-03-15 04:56:52 +0000 |
commit | e6337c04312f67e9229a73f55ef7da1f4c0e0d45 (patch) | |
tree | f5abad80bf116eec765ad20bfd7744275a11dd11 /chrome/browser/sync/glue | |
parent | 2ecd26eab5a8276544c37787d1a206d64479e339 (diff) | |
download | chromium_src-e6337c04312f67e9229a73f55ef7da1f4c0e0d45.zip chromium_src-e6337c04312f67e9229a73f55ef7da1f4c0e0d45.tar.gz chromium_src-e6337c04312f67e9229a73f55ef7da1f4c0e0d45.tar.bz2 |
[Sync] Add sync logic to FaviconCache
This enables syncing favicons behind the --enable-sync-favicons switch.
No expiration of favicons is performed yet, and only low resolutions favicons
are currently synced.
BUG=154886
Review URL: https://chromiumcodereview.appspot.com/12509004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@188260 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/sync/glue')
-rw-r--r-- | chrome/browser/sync/glue/favicon_cache.cc | 426 | ||||
-rw-r--r-- | chrome/browser/sync/glue/favicon_cache.h | 32 | ||||
-rw-r--r-- | chrome/browser/sync/glue/favicon_cache_unittest.cc | 944 | ||||
-rw-r--r-- | chrome/browser/sync/glue/session_model_associator.cc | 12 | ||||
-rw-r--r-- | chrome/browser/sync/glue/session_model_associator.h | 3 | ||||
-rw-r--r-- | chrome/browser/sync/glue/session_model_associator_unittest.cc | 2 |
6 files changed, 1358 insertions, 61 deletions
diff --git a/chrome/browser/sync/glue/favicon_cache.cc b/chrome/browser/sync/glue/favicon_cache.cc index 72fd5ae..fd2ebb8 100644 --- a/chrome/browser/sync/glue/favicon_cache.cc +++ b/chrome/browser/sync/glue/favicon_cache.cc @@ -4,10 +4,15 @@ #include "chrome/browser/sync/glue/favicon_cache.h" +#include <set> + #include "chrome/browser/favicon/favicon_service.h" #include "chrome/browser/favicon/favicon_service_factory.h" #include "chrome/browser/history/history_types.h" #include "sync/api/time.h" +#include "sync/protocol/favicon_image_specifics.pb.h" +#include "sync/protocol/favicon_tracking_specifics.pb.h" +#include "sync/protocol/sync.pb.h" #include "ui/gfx/favicon_size.h" namespace browser_sync { @@ -88,6 +93,62 @@ std::string IconSizeToString(IconSize icon_size) { } } +// Extract the favicon url from either of the favicon types. +GURL GetFaviconURLFromSpecifics(const sync_pb::EntitySpecifics& specifics) { + if (specifics.has_favicon_tracking()) + return GURL(specifics.favicon_tracking().favicon_url()); + else + return GURL(specifics.favicon_image().favicon_url()); +} + +// Convert protobuf image data into a FaviconBitmapResult. +history::FaviconBitmapResult GetImageDataFromSpecifics( + const sync_pb::FaviconData& favicon_data) { + base::RefCountedString* temp_string = + new base::RefCountedString(); + temp_string->data() = favicon_data.favicon(); + history::FaviconBitmapResult bitmap_result; + bitmap_result.bitmap_data = temp_string; + bitmap_result.pixel_size.set_height(favicon_data.height()); + bitmap_result.pixel_size.set_width(favicon_data.width()); + return bitmap_result; +} + +// Convert a FaviconBitmapResult into protobuf image data. +void FillSpecificsWithImageData( + const history::FaviconBitmapResult& bitmap_result, + sync_pb::FaviconData* favicon_data) { + if (!bitmap_result.bitmap_data.get()) + return; + favicon_data->set_height(bitmap_result.pixel_size.height()); + favicon_data->set_width(bitmap_result.pixel_size.width()); + favicon_data->set_favicon(bitmap_result.bitmap_data->front(), + bitmap_result.bitmap_data->size()); +} + +// Build a FaviconImageSpecifics from a SyncedFaviconInfo. +void BuildImageSpecifics( + const SyncedFaviconInfo* favicon_info, + sync_pb::FaviconImageSpecifics* image_specifics) { + image_specifics->set_favicon_url(favicon_info->favicon_url.spec()); + FillSpecificsWithImageData(favicon_info->bitmap_data[SIZE_16], + image_specifics->mutable_favicon_web()); + // TODO(zea): bring this back if we can handle the load. + // FillSpecificsWithImageData(favicon_info->bitmap_data[SIZE_32], + // image_specifics->mutable_favicon_web_32()); + // FillSpecificsWithImageData(favicon_info->bitmap_data[SIZE_64], + // image_specifics->mutable_favicon_touch_64()); +} + +// Build a FaviconTrackingSpecifics from a SyncedFaviconInfo. +void BuildTrackingSpecifics( + const SyncedFaviconInfo* favicon_info, + sync_pb::FaviconTrackingSpecifics* tracking_specifics) { + tracking_specifics->set_favicon_url(favicon_info->favicon_url.spec()); + tracking_specifics->set_last_visit_time_ms(favicon_info->last_visit_time); + tracking_specifics->set_is_bookmarked(favicon_info->is_bookmarked); +} + } // namespace FaviconCacheObserver::~FaviconCacheObserver() {} @@ -104,23 +165,138 @@ syncer::SyncMergeResult FaviconCache::MergeDataAndStartSyncing( const syncer::SyncDataList& initial_sync_data, scoped_ptr<syncer::SyncChangeProcessor> sync_processor, scoped_ptr<syncer::SyncErrorFactory> error_handler) { - return syncer::SyncMergeResult(type); + DCHECK(type == syncer::FAVICON_IMAGES || type == syncer::FAVICON_TRACKING); + if (type == syncer::FAVICON_IMAGES) + favicon_images_sync_processor_ = sync_processor.Pass(); + else + favicon_tracking_sync_processor_ = sync_processor.Pass(); + + syncer::SyncMergeResult merge_result(type); + merge_result.set_num_items_before_association(synced_favicons_.size()); + std::set<GURL> unsynced_favicon_urls; + for (FaviconMap::const_iterator iter = synced_favicons_.begin(); + iter != synced_favicons_.end(); ++iter) { + unsynced_favicon_urls.insert(iter->first); + } + + syncer::SyncChangeList local_changes; + for (syncer::SyncDataList::const_iterator iter = initial_sync_data.begin(); + iter != initial_sync_data.end(); ++iter) { + GURL favicon_url = GetLocalFaviconFromSyncedData(*iter); + if (favicon_url.is_valid()) { + unsynced_favicon_urls.erase(favicon_url); + MergeSyncFavicon(*iter, &local_changes); + merge_result.set_num_items_modified( + merge_result.num_items_modified() + 1); + } else { + AddLocalFaviconFromSyncedData(*iter); + merge_result.set_num_items_added(merge_result.num_items_added() + 1); + } + } + + for (std::set<GURL>::const_iterator iter = unsynced_favicon_urls.begin(); + iter != unsynced_favicon_urls.end(); ++iter) { + local_changes.push_back( + syncer::SyncChange(FROM_HERE, + syncer::SyncChange::ACTION_ADD, + CreateSyncDataFromLocalFavicon(type, *iter))); + } + merge_result.set_num_items_after_association(synced_favicons_.size()); + + // TODO(zea): enforce sync limit on favicon count. + if (type == syncer::FAVICON_IMAGES) { + merge_result.set_error( + favicon_images_sync_processor_->ProcessSyncChanges(FROM_HERE, + local_changes)); + } else { + merge_result.set_error( + favicon_tracking_sync_processor_->ProcessSyncChanges(FROM_HERE, + local_changes)); + } + return merge_result; } void FaviconCache::StopSyncing(syncer::ModelType type) { + favicon_images_sync_processor_.reset(); + favicon_tracking_sync_processor_.reset(); cancelable_task_tracker_.TryCancelAll(); page_task_map_.clear(); } syncer::SyncDataList FaviconCache::GetAllSyncData(syncer::ModelType type) const { - return syncer::SyncDataList(); + syncer::SyncDataList data_list; + for (FaviconMap::const_iterator iter = synced_favicons_.begin(); + iter != synced_favicons_.end(); ++iter) { + data_list.push_back(CreateSyncDataFromLocalFavicon(type, iter->first)); + } + return data_list; } syncer::SyncError FaviconCache::ProcessSyncChanges( const tracked_objects::Location& from_here, const syncer::SyncChangeList& change_list) { - return syncer::SyncError(); + + syncer::SyncChangeList new_changes; + syncer::SyncError error; + syncer::ModelType type = syncer::UNSPECIFIED; + for (syncer::SyncChangeList::const_iterator iter = change_list.begin(); + iter != change_list.end(); ++iter) { + type = iter->sync_data().GetDataType(); + DCHECK(type == syncer::FAVICON_IMAGES || type == syncer::FAVICON_TRACKING); + GURL favicon_url = + GetFaviconURLFromSpecifics(iter->sync_data().GetSpecifics()); + if (!favicon_url.is_valid()) { + error.Reset(FROM_HERE, "Received invalid favicon url.", type); + break; + } + FaviconMap::iterator favicon_iter = synced_favicons_.find(favicon_url); + if (iter->change_type() == syncer::SyncChange::ACTION_DELETE) { + if (favicon_iter == synced_favicons_.end()) { + // Two clients might wind up deleting different parts of the same + // favicon, so ignore this. + continue; + } else { + DVLOG(1) << "Deleting favicon at " << favicon_url.spec(); + synced_favicons_.erase(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 + // happens, and if it becomes a problem delete each part individually + // from the local model. + } + } else if (iter->change_type() == syncer::SyncChange::ACTION_UPDATE || + iter->change_type() == syncer::SyncChange::ACTION_ADD) { + // Adds and updates are treated the same due to the lack of strong + // consistency (it's possible we'll receive an update for a tracking info + // before we've received the add for the image, and should handle both + // gracefully). + if (favicon_iter == synced_favicons_.end()) { + DVLOG(1) << "Adding favicon at " << favicon_url.spec(); + AddLocalFaviconFromSyncedData(iter->sync_data()); + } else { + DVLOG(1) << "Updating favicon at " << favicon_url.spec(); + MergeSyncFavicon(iter->sync_data(), &new_changes); + } + } else { + error.Reset(FROM_HERE, "Invalid action received.", type); + break; + } + } + + if (!error.IsSet() && !new_changes.empty()) { + if (type == syncer::FAVICON_IMAGES) { + error = + favicon_images_sync_processor_->ProcessSyncChanges(FROM_HERE, + new_changes); + } else { + error = + favicon_tracking_sync_processor_->ProcessSyncChanges(FROM_HERE, + new_changes); + } + } + + return error; } void FaviconCache::OnPageFaviconUpdated(const GURL& page_url) { @@ -161,7 +337,12 @@ void FaviconCache::OnFaviconVisited(const GURL& page_url, DVLOG(1) << "Associating " << page_url.spec() << "with favicon at " << favicon_url.spec() << " and marking visited."; page_favicon_map_[page_url] = favicon_url; - UpdateSyncState(favicon_url, SYNC_TRACKING); + synced_favicons_[favicon_url]->last_visit_time = std::max( + synced_favicons_[favicon_url]->last_visit_time, + syncer::TimeToProtoTime(base::Time::Now())); + UpdateSyncState(favicon_url, + SYNC_TRACKING, + syncer::SyncChange::ACTION_UPDATE); } bool FaviconCache::GetSyncedFaviconForFaviconURL( @@ -198,7 +379,8 @@ bool FaviconCache::GetSyncedFaviconForPageURL( void FaviconCache::OnReceivedSyncFavicon(const GURL& page_url, const GURL& icon_url, - const std::string& icon_bytes) { + const std::string& icon_bytes, + int64 visit_time_ms) { if (!icon_url.is_valid() || !page_url.is_valid()) return; DVLOG(1) << "Associating " << page_url.spec() << " with favicon at " @@ -221,14 +403,17 @@ void FaviconCache::OnReceivedSyncFavicon(const GURL& page_url, return; SyncedFaviconInfo* favicon_info = GetFaviconInfo(icon_url); + if (!favicon_info) + return; // We reached the in-memory limit. base::RefCountedString* temp_string = new base::RefCountedString(); temp_string->data() = icon_bytes; favicon_info->bitmap_data[SIZE_16].bitmap_data = temp_string; // We assume legacy favicons are 16x16. favicon_info->bitmap_data[SIZE_16].pixel_size.set_width(16); - favicon_info->bitmap_data[SIZE_16].pixel_size.set_width(16); + favicon_info->bitmap_data[SIZE_16].pixel_size.set_height(16); + favicon_info->last_visit_time = visit_time_ms; - UpdateSyncState(icon_url, SYNC_BOTH); + UpdateSyncState(icon_url, SYNC_BOTH, syncer::SyncChange::ACTION_ADD); } void FaviconCache::SetLegacyDelegate(FaviconCacheObserver* observer) { @@ -254,6 +439,7 @@ void FaviconCache::OnFaviconDataAvailable( return; } + bool added_new_favicon = false; GURL favicon_url; for (size_t i = 0; i < bitmap_results.size(); ++i) { const history::FaviconBitmapResult& bitmap_result = bitmap_results[i]; @@ -261,22 +447,12 @@ void FaviconCache::OnFaviconDataAvailable( continue; // Can happen if the page is still loading. favicon_url = bitmap_result.icon_url; FaviconMap::iterator favicon_iter = synced_favicons_.find(favicon_url); - - SyncedFaviconInfo* favicon_info = NULL; - if (favicon_iter == synced_favicons_.end()) { - // If we've already reached our in memory favicon limit, just return. - // TODO(zea): UMA this. - if (kMaxFaviconsInMem != 0 && - synced_favicons_.size() > kMaxFaviconsInMem) { - return; - } - favicon_info = new SyncedFaviconInfo(favicon_url); - synced_favicons_[favicon_url] = make_linked_ptr(favicon_info); - } else { - // TODO(zea): add a dampening factor so animated favicons don't constantly - // update the image data. - favicon_info = favicon_iter->second.get(); - } + added_new_favicon |= (favicon_iter == synced_favicons_.end()); + // TODO(zea): Add a dampening factor so animated favicons don't constantly + // update the image data. + SyncedFaviconInfo* favicon_info = GetFaviconInfo(favicon_url); + if (!favicon_info) + break; // We reached the in-memory limit. favicon_info->last_visit_time = std::max( favicon_info->last_visit_time, syncer::TimeToProtoTime(base::Time::Now())); @@ -301,18 +477,63 @@ void FaviconCache::OnFaviconDataAvailable( } } - if (!favicon_url.is_valid()) - return; page_favicon_map_[page_url] = favicon_url; - UpdateSyncState(favicon_url, SYNC_BOTH); + UpdateSyncState(favicon_url, + SYNC_BOTH, + (added_new_favicon ? + syncer::SyncChange::ACTION_ADD : + syncer::SyncChange::ACTION_UPDATE)); if (legacy_delegate_) legacy_delegate_->OnFaviconUpdated(page_url, favicon_url); } -void FaviconCache::UpdateSyncState(const GURL& icon_url, - SyncState state_to_update) { - // TODO(zea): implement this. +void FaviconCache::UpdateSyncState( + const GURL& icon_url, + SyncState state_to_update, + syncer::SyncChange::SyncChangeType change_type) { + if (!favicon_images_sync_processor_.get() || + !favicon_tracking_sync_processor_.get()) + return; + + FaviconMap::const_iterator iter = synced_favicons_.find(icon_url); + DCHECK(iter != synced_favicons_.end()); + const SyncedFaviconInfo* favicon_info = iter->second.get(); + + if (state_to_update == SYNC_IMAGE || state_to_update == SYNC_BOTH) { + sync_pb::EntitySpecifics new_specifics; + sync_pb::FaviconImageSpecifics* image_specifics = + new_specifics.mutable_favicon_image(); + BuildImageSpecifics(favicon_info, image_specifics); + + syncer::SyncChangeList changes; + changes.push_back( + syncer::SyncChange(FROM_HERE, + change_type, + syncer::SyncData::CreateLocalData( + icon_url.spec(), + icon_url.spec(), + new_specifics))); + // TODO(zea): enforce sync limit on favicon count. + favicon_images_sync_processor_->ProcessSyncChanges(FROM_HERE, changes); + } + if (state_to_update == SYNC_TRACKING || state_to_update == SYNC_BOTH) { + sync_pb::EntitySpecifics new_specifics; + sync_pb::FaviconTrackingSpecifics* tracking_specifics = + new_specifics.mutable_favicon_tracking(); + BuildTrackingSpecifics(favicon_info, tracking_specifics); + + syncer::SyncChangeList changes; + changes.push_back( + syncer::SyncChange(FROM_HERE, + change_type, + syncer::SyncData::CreateLocalData( + icon_url.spec(), + icon_url.spec(), + new_specifics))); + // TODO(zea): enforce sync limit on favicon count. + favicon_tracking_sync_processor_->ProcessSyncChanges(FROM_HERE, changes); + } } SyncedFaviconInfo* FaviconCache::GetFaviconInfo( @@ -326,6 +547,153 @@ SyncedFaviconInfo* FaviconCache::GetFaviconInfo( return favicon_info; } +GURL FaviconCache::GetLocalFaviconFromSyncedData( + const syncer::SyncData& sync_favicon) const { + syncer::ModelType type = sync_favicon.GetDataType(); + DCHECK(type == syncer::FAVICON_IMAGES || type == syncer::FAVICON_TRACKING); + GURL favicon_url = GetFaviconURLFromSpecifics(sync_favicon.GetSpecifics()); + return (synced_favicons_.count(favicon_url) > 0 ? favicon_url : GURL()); +} + +void FaviconCache::MergeSyncFavicon(const syncer::SyncData& sync_favicon, + syncer::SyncChangeList* sync_changes) { + syncer::ModelType type = sync_favicon.GetDataType(); + DCHECK(type == syncer::FAVICON_IMAGES || type == syncer::FAVICON_TRACKING); + sync_pb::EntitySpecifics new_specifics; + GURL favicon_url = GetFaviconURLFromSpecifics(sync_favicon.GetSpecifics()); + if (type == syncer::FAVICON_IMAGES) { + sync_pb::FaviconImageSpecifics image_specifics = + sync_favicon.GetSpecifics().favicon_image(); + FaviconMap::const_iterator iter = synced_favicons_.find(favicon_url); + DCHECK(iter != synced_favicons_.end()); + SyncedFaviconInfo* favicon_info = iter->second.get(); + + // Remote image data always clobbers local image data. + bool needs_update = false; + if (image_specifics.has_favicon_web()) { + favicon_info->bitmap_data[SIZE_16] = GetImageDataFromSpecifics( + image_specifics.favicon_web()); + } else if (favicon_info->bitmap_data[SIZE_16].bitmap_data.get()) { + needs_update = true; + } + if (image_specifics.has_favicon_web_32()) { + favicon_info->bitmap_data[SIZE_32] = GetImageDataFromSpecifics( + image_specifics.favicon_web_32()); + } else if (favicon_info->bitmap_data[SIZE_32].bitmap_data.get()) { + needs_update = true; + } + if (image_specifics.has_favicon_touch_64()) { + favicon_info->bitmap_data[SIZE_64] = GetImageDataFromSpecifics( + image_specifics.favicon_touch_64()); + } else if (favicon_info->bitmap_data[SIZE_64].bitmap_data.get()) { + needs_update = true; + } + + if (needs_update) + BuildImageSpecifics(favicon_info, new_specifics.mutable_favicon_image()); + } else { + sync_pb::FaviconTrackingSpecifics tracking_specifics = + sync_favicon.GetSpecifics().favicon_tracking(); + FaviconMap::const_iterator iter = synced_favicons_.find(favicon_url); + DCHECK(iter != synced_favicons_.end()); + SyncedFaviconInfo* favicon_info = iter->second.get(); + + // Tracking data is merged, such that bookmark data is the logical OR + // of the two, and last visit time is the most recent. + favicon_info->last_visit_time = + (favicon_info->last_visit_time > + tracking_specifics.last_visit_time_ms() ? + favicon_info->last_visit_time : + tracking_specifics.last_visit_time_ms()); + favicon_info->is_bookmarked = (favicon_info->is_bookmarked || + tracking_specifics.is_bookmarked()); + + if (favicon_info->last_visit_time != + tracking_specifics.last_visit_time_ms() || + favicon_info->is_bookmarked != tracking_specifics.is_bookmarked()) { + BuildTrackingSpecifics(favicon_info, + new_specifics.mutable_favicon_tracking()); + } + } + + if (new_specifics.has_favicon_image() || + new_specifics.has_favicon_tracking()) { + sync_changes->push_back(syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + syncer::SyncData::CreateLocalData(favicon_url.spec(), + favicon_url.spec(), + new_specifics))); + } +} + +void FaviconCache::AddLocalFaviconFromSyncedData( + const syncer::SyncData& sync_favicon) { + syncer::ModelType type = sync_favicon.GetDataType(); + DCHECK(type == syncer::FAVICON_IMAGES || type == syncer::FAVICON_TRACKING); + if (type == syncer::FAVICON_IMAGES) { + sync_pb::FaviconImageSpecifics image_specifics = + sync_favicon.GetSpecifics().favicon_image(); + GURL favicon_url = GURL(image_specifics.favicon_url()); + DCHECK(favicon_url.is_valid()); + DCHECK(!synced_favicons_.count(favicon_url)); + + SyncedFaviconInfo* favicon_info = GetFaviconInfo(favicon_url); + if (!favicon_info) + return; // We reached the in-memory limit. + if (image_specifics.has_favicon_web()) { + favicon_info->bitmap_data[SIZE_16] = GetImageDataFromSpecifics( + image_specifics.favicon_web()); + } + if (image_specifics.has_favicon_web_32()) { + favicon_info->bitmap_data[SIZE_32] = GetImageDataFromSpecifics( + image_specifics.favicon_web_32()); + } + if (image_specifics.has_favicon_touch_64()) { + favicon_info->bitmap_data[SIZE_64] = GetImageDataFromSpecifics( + image_specifics.favicon_touch_64()); + } + } else { + sync_pb::FaviconTrackingSpecifics tracking_specifics = + sync_favicon.GetSpecifics().favicon_tracking(); + GURL favicon_url = GURL(tracking_specifics.favicon_url()); + DCHECK(favicon_url.is_valid()); + DCHECK(!synced_favicons_.count(favicon_url)); + + SyncedFaviconInfo* favicon_info = GetFaviconInfo(favicon_url); + if (!favicon_info) + return; // We reached the in-memory limit. + favicon_info->last_visit_time = tracking_specifics.last_visit_time_ms(); + favicon_info->is_bookmarked = tracking_specifics.is_bookmarked(); + } +} + +syncer::SyncData FaviconCache::CreateSyncDataFromLocalFavicon( + syncer::ModelType type, + const GURL& favicon_url) const { + DCHECK(type == syncer::FAVICON_IMAGES || type == syncer::FAVICON_TRACKING); + DCHECK(favicon_url.is_valid()); + FaviconMap::const_iterator iter = synced_favicons_.find(favicon_url); + DCHECK(iter != synced_favicons_.end()); + SyncedFaviconInfo* favicon_info = iter->second.get(); + + syncer::SyncData data; + sync_pb::EntitySpecifics specifics; + if (type == syncer::FAVICON_IMAGES) { + sync_pb::FaviconImageSpecifics* image_specifics = + specifics.mutable_favicon_image(); + BuildImageSpecifics(favicon_info, image_specifics); + } else { + sync_pb::FaviconTrackingSpecifics* tracking_specifics = + specifics.mutable_favicon_tracking(); + BuildTrackingSpecifics(favicon_info, tracking_specifics); + } + data = syncer::SyncData::CreateLocalData(favicon_url.spec(), + favicon_url.spec(), + specifics); + return data; +} + 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 eedb313..30f7e95 100644 --- a/chrome/browser/sync/glue/favicon_cache.h +++ b/chrome/browser/sync/glue/favicon_cache.h @@ -19,6 +19,7 @@ #include "chrome/browser/sessions/session_id.h" #include "chrome/common/cancelable_task_tracker.h" #include "googleurl/src/gurl.h" +#include "sync/api/sync_change.h" #include "sync/api/sync_error_factory.h" #include "sync/api/syncable_service.h" @@ -57,7 +58,6 @@ class FaviconCache : public syncer::SyncableService { virtual ~FaviconCache(); // SyncableService implementation. - // TODO(zea): implement these. virtual syncer::SyncMergeResult MergeDataAndStartSyncing( syncer::ModelType type, const syncer::SyncDataList& initial_sync_data, @@ -99,7 +99,8 @@ class FaviconCache : public syncer::SyncableService { // If |icon_bytes| is empty, only updates the page->favicon url mapping. void OnReceivedSyncFavicon(const GURL& page_url, const GURL& icon_url, - const std::string& icon_bytes); + const std::string& icon_bytes, + int64 visit_time_ms); // Support for syncing favicons using the legacy format (within tab sync). void SetLegacyDelegate(FaviconCacheObserver* observer); @@ -129,12 +130,33 @@ class FaviconCache : public syncer::SyncableService { const std::vector<history::FaviconBitmapResult>& bitmap_result); // Helper method to update the sync state of the favicon at |icon_url|. - void UpdateSyncState(const GURL& icon_url, SyncState state_to_update); + void UpdateSyncState(const GURL& icon_url, + SyncState state_to_update, + syncer::SyncChange::SyncChangeType change_type); // Helper method to get favicon info from |synced_favicons_|. If no info // exists for |icon_url|, creates a new SyncedFaviconInfo and returns it. SyncedFaviconInfo* GetFaviconInfo(const GURL& icon_url); + // Returns the local favicon url associated with |sync_favicon| if one exists + // in |synced_favicons_|, else returns an invalid GURL. + GURL GetLocalFaviconFromSyncedData( + const syncer::SyncData& sync_favicon) const; + + // Merges |sync_favicon| into |synced_favicons_|, updating |local_changes| + // with any changes that should be pushed to the sync processor. + void MergeSyncFavicon(const syncer::SyncData& sync_favicon, + syncer::SyncChangeList* sync_changes); + + // Updates |synced_favicons_| with the favicon data from |sync_favicon|. + void AddLocalFaviconFromSyncedData(const syncer::SyncData& sync_favicon); + + // Creates a SyncData object from the |type| data of |favicon_url| + // from within |synced_favicons_|. + syncer::SyncData CreateSyncDataFromLocalFavicon( + syncer::ModelType type, + const GURL& favicon_url) const; + // For testing only. size_t NumFaviconsForTest() const; @@ -155,12 +177,16 @@ class FaviconCache : public syncer::SyncableService { // TODO(zea): consider creating a favicon handler here for fetching unsynced // favicons from the web. + // Weak pointer factory for favicon loads. base::WeakPtrFactory<FaviconCache> weak_ptr_factory_; // Delegate for legacy favicon sync support. // TODO(zea): Remove this eventually. FaviconCacheObserver* legacy_delegate_; + scoped_ptr<syncer::SyncChangeProcessor> favicon_images_sync_processor_; + scoped_ptr<syncer::SyncChangeProcessor> favicon_tracking_sync_processor_; + DISALLOW_COPY_AND_ASSIGN(FaviconCache); }; diff --git a/chrome/browser/sync/glue/favicon_cache_unittest.cc b/chrome/browser/sync/glue/favicon_cache_unittest.cc index 3727aea..bee69c1 100644 --- a/chrome/browser/sync/glue/favicon_cache_unittest.cc +++ b/chrome/browser/sync/glue/favicon_cache_unittest.cc @@ -4,38 +4,383 @@ #include "chrome/browser/sync/glue/favicon_cache.h" +#include "base/stringprintf.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" +#include "sync/protocol/sync.pb.h" #include "testing/gtest/include/gtest/gtest.h" namespace browser_sync { -class SyncFaviconCacheTest : public testing::Test { +namespace { + +// Total number of favicons to use in sync tests. +const int kTotalFavicons = 10; + +// TestChangeProcessor -------------------------------------------------------- + +// Dummy SyncChangeProcessor used to help review what SyncChanges are pushed +// back up to Sync. +class TestChangeProcessor : public syncer::SyncChangeProcessor { public: - SyncFaviconCacheTest() : cache_(NULL) { + TestChangeProcessor(); + virtual ~TestChangeProcessor(); + + // Store a copy of all the changes passed in so we can examine them later. + virtual syncer::SyncError ProcessSyncChanges( + const tracked_objects::Location& from_here, + const syncer::SyncChangeList& change_list) OVERRIDE; + + bool contains_guid(const std::string& guid) const { + return change_map_.count(guid) != 0; } - virtual ~SyncFaviconCacheTest() {} - size_t GetFaviconCount() { - return cache_.NumFaviconsForTest(); - } - bool ExpectFaviconEquals(std::string page_url, std::string bytes) { - GURL gurl(page_url); - scoped_refptr<base::RefCountedMemory> favicon; - if (!cache_.GetSyncedFaviconForPageURL(gurl, &favicon)) - return false; - if (favicon->size() != bytes.size()) - return false; - for (size_t i = 0; i < favicon->size(); ++i) { - if (bytes[i] != *(favicon->front() + i)) - return false; + syncer::SyncChange change_for_guid(const std::string& guid) const { + DCHECK(contains_guid(guid)); + return change_map_.find(guid)->second; + } + + // Returns the last change list received, and resets the internal list. + syncer::SyncChangeList GetAndResetChangeList() { + syncer::SyncChangeList list; + list.swap(change_list_); + return list; + } + + void set_erroneous(bool erroneous) { erroneous_ = erroneous; } + + private: + // Track the changes received in ProcessSyncChanges. + std::map<std::string, syncer::SyncChange> change_map_; + syncer::SyncChangeList change_list_; + bool erroneous_; + + DISALLOW_COPY_AND_ASSIGN(TestChangeProcessor); +}; + +TestChangeProcessor::TestChangeProcessor() : erroneous_(false) { +} + +TestChangeProcessor::~TestChangeProcessor() { +} + +syncer::SyncError TestChangeProcessor::ProcessSyncChanges( + const tracked_objects::Location& from_here, + const syncer::SyncChangeList& change_list) { + if (erroneous_) { + return syncer::SyncError( + FROM_HERE, "Some error.", change_list[0].sync_data().GetDataType()); + } + + change_list_ = change_list; + change_map_.erase(change_map_.begin(), change_map_.end()); + for (syncer::SyncChangeList::const_iterator iter = change_list.begin(); + iter != change_list.end(); ++iter) { + change_map_[iter->sync_data().GetTitle()] = *iter; + } + return syncer::SyncError(); +} + + +// SyncChangeProcessorDelegate ------------------------------------------------ + +class SyncChangeProcessorDelegate : public syncer::SyncChangeProcessor { + public: + explicit SyncChangeProcessorDelegate(syncer::SyncChangeProcessor* recipient); + virtual ~SyncChangeProcessorDelegate(); + + // syncer::SyncChangeProcessor implementation. + virtual syncer::SyncError ProcessSyncChanges( + const tracked_objects::Location& from_here, + const syncer::SyncChangeList& change_list) OVERRIDE; + + private: + // The recipient of all sync changes. + syncer::SyncChangeProcessor* recipient_; + + DISALLOW_COPY_AND_ASSIGN(SyncChangeProcessorDelegate); +}; + +SyncChangeProcessorDelegate::SyncChangeProcessorDelegate( + syncer::SyncChangeProcessor* recipient) + : recipient_(recipient) { + DCHECK(recipient_); +} + +SyncChangeProcessorDelegate::~SyncChangeProcessorDelegate() { +} + +syncer::SyncError SyncChangeProcessorDelegate::ProcessSyncChanges( + const tracked_objects::Location& from_here, + const syncer::SyncChangeList& change_list) { + return recipient_->ProcessSyncChanges(from_here, change_list); +} + +// TestFaviconData ------------------------------------------------------------ +struct TestFaviconData { + TestFaviconData() : last_visit_time(0), is_bookmarked(false) {} + GURL page_url; + GURL icon_url; + std::string image_16; + std::string image_32; + std::string image_64; + int last_visit_time; + bool is_bookmarked; +}; + +TestFaviconData BuildFaviconData(int index) { + TestFaviconData data; + data.page_url = GURL(base::StringPrintf("http://bla.com/%i.html", index)); + data.icon_url = GURL(base::StringPrintf("http://bla.com/%i.ico", index)); + data.image_16 = base::StringPrintf("16 %i", index); + // TODO(zea): enable this once the cache supports writing them. + // data.image_32 = base::StringPrintf("32 %i", index); + // data.image_64 = base::StringPrintf("64 %i", index); + data.last_visit_time = index; + return data; +} + +void FillImageSpecifics( + const TestFaviconData& test_data, + sync_pb::FaviconImageSpecifics* image_specifics) { + image_specifics->set_favicon_url(test_data.icon_url.spec()); + if (!test_data.image_16.empty()) { + image_specifics->mutable_favicon_web()->set_height(16); + image_specifics->mutable_favicon_web()->set_width(16); + image_specifics->mutable_favicon_web()->set_favicon(test_data.image_16); + } + if (!test_data.image_32.empty()) { + image_specifics->mutable_favicon_web_32()->set_height(32); + image_specifics->mutable_favicon_web_32()->set_width(32); + image_specifics->mutable_favicon_web_32()->set_favicon(test_data.image_32); + } + if (!test_data.image_64.empty()) { + image_specifics->mutable_favicon_touch_64()->set_height(64); + image_specifics->mutable_favicon_touch_64()->set_width(64); + image_specifics->mutable_favicon_touch_64()-> + set_favicon(test_data.image_64); + } +} + +void FillTrackingSpecifics( + const TestFaviconData& test_data, + sync_pb::FaviconTrackingSpecifics* tracking_specifics) { + tracking_specifics->set_favicon_url(test_data.icon_url.spec()); + tracking_specifics->set_last_visit_time_ms(test_data.last_visit_time); + tracking_specifics->set_is_bookmarked(test_data.is_bookmarked); +} + +testing::AssertionResult CompareFaviconDataToSpecifics( + const TestFaviconData& test_data, + const sync_pb::EntitySpecifics& specifics) { + if (specifics.has_favicon_image()) { + sync_pb::FaviconImageSpecifics image_specifics = specifics.favicon_image(); + if (image_specifics.favicon_url() != test_data.icon_url.spec()) + return testing::AssertionFailure() << "Image icon url doesn't match."; + if (!test_data.image_16.empty()) { + if (image_specifics.favicon_web().favicon() != test_data.image_16 || + image_specifics.favicon_web().height() != 16 || + image_specifics.favicon_web().width() != 16) { + return testing::AssertionFailure() << "16p image data doesn't match."; + } + } else if (image_specifics.has_favicon_web()) { + return testing::AssertionFailure() << "Missing 16p favicon."; + } + if (!test_data.image_32.empty()) { + if (image_specifics.favicon_web_32().favicon() != test_data.image_32 || + image_specifics.favicon_web().height() != 32 || + image_specifics.favicon_web().width() != 32) { + return testing::AssertionFailure() << "32p image data doesn't match."; + } + } else if (image_specifics.has_favicon_web_32()) { + return testing::AssertionFailure() << "Missing 32p favicon."; + } + if (!test_data.image_64.empty()) { + if (image_specifics.favicon_touch_64().favicon() != test_data.image_64 || + image_specifics.favicon_web().height() != 64 || + image_specifics.favicon_web().width() != 64) { + return testing::AssertionFailure() << "64p image data doesn't match."; + } + } else if (image_specifics.has_favicon_touch_64()) { + return testing::AssertionFailure() << "Missing 64p favicon."; } - return true; + } else { + sync_pb::FaviconTrackingSpecifics tracking_specifics = + specifics.favicon_tracking(); + if (tracking_specifics.favicon_url() != test_data.icon_url.spec()) + return testing::AssertionFailure() << "Tracking icon url doesn't match."; + if (tracking_specifics.last_visit_time_ms() != test_data.last_visit_time) + return testing::AssertionFailure() << "Visit time doesn't match."; + if (tracking_specifics.is_bookmarked() != test_data.is_bookmarked) + return testing::AssertionFailure() << "Bookmark status doens't match."; + } + return testing::AssertionSuccess(); +} + +testing::AssertionResult VerifyChanges( + syncer::ModelType expected_model_type, + const std::vector<syncer::SyncChange::SyncChangeType>& + expected_change_types, + const std::vector<int>& expected_icons, + const syncer::SyncChangeList& change_list) { + DCHECK_EQ(expected_change_types.size(), expected_icons.size()); + if (change_list.size() != expected_icons.size()) + return testing::AssertionFailure() << "Change list size doesn't match."; + for (size_t i = 0; i < expected_icons.size(); ++i) { + TestFaviconData data = BuildFaviconData(expected_icons[i]); + if (change_list[i].sync_data().GetDataType() != expected_model_type) + 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; } + return testing::AssertionSuccess(); +} + +} // namespace + +class SyncFaviconCacheTest : public testing::Test { + public: + SyncFaviconCacheTest(); + virtual ~SyncFaviconCacheTest() {} + + void SetUpInitialSync(const syncer::SyncDataList& initial_image_data, + const syncer::SyncDataList& initial_tracking_data); + + size_t GetFaviconCount() const; + testing::AssertionResult ExpectFaviconEquals( + const std::string& page_url, + const std::string& bytes) const; + testing::AssertionResult VerifyLocalIcons( + const std::vector<int>& expected_icons); + testing::AssertionResult VerifyLocalCustomIcons( + const std::vector<TestFaviconData>& expected_icons); + + scoped_ptr<syncer::SyncChangeProcessor> CreateAndPassProcessor(); + scoped_ptr<syncer::SyncErrorFactory> CreateAndPassSyncErrorFactory(); + FaviconCache* cache() { return &cache_; } + TestChangeProcessor* processor() { return sync_processor_.get(); } private: FaviconCache cache_; + + // Our dummy ChangeProcessor used to inspect changes pushed to Sync. + scoped_ptr<TestChangeProcessor> sync_processor_; + scoped_ptr<SyncChangeProcessorDelegate> sync_processor_delegate_; }; +SyncFaviconCacheTest::SyncFaviconCacheTest() + : cache_(NULL), + sync_processor_(new TestChangeProcessor), + sync_processor_delegate_(new SyncChangeProcessorDelegate( + sync_processor_.get())) { +} + +void SyncFaviconCacheTest::SetUpInitialSync( + const syncer::SyncDataList& initial_image_data, + const syncer::SyncDataList& initial_tracking_data) { + cache()->MergeDataAndStartSyncing(syncer::FAVICON_IMAGES, + initial_image_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + ASSERT_EQ(0U, processor()->GetAndResetChangeList().size()); + cache()->MergeDataAndStartSyncing(syncer::FAVICON_TRACKING, + initial_tracking_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + ASSERT_EQ(0U, processor()->GetAndResetChangeList().size()); +} + +size_t SyncFaviconCacheTest::GetFaviconCount() const { + return cache_.NumFaviconsForTest(); +} + +testing::AssertionResult SyncFaviconCacheTest::ExpectFaviconEquals( + const std::string& page_url, + const std::string& bytes) const { + GURL gurl(page_url); + scoped_refptr<base::RefCountedMemory> favicon; + if (!cache_.GetSyncedFaviconForPageURL(gurl, &favicon)) + return testing::AssertionFailure() << "Favicon is missing."; + if (favicon->size() != bytes.size()) + return testing::AssertionFailure() << "Favicon sizes don't match."; + for (size_t i = 0; i < favicon->size(); ++i) { + if (bytes[i] != *(favicon->front() + i)) + return testing::AssertionFailure() << "Favicon data doesn't match."; + } + return testing::AssertionSuccess(); +} + +testing::AssertionResult SyncFaviconCacheTest::VerifyLocalIcons( + const std::vector<int>& expected_icons) { + std::vector<TestFaviconData> expected_custom_icons; + for (size_t i = 0; i < expected_icons.size(); ++i) { + expected_custom_icons.push_back(BuildFaviconData(expected_icons[i])); + } + return VerifyLocalCustomIcons(expected_custom_icons); +} + + +testing::AssertionResult SyncFaviconCacheTest::VerifyLocalCustomIcons( + const std::vector<TestFaviconData>& expected_custom_icons) { + syncer::SyncDataList image_data_list = + cache()->GetAllSyncData(syncer::FAVICON_IMAGES); + syncer::SyncDataList tracking_data_list = + cache()->GetAllSyncData(syncer::FAVICON_TRACKING); + if (expected_custom_icons.size() > image_data_list.size() || + expected_custom_icons.size() > tracking_data_list.size()) + return testing::AssertionFailure() << "Number of icons doesn't match."; + for (size_t i = 0; i < expected_custom_icons.size(); ++i) { + const TestFaviconData& test_data = expected_custom_icons[i]; + // Find the test data in the data lists. Assume that both lists have the + // same ordering, which may not match the |expected_custom_icons| ordering. + bool found_match = false; + for (size_t j = 0; j < image_data_list.size(); ++j) { + if (image_data_list[j].GetTitle() != test_data.icon_url.spec()) + continue; + found_match = true; + const sync_pb::FaviconImageSpecifics& image_specifics = + image_data_list[j].GetSpecifics().favicon_image(); + sync_pb::FaviconImageSpecifics expected_image_specifics; + FillImageSpecifics(test_data, &expected_image_specifics); + if (image_specifics.SerializeAsString() != + expected_image_specifics.SerializeAsString()) { + return testing::AssertionFailure() << "Image data doesn't match."; + } + const sync_pb::FaviconTrackingSpecifics& tracking_specifics = + tracking_data_list[j].GetSpecifics().favicon_tracking(); + sync_pb::FaviconTrackingSpecifics expected_tracking_specifics; + FillTrackingSpecifics(test_data, &expected_tracking_specifics); + if (tracking_specifics.SerializeAsString() != + expected_tracking_specifics.SerializeAsString()) { + return testing::AssertionFailure() << "Tracking data doesn't match."; + } + } + if (!found_match) + return testing::AssertionFailure() << "Could not find favicon."; + } + return testing::AssertionSuccess(); +} + +scoped_ptr<syncer::SyncChangeProcessor> +SyncFaviconCacheTest::CreateAndPassProcessor() { + return scoped_ptr<syncer::SyncChangeProcessor>( + new SyncChangeProcessorDelegate(sync_processor_.get())); +} + +scoped_ptr<syncer::SyncErrorFactory> SyncFaviconCacheTest:: + CreateAndPassSyncErrorFactory() { + return scoped_ptr<syncer::SyncErrorFactory>( + new syncer::SyncErrorFactoryMock()); +} + // A freshly constructed cache should be empty. TEST_F(SyncFaviconCacheTest, Empty) { EXPECT_EQ(0U, GetFaviconCount()); @@ -46,7 +391,7 @@ TEST_F(SyncFaviconCacheTest, ReceiveSyncFavicon) { std::string fav_url = "http://www.google.com/favicon.ico"; std::string bytes = "bytes"; EXPECT_EQ(0U, GetFaviconCount()); - cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes); + cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes, 0); EXPECT_EQ(1U, GetFaviconCount()); EXPECT_TRUE(ExpectFaviconEquals(page_url, bytes)); } @@ -56,12 +401,15 @@ TEST_F(SyncFaviconCacheTest, ReceiveEmptySyncFavicon) { std::string fav_url = "http://www.google.com/favicon.ico"; std::string bytes = "bytes"; EXPECT_EQ(0U, GetFaviconCount()); - cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), std::string()); + cache()->OnReceivedSyncFavicon(GURL(page_url), + GURL(fav_url), + std::string(), + 0); EXPECT_EQ(0U, GetFaviconCount()); EXPECT_FALSE(ExpectFaviconEquals(page_url, std::string())); // Then receive the actual favicon. - cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes); + cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes, 0); EXPECT_EQ(1U, GetFaviconCount()); EXPECT_TRUE(ExpectFaviconEquals(page_url, bytes)); } @@ -72,13 +420,13 @@ TEST_F(SyncFaviconCacheTest, ReceiveUpdatedSyncFavicon) { std::string bytes = "bytes"; std::string bytes2 = "bytes2"; EXPECT_EQ(0U, GetFaviconCount()); - cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes); + cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes, 0); EXPECT_EQ(1U, GetFaviconCount()); EXPECT_TRUE(ExpectFaviconEquals(page_url, bytes)); // The cache should not update existing favicons from tab sync favicons // (which can be reassociated several times). - cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes2); + cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes2, 0); EXPECT_EQ(1U, GetFaviconCount()); EXPECT_TRUE(ExpectFaviconEquals(page_url, bytes)); EXPECT_FALSE(ExpectFaviconEquals(page_url, bytes2)); @@ -90,14 +438,562 @@ TEST_F(SyncFaviconCacheTest, MultipleMappings) { std::string fav_url = "http://www.google.com/favicon.ico"; std::string bytes = "bytes"; EXPECT_EQ(0U, GetFaviconCount()); - cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes); + cache()->OnReceivedSyncFavicon(GURL(page_url), GURL(fav_url), bytes, 0); EXPECT_EQ(1U, GetFaviconCount()); EXPECT_TRUE(ExpectFaviconEquals(page_url, bytes)); // Map another page to the same favicon. They should share the same data. - cache()->OnReceivedSyncFavicon(GURL(page2_url), GURL(fav_url), bytes); + cache()->OnReceivedSyncFavicon(GURL(page2_url), GURL(fav_url), bytes, 0); EXPECT_EQ(1U, GetFaviconCount()); EXPECT_TRUE(ExpectFaviconEquals(page2_url, bytes)); } +TEST_F(SyncFaviconCacheTest, SyncEmpty) { + syncer::SyncMergeResult merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_IMAGES, + syncer::SyncDataList(), + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + + EXPECT_EQ(0U, cache()->GetAllSyncData(syncer::FAVICON_IMAGES).size()); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(0, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(0, merge_result.num_items_before_association()); + EXPECT_EQ(0, merge_result.num_items_after_association()); + + merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_TRACKING, + syncer::SyncDataList(), + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + + EXPECT_EQ(0U, cache()->GetAllSyncData(syncer::FAVICON_TRACKING).size()); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(0, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(0, merge_result.num_items_before_association()); + EXPECT_EQ(0, merge_result.num_items_after_association()); +} + +// Setting up sync with existing local favicons should push those favicons into +// sync. +TEST_F(SyncFaviconCacheTest, SyncExistingLocal) { + std::vector<syncer::SyncChange::SyncChangeType> expected_change_types; + std::vector<int> expected_icons; + for (int i = 0; i < kTotalFavicons; ++i) { + TestFaviconData favicon = BuildFaviconData(i); + cache()->OnReceivedSyncFavicon(favicon.page_url, + favicon.icon_url, + favicon.image_16, + i); + expected_change_types.push_back(syncer::SyncChange::ACTION_ADD); + expected_icons.push_back(i); + } + + syncer::SyncMergeResult merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_IMAGES, + syncer::SyncDataList(), + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_IMAGES).size()); + syncer::SyncChangeList change_list = processor()->GetAndResetChangeList(); + EXPECT_TRUE(VerifyChanges(syncer::FAVICON_IMAGES, + expected_change_types, + expected_icons, + change_list)); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(0, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); + + merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_TRACKING, + syncer::SyncDataList(), + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_TRACKING).size()); + change_list = processor()->GetAndResetChangeList(); + EXPECT_TRUE(VerifyChanges(syncer::FAVICON_TRACKING, + expected_change_types, + expected_icons, + change_list)); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(0, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); +} + +// Setting up sync with existing sync data should load that data into the local +// cache. +TEST_F(SyncFaviconCacheTest, SyncExistingRemote) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + std::vector<int> expected_icons; + for (int i = 0; i < kTotalFavicons; ++i) { + expected_icons.push_back(i); + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(BuildFaviconData(i), + 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)); + } + + syncer::SyncMergeResult merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_IMAGES, + initial_image_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_IMAGES).size()); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_added()); + EXPECT_EQ(0, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(0, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); + + merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_TRACKING, + initial_tracking_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_TRACKING).size()); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); + + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); +} + +// Setting up sync with local data and sync data should merge the two image +// sets, with remote data having priority in case both exist. +TEST_F(SyncFaviconCacheTest, SyncMergesImages) { + // First go through and add local 16p favicons. + for (int i = 0; i < kTotalFavicons; ++i) { + TestFaviconData favicon = BuildFaviconData(i); + cache()->OnReceivedSyncFavicon(favicon.page_url, + favicon.icon_url, + favicon.image_16, + i); + } + + // Then go through and create the initial sync data, which does not have 16p + // favicons for the first half, and has custom 16p favicons for the second. + std::vector<syncer::SyncChange::SyncChangeType> expected_change_types; + std::vector<int> expected_icons; + std::vector<TestFaviconData> expected_data; + syncer::SyncDataList initial_image_data, initial_tracking_data; + for (int i = 0; i < kTotalFavicons; ++i) { + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + TestFaviconData test_data = BuildFaviconData(i); + if (i < kTotalFavicons/2) { + test_data.image_16 = std::string(); + expected_icons.push_back(i); + expected_change_types.push_back(syncer::SyncChange::ACTION_UPDATE); + } else { + test_data.image_16 += "custom"; + expected_data.push_back(test_data); + } + FillImageSpecifics(test_data, + image_specifics.mutable_favicon_image()); + + initial_image_data.push_back( + syncer::SyncData::CreateRemoteData(1, + image_specifics)); + FillTrackingSpecifics(test_data, + tracking_specifics.mutable_favicon_tracking()); + initial_tracking_data.push_back( + syncer::SyncData::CreateRemoteData(1, + tracking_specifics)); + } + + syncer::SyncMergeResult merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_IMAGES, + initial_image_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_IMAGES).size()); + syncer::SyncChangeList changes = processor()->GetAndResetChangeList(); + EXPECT_EQ((unsigned long)kTotalFavicons/2, changes.size()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); + + merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_TRACKING, + initial_tracking_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_TRACKING).size()); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); + + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); + ASSERT_TRUE(VerifyLocalCustomIcons(expected_data)); + ASSERT_TRUE(VerifyChanges(syncer::FAVICON_IMAGES, + expected_change_types, + expected_icons, + changes)); +} + +// Setting up sync with local data and sync data should merge the two tracking +// sets, such that the visit time is the most recent. +TEST_F(SyncFaviconCacheTest, SyncMergesTracking) { + // First go through and add local 16p favicons. + for (int i = 0; i < kTotalFavicons; ++i) { + TestFaviconData favicon = BuildFaviconData(i); + cache()->OnReceivedSyncFavicon(favicon.page_url, + favicon.icon_url, + favicon.image_16, + i); + } + + // Then go through and create the initial sync data, which for the first half + // the local has a newer visit, and for the second the remote does. + std::vector<syncer::SyncChange::SyncChangeType> expected_change_types; + std::vector<int> expected_icons; + std::vector<TestFaviconData> expected_data; + syncer::SyncDataList initial_image_data, initial_tracking_data; + for (int i = 0; i < kTotalFavicons; ++i) { + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + TestFaviconData test_data = BuildFaviconData(i); + if (i < kTotalFavicons/2) { + test_data.last_visit_time = i-1; + expected_icons.push_back(i); + expected_change_types.push_back(syncer::SyncChange::ACTION_UPDATE); + } else { + test_data.last_visit_time = i+1; + expected_data.push_back(test_data); + } + FillImageSpecifics(test_data, + image_specifics.mutable_favicon_image()); + + initial_image_data.push_back( + syncer::SyncData::CreateRemoteData(1, + image_specifics)); + FillTrackingSpecifics(test_data, + tracking_specifics.mutable_favicon_tracking()); + initial_tracking_data.push_back( + syncer::SyncData::CreateRemoteData(1, + tracking_specifics)); + } + + syncer::SyncMergeResult merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_IMAGES, + initial_image_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_IMAGES).size()); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); + + merge_result = + cache()->MergeDataAndStartSyncing(syncer::FAVICON_TRACKING, + initial_tracking_data, + CreateAndPassProcessor(), + CreateAndPassSyncErrorFactory()); + EXPECT_EQ((unsigned long)kTotalFavicons, + cache()->GetAllSyncData(syncer::FAVICON_TRACKING).size()); + syncer::SyncChangeList changes = processor()->GetAndResetChangeList(); + EXPECT_EQ((unsigned long)kTotalFavicons/2, changes.size()); + EXPECT_EQ(0, merge_result.num_items_added()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_modified()); + EXPECT_EQ(0, merge_result.num_items_deleted()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_before_association()); + EXPECT_EQ(kTotalFavicons, merge_result.num_items_after_association()); + + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); + ASSERT_TRUE(VerifyLocalCustomIcons(expected_data)); + ASSERT_TRUE(VerifyChanges(syncer::FAVICON_TRACKING, + expected_change_types, + expected_icons, + changes)); +} + +// Receiving old icons (missing image data) should result in pushing the new +// merged icons back to the remote syncer. +TEST_F(SyncFaviconCacheTest, ReceiveStaleImages) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + syncer::SyncChangeList stale_changes; + std::vector<int> expected_icons; + std::vector<syncer::SyncChange::SyncChangeType> expected_change_types; + for (int i = 0; i < kTotalFavicons; ++i) { + expected_icons.push_back(i); + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(BuildFaviconData(i), + image_specifics.mutable_favicon_image()); + initial_image_data.push_back( + syncer::SyncData::CreateRemoteData(1, + image_specifics)); + expected_change_types.push_back(syncer::SyncChange::ACTION_UPDATE); + image_specifics.mutable_favicon_image()->clear_favicon_web(); + stale_changes.push_back( + syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + 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); + + // Now receive the same icons as an update, but with missing image data. + cache()->ProcessSyncChanges(FROM_HERE, stale_changes); + syncer::SyncChangeList changes = processor()->GetAndResetChangeList(); + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); + ASSERT_EQ((unsigned long)kTotalFavicons, changes.size()); + ASSERT_TRUE(VerifyChanges(syncer::FAVICON_IMAGES, + expected_change_types, + expected_icons, + changes)); +} + +// New icons should be added locally without pushing anything back to the +// remote syncer. +TEST_F(SyncFaviconCacheTest, ReceiveNewImages) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + syncer::SyncChangeList new_changes; + std::vector<int> expected_icons; + for (int i = 0; i < kTotalFavicons; ++i) { + expected_icons.push_back(i); + TestFaviconData test_data = BuildFaviconData(i); + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(test_data, + image_specifics.mutable_favicon_image()); + new_changes.push_back( + syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + syncer::SyncData::CreateRemoteData(1, image_specifics))); + image_specifics.mutable_favicon_image()->mutable_favicon_web()-> + mutable_favicon()->append("old"); + 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); + + // Now receive the new icons as an update. + cache()->ProcessSyncChanges(FROM_HERE, new_changes); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); +} + +// Recieving the same icons as the local data should have no effect. +TEST_F(SyncFaviconCacheTest, ReceiveSameImages) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + syncer::SyncChangeList same_changes; + std::vector<int> expected_icons; + for (int i = 0; i < kTotalFavicons; ++i) { + expected_icons.push_back(i); + TestFaviconData test_data = BuildFaviconData(i); + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(test_data, + image_specifics.mutable_favicon_image()); + same_changes.push_back( + syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + syncer::SyncData::CreateRemoteData(1, image_specifics))); + 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); + + // Now receive the new icons as an update. + cache()->ProcessSyncChanges(FROM_HERE, same_changes); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); +} + +// Receiving stale tracking (old visit times) should result in pushing back +// the newer visit times to the remote syncer. +TEST_F(SyncFaviconCacheTest, ReceiveStaleTracking) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + syncer::SyncChangeList stale_changes; + std::vector<int> expected_icons; + std::vector<syncer::SyncChange::SyncChangeType> expected_change_types; + for (int i = 0; i < kTotalFavicons; ++i) { + expected_icons.push_back(i); + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(BuildFaviconData(i), + image_specifics.mutable_favicon_image()); + initial_image_data.push_back( + syncer::SyncData::CreateRemoteData(1, + image_specifics)); + expected_change_types.push_back(syncer::SyncChange::ACTION_UPDATE); + FillTrackingSpecifics(BuildFaviconData(i), + tracking_specifics.mutable_favicon_tracking()); + initial_tracking_data.push_back( + syncer::SyncData::CreateRemoteData(1, + tracking_specifics)); + tracking_specifics.mutable_favicon_tracking()->set_last_visit_time_ms(-1); + stale_changes.push_back( + syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + syncer::SyncData::CreateRemoteData(1, tracking_specifics))); + } + + SetUpInitialSync(initial_image_data, initial_tracking_data); + + // Now receive the same icons as an update, but with missing image data. + cache()->ProcessSyncChanges(FROM_HERE, stale_changes); + syncer::SyncChangeList changes = processor()->GetAndResetChangeList(); + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); + ASSERT_EQ((unsigned long)kTotalFavicons, changes.size()); + ASSERT_TRUE(VerifyChanges(syncer::FAVICON_TRACKING, + expected_change_types, + expected_icons, + changes)); +} + +// New tracking information should be added locally without pushing anything +// back to the remote syncer. +TEST_F(SyncFaviconCacheTest, ReceiveNewTracking) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + syncer::SyncChangeList new_changes; + std::vector<int> expected_icons; + // We start from one here so that we don't have to deal with a -1 visit time. + for (int i = 1; i <= kTotalFavicons; ++i) { + expected_icons.push_back(i); + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(BuildFaviconData(i), + image_specifics.mutable_favicon_image()); + initial_image_data.push_back( + syncer::SyncData::CreateRemoteData(1, + image_specifics)); + FillTrackingSpecifics(BuildFaviconData(i), + tracking_specifics.mutable_favicon_tracking()); + new_changes.push_back( + syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + syncer::SyncData::CreateRemoteData(1, tracking_specifics))); + tracking_specifics.mutable_favicon_tracking()->set_last_visit_time_ms(i-1); + initial_tracking_data.push_back( + syncer::SyncData::CreateRemoteData(1, + tracking_specifics)); + } + + SetUpInitialSync(initial_image_data, initial_tracking_data); + + // Now receive the new icons as an update. + cache()->ProcessSyncChanges(FROM_HERE, new_changes); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); +} + +// Receiving the same tracking information as the local data should have no +// effect. +TEST_F(SyncFaviconCacheTest, ReceiveSameTracking) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + syncer::SyncChangeList same_changes; + std::vector<int> expected_icons; + for (int i = 0; i < kTotalFavicons; ++i) { + expected_icons.push_back(i); + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(BuildFaviconData(i), + 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)); + same_changes.push_back( + syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + syncer::SyncData::CreateRemoteData(1, tracking_specifics))); + } + + SetUpInitialSync(initial_image_data, initial_tracking_data); + + // Now receive the new icons as an update. + cache()->ProcessSyncChanges(FROM_HERE, same_changes); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + ASSERT_TRUE(VerifyLocalIcons(expected_icons)); +} + +// Verify we can delete favicons after setting up sync. +TEST_F(SyncFaviconCacheTest, DeleteFavicons) { + syncer::SyncDataList initial_image_data, initial_tracking_data; + syncer::SyncChangeList deletions; + for (int i = 0; i < kTotalFavicons; ++i) { + sync_pb::EntitySpecifics image_specifics, tracking_specifics; + FillImageSpecifics(BuildFaviconData(i), + 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)); + deletions.push_back( + syncer::SyncChange( + FROM_HERE, + syncer::SyncChange::ACTION_DELETE, + syncer::SyncData::CreateRemoteData(1, tracking_specifics))); + } + + SetUpInitialSync(initial_image_data, initial_tracking_data); + + // Now receive the new icons as an update. + EXPECT_EQ((unsigned long)kTotalFavicons, GetFaviconCount()); + cache()->ProcessSyncChanges(FROM_HERE, deletions); + EXPECT_EQ(0U, processor()->GetAndResetChangeList().size()); + EXPECT_EQ(0U, GetFaviconCount()); +} + } // namespace browser_sync diff --git a/chrome/browser/sync/glue/session_model_associator.cc b/chrome/browser/sync/glue/session_model_associator.cc index 5b74fda..7904aca 100644 --- a/chrome/browser/sync/glue/session_model_associator.cc +++ b/chrome/browser/sync/glue/session_model_associator.cc @@ -900,7 +900,9 @@ void SessionModelAssociator::LoadForeignTabFavicon( const std::string& favicon_url = tab.navigation(i).favicon_url(); favicon_cache_.OnReceivedSyncFavicon(GURL(page_url), GURL(favicon_url), - std::string()); + std::string(), + syncer::TimeToProtoTime( + base::Time::Now())); } } @@ -929,7 +931,11 @@ void SessionModelAssociator::LoadForeignTabFavicon( const std::string& favicon = tab.favicon(); DVLOG(1) << "Storing synced favicon for url " << navigation_url.spec() << " with size " << favicon.size() << " bytes."; - favicon_cache_.OnReceivedSyncFavicon(navigation_url, favicon_source, favicon); + favicon_cache_.OnReceivedSyncFavicon(navigation_url, + favicon_source, + favicon, + syncer::TimeToProtoTime( + base::Time::Now())); } bool SessionModelAssociator::UpdateSyncModelDataFromClient( @@ -1113,7 +1119,7 @@ void SessionModelAssociator::QuitLoopForSubtleTesting() { } } -FaviconCache* SessionModelAssociator::GetFaviconCacheForTesting() { +FaviconCache* SessionModelAssociator::GetFaviconCache() { return &favicon_cache_; } diff --git a/chrome/browser/sync/glue/session_model_associator.h b/chrome/browser/sync/glue/session_model_associator.h index 0b3579f..b1c0f04 100644 --- a/chrome/browser/sync/glue/session_model_associator.h +++ b/chrome/browser/sync/glue/session_model_associator.h @@ -233,6 +233,8 @@ class SessionModelAssociator current_machine_tag_ = machine_tag; } + FaviconCache* GetFaviconCache(); + private: friend class SyncSessionModelAssociatorTest; FRIEND_TEST_ALL_PREFIXES(ProfileSyncServiceSessionTest, WriteSessionToNode); @@ -351,7 +353,6 @@ class SessionModelAssociator // For testing only. void QuitLoopForSubtleTesting(); - FaviconCache* GetFaviconCacheForTesting(); // Unique client tag. std::string current_machine_tag_; diff --git a/chrome/browser/sync/glue/session_model_associator_unittest.cc b/chrome/browser/sync/glue/session_model_associator_unittest.cc index fe931d6..1ad13c5 100644 --- a/chrome/browser/sync/glue/session_model_associator_unittest.cc +++ b/chrome/browser/sync/glue/session_model_associator_unittest.cc @@ -61,7 +61,7 @@ class SyncSessionModelAssociatorTest : public testing::Test { bool FaviconEquals(const GURL page_url, std::string expected_bytes) { - FaviconCache* cache = model_associator_.GetFaviconCacheForTesting(); + FaviconCache* cache = model_associator_.GetFaviconCache(); GURL gurl(page_url); scoped_refptr<base::RefCountedMemory> favicon; if (!cache->GetSyncedFaviconForPageURL(gurl, &favicon)) |