diff options
| -rw-r--r-- | chrome/browser/extensions/app_notification.cc | 46 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification.h | 18 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification_manager.cc | 422 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification_manager.h | 97 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification_manager_sync_unittest.cc | 588 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification_manager_unittest.cc | 18 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification_storage_unittest.cc | 10 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification_test_util.cc | 18 | ||||
| -rw-r--r-- | chrome/browser/extensions/app_notification_test_util.h | 9 | ||||
| -rw-r--r-- | chrome/browser/extensions/extension_app_api.cc | 5 | ||||
| -rw-r--r-- | chrome/chrome_tests.gypi | 1 | ||||
| -rw-r--r-- | chrome/common/chrome_notification_types.h | 4 |
12 files changed, 94 insertions, 1142 deletions
diff --git a/chrome/browser/extensions/app_notification.cc b/chrome/browser/extensions/app_notification.cc index 2dbae2b..f8e60e3 100644 --- a/chrome/browser/extensions/app_notification.cc +++ b/chrome/browser/extensions/app_notification.cc @@ -5,13 +5,15 @@ #include "chrome/browser/extensions/app_notification.h" #include "base/memory/scoped_ptr.h" -#include "chrome/common/guid.h" + +AppNotification::AppNotification(const std::string& title, + const std::string& body) + : title_(title), body_(body) {} + +AppNotification::~AppNotification() {} namespace { -const char* kIsLocalKey = "is_local"; -const char* kGuidKey = "guid"; -const char* kExtensionIdKey = "extension_id"; const char* kTitleKey = "title"; const char* kBodyKey = "body"; const char* kLinkUrlKey = "link_url"; @@ -19,27 +21,8 @@ const char* kLinkTextKey = "link_text"; } // namespace -AppNotification::AppNotification(bool is_local, - const std::string& guid, - const std::string& extension_id, - const std::string& title, - const std::string& body) - : is_local_(is_local), - extension_id_(extension_id), - title_(title), - body_(body) { - guid_ = guid.empty() ? guid::GenerateGUID() : guid; -} - -AppNotification::~AppNotification() {} - void AppNotification::ToDictionaryValue(DictionaryValue* result) { CHECK(result); - result->SetBoolean(kIsLocalKey, is_local_); - if (!guid_.empty()) - result->SetString(kGuidKey, guid_); - if (!extension_id_.empty()) - result->SetString(kExtensionIdKey, extension_id_); if (!title_.empty()) result->SetString(kTitleKey, title_); if (!body_.empty()) @@ -53,17 +36,9 @@ void AppNotification::ToDictionaryValue(DictionaryValue* result) { // static AppNotification* AppNotification::FromDictionaryValue( const DictionaryValue& value) { - scoped_ptr<AppNotification> result(new AppNotification(true, "", "", "", "")); - if (value.HasKey(kIsLocalKey) && !value.GetBoolean( - kIsLocalKey, &result->is_local_)) { - return NULL; - } - if (value.HasKey(kGuidKey) && !value.GetString(kGuidKey, &result->guid_)) - return NULL; - if (value.HasKey(kExtensionIdKey) && - !value.GetString(kExtensionIdKey, &result->extension_id_)) - return NULL; + scoped_ptr<AppNotification> result(new AppNotification("", "")); + if (value.HasKey(kTitleKey) && !value.GetString(kTitleKey, &result->title_)) return NULL; if (value.HasKey(kBodyKey) && !value.GetString(kBodyKey, &result->body_)) @@ -85,10 +60,7 @@ AppNotification* AppNotification::FromDictionaryValue( } bool AppNotification::Equals(const AppNotification& other) const { - return (is_local_ == other.is_local_ && - guid_ == other.guid_ && - extension_id_ == other.extension_id_ && - title_ == other.title_ && + return (title_ == other.title_ && body_ == other.body_ && link_url_ == other.link_url_ && link_text_ == other.link_text_); diff --git a/chrome/browser/extensions/app_notification.h b/chrome/browser/extensions/app_notification.h index 4a9deb6..8fd7743 100644 --- a/chrome/browser/extensions/app_notification.h +++ b/chrome/browser/extensions/app_notification.h @@ -17,14 +17,7 @@ // displayed on the New Tab Page. class AppNotification { public: - // Creates an instance with the given properties. - // If |is_local| is true, notification is not synced. - // If |guid| is empty, a new guid is automatically created. - AppNotification(bool is_local, - const std::string& guid, - const std::string& extension_id, - const std::string& title, - const std::string& body); + AppNotification(const std::string& title, const std::string& body); ~AppNotification(); // Setters for optional properties. @@ -32,9 +25,6 @@ class AppNotification { void set_link_text(const std::string& text) { link_text_ = text; } // Accessors. - bool is_local() const { return is_local_; } - const std::string& guid() const { return guid_; } - const std::string& extension_id() const { return extension_id_; } const std::string& title() const { return title_; } const std::string& body() const { return body_; } const GURL& link_url() const { return link_url_; } @@ -51,12 +41,6 @@ class AppNotification { // If you add to the list of data members, make sure to add appropriate checks // to the Equals and {To,From}DictionaryValue methods, keeping in mind // backwards compatibility. - - // Whether notification is local only, which means it is not synced - // across machines. - bool is_local_; - std::string guid_; - std::string extension_id_; std::string title_; std::string body_; GURL link_url_; diff --git a/chrome/browser/extensions/app_notification_manager.cc b/chrome/browser/extensions/app_notification_manager.cc index 53b753d4..01a2bbc 100644 --- a/chrome/browser/extensions/app_notification_manager.cc +++ b/chrome/browser/extensions/app_notification_manager.cc @@ -4,75 +4,31 @@ #include "chrome/browser/extensions/app_notification_manager.h" -#include "base/auto_reset.h" #include "base/bind.h" #include "base/file_path.h" -#include "base/location.h" #include "base/stl_util.h" #include "chrome/browser/profiles/profile.h" -#include "chrome/browser/sync/protocol/app_notification_specifics.pb.h" #include "chrome/common/chrome_notification_types.h" #include "chrome/common/extensions/extension.h" #include "content/browser/browser_thread.h" #include "content/common/notification_service.h" -typedef std::map<std::string, SyncData> SyncDataMap; +AppNotificationManager::AppNotificationManager(Profile* profile) + : profile_(profile) { + registrar_.Add(this, + chrome::NOTIFICATION_EXTENSION_UNINSTALLED, + Source<Profile>(profile_)); +} namespace { -class GuidComparator - : public std::binary_function<linked_ptr<AppNotification>, - std::string, - bool> { - public: - bool operator() (linked_ptr<AppNotification> notif, - const std::string& guid) const { - return notif->guid() == guid; - } -}; - void DeleteStorageOnFileThread(AppNotificationStorage* storage) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); delete storage; } -const AppNotification* FindByGuid(const AppNotificationList& list, - const std::string& guid) { - AppNotificationList::const_iterator iter = std::find_if( - list.begin(), list.end(), std::bind2nd(GuidComparator(), guid)); - return iter == list.end() ? NULL : iter->get(); -} - -void RemoveByGuid(AppNotificationList* list, const std::string& guid) { - if (!list) - return; - - AppNotificationList::iterator iter = std::find_if( - list->begin(), list->end(), std::bind2nd(GuidComparator(), guid)); - if (iter != list->end()) - list->erase(iter); -} - -void PopulateGuidToSyncDataMap(const SyncDataList& sync_data, - SyncDataMap* data_map) { - for (SyncDataList::const_iterator iter = sync_data.begin(); - iter != sync_data.end(); ++iter) { - (*data_map)[iter->GetSpecifics().GetExtension( - sync_pb::app_notification).guid()] = *iter; - } -} } // namespace -AppNotificationManager::AppNotificationManager(Profile* profile) - : profile_(profile), - sync_processor_(NULL), - models_associated_(false), - processing_syncer_changes_(false) { - registrar_.Add(this, - chrome::NOTIFICATION_EXTENSION_UNINSTALLED, - Source<Profile>(profile_)); -} - AppNotificationManager::~AppNotificationManager() { // Post a task to delete our storage on the file thread. BrowserThread::PostTask( @@ -86,79 +42,59 @@ void AppNotificationManager::Init() { BrowserThread::PostTask( BrowserThread::FILE, FROM_HERE, - base::Bind(&AppNotificationManager::LoadOnFileThread, - this, storage_path)); + base::Bind( + &AppNotificationManager::LoadOnFileThread, this, storage_path)); } -bool AppNotificationManager::Add(AppNotification* item) { - // Use a scoped_ptr to delete in error paths. - scoped_ptr<AppNotification> scoped_item(item); - if (!loaded()) - return false; - const std::string& extension_id = item->extension_id(); - AppNotificationList& list = GetAllInternal(extension_id); - linked_ptr<AppNotification> linked_item(scoped_item.release()); - list.push_back(linked_ptr<AppNotification>(linked_item)); +void AppNotificationManager::Add(const std::string& extension_id, + AppNotification* item) { + NotificationMap::iterator found = notifications_.find(extension_id); + if (found == notifications_.end()) { + notifications_[extension_id] = AppNotificationList(); + found = notifications_.find(extension_id); + } + CHECK(found != notifications_.end()); + AppNotificationList& list = (*found).second; + list.push_back(linked_ptr<AppNotification>(item)); - SyncAddChange(*linked_item); + if (!storage_.get()) + return; - if (storage_.get()) { - BrowserThread::PostTask( - BrowserThread::FILE, - FROM_HERE, - base::Bind(&AppNotificationManager::SaveOnFileThread, - this, extension_id, list)); - } - return true; + BrowserThread::PostTask( + BrowserThread::FILE, + FROM_HERE, + base::Bind( + &AppNotificationManager::SaveOnFileThread, this, extension_id, list)); } const AppNotificationList* AppNotificationManager::GetAll( - const std::string& extension_id) const { - if (!loaded()) - return NULL; - if (ContainsKey(*notifications_, extension_id)) - return &((*notifications_)[extension_id]); - return NULL; -} - -AppNotificationList& AppNotificationManager::GetAllInternal( const std::string& extension_id) { - NotificationMap::iterator found = notifications_->find(extension_id); - if (found == notifications_->end()) { - (*notifications_)[extension_id] = AppNotificationList(); - found = notifications_->find(extension_id); - } - CHECK(found != notifications_->end()); - return found->second; + if (ContainsKey(notifications_, extension_id)) + return ¬ifications_[extension_id]; + return NULL; } const AppNotification* AppNotificationManager::GetLast( const std::string& extension_id) { - if (!loaded()) - return NULL; - NotificationMap::iterator found = notifications_->find(extension_id); - if (found == notifications_->end()) + NotificationMap::iterator found = notifications_.find(extension_id); + if (found == notifications_.end()) return NULL; const AppNotificationList& list = found->second; return list.rbegin()->get(); } void AppNotificationManager::ClearAll(const std::string& extension_id) { - if (!loaded()) - return; - NotificationMap::iterator found = notifications_->find(extension_id); - if (found != notifications_->end()) { - SyncClearAllChange(found->second); - notifications_->erase(found); - } + NotificationMap::iterator found = notifications_.find(extension_id); + if (found != notifications_.end()) + notifications_.erase(found); - if (storage_.get()) { - BrowserThread::PostTask( - BrowserThread::FILE, - FROM_HERE, - base::Bind(&AppNotificationManager::DeleteOnFileThread, - this, extension_id)); - } + if (!storage_.get()) + return; + BrowserThread::PostTask( + BrowserThread::FILE, + FROM_HERE, + base::Bind( + &AppNotificationManager::DeleteOnFileThread, this, extension_id)); } void AppNotificationManager::Observe(int type, @@ -170,53 +106,43 @@ void AppNotificationManager::Observe(int type, void AppNotificationManager::LoadOnFileThread(const FilePath& storage_path) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - DCHECK(!loaded()); storage_.reset(AppNotificationStorage::Create(storage_path)); if (!storage_.get()) return; - scoped_ptr<NotificationMap> result(new NotificationMap()); + NotificationMap result; std::set<std::string> ids; if (!storage_->GetExtensionIds(&ids)) return; std::set<std::string>::const_iterator i; for (i = ids.begin(); i != ids.end(); ++i) { const std::string& id = *i; - AppNotificationList& list = (*result)[id]; + AppNotificationList& list = result[id]; if (!storage_->Get(id, &list)) - result->erase(id); + result.erase(id); } BrowserThread::PostTask( BrowserThread::UI, FROM_HERE, - base::Bind(&AppNotificationManager::HandleLoadResults, - this, result.release())); + base::Bind(&AppNotificationManager::HandleLoadResults, this, result)); } -void AppNotificationManager::HandleLoadResults(NotificationMap* map) { +void AppNotificationManager::HandleLoadResults(const NotificationMap& map) { CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(map); - DCHECK(!loaded()); - notifications_.reset(map); - - // Generate STATE_CHAGNED notifications for extensions that have at - // least one notification loaded. NotificationMap::const_iterator i; - for (i = map->begin(); i != map->end(); ++i) { + for (i = map.begin(); i != map.end(); ++i) { const std::string& id = i->first; - if (i->second.empty()) + const AppNotificationList& list = i->second; + if (list.empty()) continue; + notifications_[id].insert(notifications_[id].begin(), + list.begin(), + list.end()); NotificationService::current()->Notify( chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED, Source<Profile>(profile_), Details<const std::string>(&id)); } - - // Generate MANAGER_LOADED notification. - NotificationService::current()->Notify( - chrome::NOTIFICATION_APP_NOTIFICATION_MANAGER_LOADED, - Source<AppNotificationManager>(this), - NotificationService::NoDetails()); } void AppNotificationManager::SaveOnFileThread(const std::string& extension_id, @@ -230,249 +156,3 @@ void AppNotificationManager::DeleteOnFileThread( CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); storage_->Delete(extension_id); } - -SyncDataList AppNotificationManager::GetAllSyncData( - syncable::ModelType type) const { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(loaded()); - DCHECK_EQ(syncable::APP_NOTIFICATIONS, type); - SyncDataList data; - for (NotificationMap::const_iterator iter = notifications_->begin(); - iter != notifications_->end(); ++iter) { - - // Skip local notifications since they should not be synced. - const AppNotificationList list = (*iter).second; - for (AppNotificationList::const_iterator list_iter = list.begin(); - list_iter != list.end(); ++list_iter) { - const AppNotification* notification = (*list_iter).get(); - if (notification->is_local()) { - continue; - } - data.push_back(CreateSyncDataFromNotification(*notification)); - } - } - - return data; -} - -SyncError AppNotificationManager::ProcessSyncChanges( - const tracked_objects::Location& from_here, - const SyncChangeList& change_list) { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - DCHECK(loaded()); - if (!models_associated_) - return SyncError(FROM_HERE, "Models not yet associated.", - syncable::APP_NOTIFICATIONS); - - AutoReset<bool> processing_changes(&processing_syncer_changes_, true); - - SyncError error; - for (SyncChangeList::const_iterator iter = change_list.begin(); - iter != change_list.end(); ++iter) { - SyncData sync_data = iter->sync_data(); - DCHECK_EQ(syncable::APP_NOTIFICATIONS, sync_data.GetDataType()); - SyncChange::SyncChangeType change_type = iter->change_type(); - - scoped_ptr<AppNotification> new_notif(CreateNotificationFromSyncData( - sync_data)); - if (!new_notif.get()) { - NOTREACHED() << "Failed to read notification."; - continue; - } - const AppNotification* existing_notif = GetNotification( - new_notif->extension_id(), new_notif->guid()); - if (existing_notif && existing_notif->is_local()) { - NOTREACHED() << "Matched with notification marked as local"; - error = SyncError(FROM_HERE, - "ProcessSyncChanges received a local only notification" + - SyncChange::ChangeTypeToString(change_type), - syncable::APP_NOTIFICATIONS); - continue; - } - - if (change_type == SyncChange::ACTION_ADD && !existing_notif) { - Add(new_notif.release()); - } else if (change_type == SyncChange::ACTION_DELETE && existing_notif) { - Remove(existing_notif->extension_id(), existing_notif->guid()); - } else { - // Something really unexpected happened. Either we received an - // ACTION_INVALID, or Sync is in a crazy state: - // - Trying to UPDATE: notifications are immutable. - // . Trying to DELETE a non-existent item. - // - Trying to ADD an item that already exists. - NOTREACHED() << "Unexpected sync change state."; - error = SyncError(FROM_HERE, "ProcessSyncChanges failed on ChangeType " + - SyncChange::ChangeTypeToString(change_type), - syncable::APP_NOTIFICATIONS); - continue; - } - } - - return error; -} - -SyncError AppNotificationManager::MergeDataAndStartSyncing( - syncable::ModelType type, - const SyncDataList& initial_sync_data, - SyncChangeProcessor* sync_processor) { - CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); - // AppNotificationDataTypeController ensures that modei is fully should before - // this method is called by waiting until the load notification is received - // from AppNotificationManager. - DCHECK(loaded()); - DCHECK_EQ(type, syncable::APP_NOTIFICATIONS); - DCHECK(!sync_processor_); - sync_processor_ = sync_processor; - - // We may add, or remove notifications here, so ensure we don't step on - // our own toes. - AutoReset<bool> processing_changes(&processing_syncer_changes_, true); - - SyncDataMap local_data_map; - PopulateGuidToSyncDataMap(GetAllSyncData(syncable::APP_NOTIFICATIONS), - &local_data_map); - - for (SyncDataList::const_iterator iter = initial_sync_data.begin(); - iter != initial_sync_data.end(); ++iter) { - const SyncData& sync_data = *iter; - DCHECK_EQ(syncable::APP_NOTIFICATIONS, sync_data.GetDataType()); - scoped_ptr<AppNotification> sync_notif(CreateNotificationFromSyncData( - sync_data)); - CHECK(sync_notif.get()); - const AppNotification* local_notif = GetNotification( - sync_notif->extension_id(), sync_notif->guid()); - if (local_notif) { - local_data_map.erase(sync_notif->guid()); - // Local notification should always match with sync notification as - // notifications are immutable. - if (local_notif->is_local() || !sync_notif->Equals(*local_notif)) { - return SyncError(FROM_HERE, - "MergeDataAndStartSyncing failed: local notification and sync " - "notification have same guid but different data.", - syncable::APP_NOTIFICATIONS); - } - } else { - // Sync model has a notification that local model does not, add it. - Add(sync_notif.release()); - } - } - - // TODO(munjal): crbug.com/10059. Work with Lingesh/Antony to resolve. - SyncChangeList new_changes; - for (SyncDataMap::const_iterator iter = local_data_map.begin(); - iter != local_data_map.end(); ++iter) { - new_changes.push_back(SyncChange(SyncChange::ACTION_ADD, iter->second)); - } - - SyncError error; - if (new_changes.size() > 0) - error = sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes); - models_associated_ = !error.IsSet(); - return error; -} - -void AppNotificationManager::StopSyncing(syncable::ModelType type) { - DCHECK_EQ(type, syncable::APP_NOTIFICATIONS); - models_associated_ = false; - sync_processor_ = NULL; -} - -void AppNotificationManager::SyncAddChange(const AppNotification& notif) { - // Skip if either: - // - Notification is marked as local. - // - Sync is not enabled by user. - // - Change is generated from within the manager. - if (notif.is_local() || !models_associated_ || processing_syncer_changes_) - return; - - // TODO(munjal): crbug.com/10059. Work with Lingesh/Antony to resolve. - - SyncChangeList changes; - SyncData sync_data = CreateSyncDataFromNotification(notif); - changes.push_back(SyncChange(SyncChange::ACTION_ADD, sync_data)); - sync_processor_->ProcessSyncChanges(FROM_HERE, changes); -} - -void AppNotificationManager::SyncClearAllChange( - const AppNotificationList& list) { - // Skip if either: - // - Sync is not enabled by user. - // - Change is generated from within the manager. - if (!models_associated_ || processing_syncer_changes_) - return; - - SyncChangeList changes; - for (AppNotificationList::const_iterator iter = list.begin(); - iter != list.end(); ++iter) { - const AppNotification& notif = *iter->get(); - // Skip notifications marked as local. - if (notif.is_local()) - continue; - changes.push_back(SyncChange( - SyncChange::ACTION_DELETE, - CreateSyncDataFromNotification(notif))); - } - sync_processor_->ProcessSyncChanges(FROM_HERE, changes); -} - -const AppNotification* AppNotificationManager::GetNotification( - const std::string& extension_id, const std::string& guid) { - DCHECK(loaded()); - const AppNotificationList& list = GetAllInternal(extension_id); - return FindByGuid(list, guid); -} - -void AppNotificationManager::Remove(const std::string& extension_id, - const std::string& guid) { - DCHECK(loaded()); - AppNotificationList& list = GetAllInternal(extension_id); - RemoveByGuid(&list, guid); - - if (storage_.get()) { - BrowserThread::PostTask( - BrowserThread::FILE, - FROM_HERE, - base::Bind(&AppNotificationManager::SaveOnFileThread, - this, extension_id, list)); - } -} - -// static -SyncData AppNotificationManager::CreateSyncDataFromNotification( - const AppNotification& notification) { - DCHECK(!notification.is_local()); - sync_pb::EntitySpecifics specifics; - sync_pb::AppNotificationSpecifics* notif_specifics = - specifics.MutableExtension(sync_pb::app_notification); - notif_specifics->set_app_id(notification.extension_id()); - notif_specifics->set_body_text(notification.body()); - notif_specifics->set_guid(notification.guid()); - notif_specifics->set_link_text(notification.link_text()); - notif_specifics->set_link_url(notification.link_url().spec()); - notif_specifics->set_title(notification.title()); - - return SyncData::CreateLocalData( - notif_specifics->guid(), notif_specifics->app_id(), specifics); -} - -// static -AppNotification* AppNotificationManager::CreateNotificationFromSyncData( - const SyncData& sync_data) { - sync_pb::AppNotificationSpecifics specifics = - sync_data.GetSpecifics().GetExtension(sync_pb::app_notification); - - // Check for mandatory fields. - if (!specifics.has_app_id() || !specifics.has_guid() || - !specifics.has_title() || !specifics.has_body_text()) { - return NULL; - } - - AppNotification* notification = new AppNotification( - false, specifics.guid(), specifics.app_id(), - specifics.title(), specifics.body_text()); - if (specifics.has_link_text()) - notification->set_link_text(specifics.link_text()); - if (specifics.has_link_url()) - notification->set_link_url(GURL(specifics.link_url())); - return notification; -} diff --git a/chrome/browser/extensions/app_notification_manager.h b/chrome/browser/extensions/app_notification_manager.h index 7c37cc7..8148a08 100644 --- a/chrome/browser/extensions/app_notification_manager.h +++ b/chrome/browser/extensions/app_notification_manager.h @@ -9,15 +9,10 @@ #include <map> #include "base/compiler_specific.h" -#include "base/gtest_prod_util.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" -#include "base/synchronization/waitable_event.h" #include "chrome/browser/extensions/app_notification.h" #include "chrome/browser/extensions/app_notification_storage.h" -#include "chrome/browser/sync/api/sync_change.h" -#include "chrome/browser/sync/api/syncable_service.h" -#include "content/common/notification_details.h" #include "content/common/notification_observer.h" #include "content/common/notification_registrar.h" @@ -26,20 +21,17 @@ class Profile; // This class keeps track of notifications for installed apps. class AppNotificationManager : public base::RefCountedThreadSafe<AppNotificationManager>, - public NotificationObserver, - public SyncableService { + public NotificationObserver { public: explicit AppNotificationManager(Profile* profile); - virtual ~AppNotificationManager(); // Starts up the process of reading from persistent storage. void Init(); - // Returns whether add was succcessful. - // Takes ownership of |item| in all cases. - bool Add(AppNotification* item); + // Takes ownership of |notification|. + void Add(const std::string& extension_id, AppNotification* item); - const AppNotificationList* GetAll(const std::string& extension_id) const; + const AppNotificationList* GetAll(const std::string& extension_id); // Returns the most recently added notification for |extension_id| if there // are any, or NULL otherwise. @@ -53,105 +45,32 @@ class AppNotificationManager const NotificationSource& source, const NotificationDetails& details) OVERRIDE; - bool loaded() const { return notifications_.get() != NULL; } - - // SyncableService implementation. - - // Returns all syncable notifications from this model as SyncData. - virtual SyncDataList GetAllSyncData(syncable::ModelType type) const OVERRIDE; - // Process notifications related changes from Sync, merging them into - // our model. - virtual SyncError ProcessSyncChanges( - const tracked_objects::Location& from_here, - const SyncChangeList& change_list) OVERRIDE; - // Associate and merge sync data model with our data model. - virtual SyncError MergeDataAndStartSyncing( - syncable::ModelType type, - const SyncDataList& initial_sync_data, - SyncChangeProcessor* sync_processor) OVERRIDE; - virtual void StopSyncing(syncable::ModelType type) OVERRIDE; - private: - friend class AppNotificationManagerSyncTest; - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - NotificationToSyncDataToNotification); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - GetAllSyncDataNoLocal); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - GetAllSyncDataSomeLocal); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ModelAssocModelEmpty); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ModelAssocBothNonEmptyNoOverlap); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ModelAssocBothNonEmptySomeOverlap); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ModelAssocBothNonEmptyTitleMismatch); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ModelAssocBothNonEmptyMatchesLocal); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ProcessSyncChangesErrorModelAssocNotDone); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ProcessSyncChangesEmptyModel); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - StopSyncing); - FRIEND_TEST_ALL_PREFIXES(AppNotificationManagerSyncTest, - ClearAllGetsSynced); - friend class base::RefCountedThreadSafe<AppNotificationManager>; // Maps extension id to a list of notifications for that extension. typedef std::map<std::string, AppNotificationList> NotificationMap; + virtual ~AppNotificationManager(); + // Starts loading storage_ using |storage_path|. void LoadOnFileThread(const FilePath& storage_path); // Called on the UI thread to handle the loaded results from storage_. - void HandleLoadResults(NotificationMap* map); + void HandleLoadResults(const NotificationMap& map); void SaveOnFileThread(const std::string& extension_id, const AppNotificationList& list); void DeleteOnFileThread(const std::string& extension_id); - // Gets notficiations for a given extension id. - AppNotificationList& GetAllInternal(const std::string& extension_id); - - // Removes the notification with given extension id and given guid. - void Remove(const std::string& extension_id, const std::string& guid); - - // Returns the notification for the given |extension_id| and |guid|, - // NULL if no such notification exists. - const AppNotification* GetNotification(const std::string& extension_id, - const std::string& guid); - - // Sends a change to syncer to add the given notification. - void SyncAddChange(const AppNotification& notif); - - // Sends changes to syncer to remove all notifications in the given list. - void SyncClearAllChange(const AppNotificationList& list); - - // Converters from AppNotification to SyncData and vice versa. - static SyncData CreateSyncDataFromNotification( - const AppNotification& notification); - static AppNotification* CreateNotificationFromSyncData( - const SyncData& sync_data); - Profile* profile_; NotificationRegistrar registrar_; - scoped_ptr<NotificationMap> notifications_; + NotificationMap notifications_; // This should only be used on the FILE thread. scoped_ptr<AppNotificationStorage> storage_; - // Sync change processor we use to push all our changes. - SyncChangeProcessor* sync_processor_; - // Whether the sync model is associated with the local model. - // In other words, whether we are ready to apply sync changes. - bool models_associated_; - // Whether syncer changes are being processed right now. - bool processing_syncer_changes_; - DISALLOW_COPY_AND_ASSIGN(AppNotificationManager); }; diff --git a/chrome/browser/extensions/app_notification_manager_sync_unittest.cc b/chrome/browser/extensions/app_notification_manager_sync_unittest.cc deleted file mode 100644 index 41589d7..0000000 --- a/chrome/browser/extensions/app_notification_manager_sync_unittest.cc +++ /dev/null @@ -1,588 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "base/bind.h" -#include "base/memory/scoped_ptr.h" -#include "base/string_number_conversions.h" -#include "chrome/browser/extensions/app_notification_manager.h" -#include "chrome/browser/extensions/app_notification.h" -#include "chrome/browser/sync/protocol/app_notification_specifics.pb.h" -#include "chrome/test/base/testing_profile.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace { - -// Extract notification guid from SyncData. -std::string GetGuid(const SyncData& sync_data) { - return sync_data.GetSpecifics().GetExtension( - sync_pb::app_notification).guid(); -} - -// Dummy SyncChangeProcessor used to help review what SyncChanges are pushed -// back up to Sync. -class TestChangeProcessor : public SyncChangeProcessor { - public: - TestChangeProcessor() { } - virtual ~TestChangeProcessor() { } - - // Store a copy of all the changes passed in so we can examine them later. - virtual SyncError ProcessSyncChanges( - const tracked_objects::Location& from_here, - const SyncChangeList& change_list) { - // change_map_.erase(change_map_.begin(), change_map_.end()); - for (SyncChangeList::const_iterator iter = change_list.begin(); - iter != change_list.end(); ++iter) { - change_map_[GetGuid(iter->sync_data())] = *iter; - } - - return SyncError(); - } - - bool ContainsGuid(const std::string& guid) { - return change_map_.find(guid) != change_map_.end(); - } - - SyncChange GetChangeByGuid(const std::string& guid) { - DCHECK(ContainsGuid(guid)); - return change_map_[guid]; - } - - int change_list_size() { return change_map_.size(); } - - private: - // Track the changes received in ProcessSyncChanges. - std::map<std::string, SyncChange> change_map_; - - DISALLOW_COPY_AND_ASSIGN(TestChangeProcessor); -}; - -} // namespace - -class AppNotificationManagerSyncTest : public testing::Test { - public: - AppNotificationManagerSyncTest() - : ui_thread_(BrowserThread::UI, &ui_loop_), - file_thread_(BrowserThread::FILE) { } - - ~AppNotificationManagerSyncTest() { - model_ = NULL; - } - - virtual void SetUp() { - ASSERT_TRUE(BrowserThread::CurrentlyOn(BrowserThread::UI)); - file_thread_.Start(); - - ASSERT_TRUE(temp_dir_.CreateUniqueTempDir()); - profile_.reset(new TestingProfile(temp_dir_.path())); - model_ = new AppNotificationManager(profile_.get()); - model_->Init(); - - WaitForFileThread(); - ASSERT_TRUE(model_->loaded()); - } - - virtual void TearDown() { } - - static void PostQuitToUIThread() { - BrowserThread::PostTask(BrowserThread::UI, - FROM_HERE, - new MessageLoop::QuitTask()); - } - - static void WaitForFileThread() { - BrowserThread::PostTask(BrowserThread::FILE, - FROM_HERE, - base::Bind(&PostQuitToUIThread)); - MessageLoop::current()->Run(); - } - - AppNotificationManager* model() { return model_.get(); } - TestChangeProcessor* processor() { return &processor_; } - - // Creates a notification whose properties are set from the given integer. - static AppNotification* CreateNotification(int suffix) { - return CreateNotification(false, suffix); - } - static AppNotification* CreateNotification(bool is_local, int suffix) { - std::string s = base::IntToString(suffix); - return CreateNotification( - is_local, "guid" + s, "ext" + s, "text" + s, "body" + s, - "http://www.url" + s + ".com", "link text " + s); - } - static AppNotification* CreateNotification( - bool is_local, int suffix, const std::string& extension_id) { - std::string s = base::IntToString(suffix); - return CreateNotification( - is_local, "guid" + s, extension_id, "text" + s, "body" + s, - "http://www.url" + s + ".com", "link text " + s); - } - - // Creates a notification whose properties are set from the given integer - // but does not set link url and link text. - static AppNotification* CreateNotificationNoLink(int suffix) { - return CreateNotificationNoLink(false, suffix); - } - static AppNotification* CreateNotificationNoLink(bool is_local, int suffix) { - std::string s = base::IntToString(suffix); - return CreateNotification( - is_local, "guid" + s, "ext" + s, "text" + s, "body" + s, "", ""); - } - - // link_url and link_text are only set if the passed in values are not empty. - static AppNotification* CreateNotification(bool is_local, - const std::string& guid, - const std::string& extension_id, - const std::string& title, - const std::string& body, - const std::string& link_url, - const std::string& link_text) { - AppNotification* notif = new AppNotification( - is_local, guid, extension_id, title, body); - if (!link_url.empty()) - notif->set_link_url(GURL(link_url)); - if (!link_text.empty()) - notif->set_link_text(link_text); - return notif; - } - - static AppNotification* CreateNotification(const AppNotification& notif) { - return CreateNotification( - notif.is_local(), notif.guid(), notif.extension_id(), notif.title(), - notif.body(), notif.link_url().spec(), notif.link_text()); - } - - static SyncData CreateSyncData(int suffix) { - scoped_ptr<AppNotification> notif(CreateNotification(suffix)); - return AppNotificationManager::CreateSyncDataFromNotification(*notif); - } - static SyncData CreateSyncData(int suffix, const std::string& extension_id) { - scoped_ptr<AppNotification> notif( - CreateNotification(false, suffix, extension_id)); - return AppNotificationManager::CreateSyncDataFromNotification(*notif); - } - - // Helper to create SyncChange. Takes ownership of |notif|. - static SyncChange CreateSyncChange(SyncChange::SyncChangeType type, - AppNotification* notif) { - // Take control of notif to clean it up after we create data out of it. - scoped_ptr<AppNotification> scoped_notif(notif); - return SyncChange( - type, AppNotificationManager::CreateSyncDataFromNotification(*notif)); - } - - void AssertSyncChange(const SyncChange& change, - SyncChange::SyncChangeType type, - const AppNotification& notif) { - ASSERT_EQ(type, change.change_type()); - const AppNotification* notif2 = - AppNotificationManager::CreateNotificationFromSyncData( - change.sync_data()); - ASSERT_TRUE(notif.Equals(*notif2)); - } - - protected: - MessageLoop ui_loop_; - BrowserThread ui_thread_; - BrowserThread file_thread_; - - // We keep two TemplateURLServices to test syncing between them. - ScopedTempDir temp_dir_; - scoped_ptr<TestingProfile> profile_; - scoped_refptr<AppNotificationManager> model_; - - TestChangeProcessor processor_; - - DISALLOW_COPY_AND_ASSIGN(AppNotificationManagerSyncTest); -}; - -// Create an AppNotification, convert it to SyncData and convert it back. -TEST_F(AppNotificationManagerSyncTest, NotificationToSyncDataToNotification) { - { // Partial properties set. - scoped_ptr<AppNotification> notif1(CreateNotificationNoLink(1)); - SyncData sync_data = - AppNotificationManager::CreateSyncDataFromNotification(*notif1); - scoped_ptr<AppNotification> notif2( - AppNotificationManager::CreateNotificationFromSyncData(sync_data)); - EXPECT_TRUE(notif2.get()); - EXPECT_TRUE(notif1->Equals(*notif2)); - } - { // All properties set. - scoped_ptr<AppNotification> notif1(CreateNotification(1)); - SyncData sync_data = - AppNotificationManager::CreateSyncDataFromNotification(*notif1); - scoped_ptr<AppNotification> notif2( - AppNotificationManager::CreateNotificationFromSyncData(sync_data)); - EXPECT_TRUE(notif2.get()); - EXPECT_TRUE(notif1->Equals(*notif2)); - } -} - -// GetAllSyncData returns all notifications since none are marked local only. -TEST_F(AppNotificationManagerSyncTest, GetAllSyncDataNoLocal) { - model()->Add(CreateNotificationNoLink(1)); - model()->Add(CreateNotification(2)); - model()->Add(CreateNotification(3)); - SyncDataList all_sync_data = - model()->GetAllSyncData(syncable::APP_NOTIFICATIONS); - - EXPECT_EQ(3U, all_sync_data.size()); - - for (SyncDataList::const_iterator iter = all_sync_data.begin(); - iter != all_sync_data.end(); ++iter) { - scoped_ptr<AppNotification> notif1( - AppNotificationManager::CreateNotificationFromSyncData(*iter)); - - const std::string& guid = notif1->guid(); - const std::string& ext_id = notif1->extension_id(); - - const AppNotification* notif2 = model()->GetNotification(ext_id, guid); - ASSERT_TRUE(notif1->Equals(*notif2)); - } -} - -// GetAllSyncData should not return notifications marked as local only. -TEST_F(AppNotificationManagerSyncTest, GetAllSyncDataSomeLocal) { - model()->Add(CreateNotificationNoLink(1)); - model()->Add(CreateNotification(true, 2)); - model()->Add(CreateNotification(3)); - model()->Add(CreateNotification(true, 4)); - model()->Add(CreateNotification(5)); - SyncDataList all_sync_data = - model()->GetAllSyncData(syncable::APP_NOTIFICATIONS); - - EXPECT_EQ(3U, all_sync_data.size()); - - for (SyncDataList::const_iterator iter = all_sync_data.begin(); - iter != all_sync_data.end(); ++iter) { - scoped_ptr<AppNotification> notif1( - AppNotificationManager::CreateNotificationFromSyncData(*iter)); - const std::string& guid = notif1->guid(); - const std::string& ext_id = notif1->extension_id(); - - const AppNotification* notif2 = model()->GetNotification(ext_id, guid); - ASSERT_TRUE(notif1->Equals(*notif2)); - } -} - -// Model assocation: both models are empty. -TEST_F(AppNotificationManagerSyncTest, ModelAssocBothEmpty) { - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - SyncDataList(), // Empty. - processor()); - - EXPECT_EQ(0U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size()); - EXPECT_EQ(0, processor()->change_list_size()); -} - -// Model assocation: empty sync model and non-empty local model. -TEST_F(AppNotificationManagerSyncTest, ModelAssocModelEmpty) { - SyncDataList initial_data; - initial_data.push_back(CreateSyncData(1)); - initial_data.push_back(CreateSyncData(2)); - initial_data.push_back(CreateSyncData(3)); - initial_data.push_back(CreateSyncData(4)); - - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - initial_data, - processor()); - - EXPECT_EQ(4U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size()); - // Model should all of the initial sync data. - for (SyncDataList::const_iterator iter = initial_data.begin(); - iter != initial_data.end(); ++iter) { - scoped_ptr<AppNotification> notif1( - AppNotificationManager::CreateNotificationFromSyncData(*iter)); - const std::string& ext_id = notif1->extension_id(); - const std::string& guid = notif1->guid(); - const AppNotification* notif2 = model()->GetNotification(ext_id, guid); - EXPECT_TRUE(notif2); - EXPECT_TRUE(notif1->Equals(*notif2)); - } - - EXPECT_EQ(0, processor()->change_list_size()); -} - -// Model has some notifications, some of them are local only. Sync has some -// notifications. No items match up. -TEST_F(AppNotificationManagerSyncTest, ModelAssocBothNonEmptyNoOverlap) { - AppNotification* n1 = CreateNotification(1); - model()->Add(n1); - AppNotification* n2 = CreateNotification(true, 2); - model()->Add(n2); - AppNotification* n3 = CreateNotification(3); - model()->Add(n3); - - SyncDataList initial_data; - initial_data.push_back(CreateSyncData(4)); - initial_data.push_back(CreateSyncData(5)); - initial_data.push_back(CreateSyncData(6)); - initial_data.push_back(CreateSyncData(7)); - - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - initial_data, - processor()); - - EXPECT_EQ(6U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size()); - for (SyncDataList::const_iterator iter = initial_data.begin(); - iter != initial_data.end(); ++iter) { - scoped_ptr<AppNotification> notif1( - AppNotificationManager::CreateNotificationFromSyncData(*iter)); - const std::string& ext_id = notif1->extension_id(); - const std::string& guid = notif1->guid(); - const AppNotification* notif2 = model()->GetNotification(ext_id, guid); - EXPECT_TRUE(notif2); - EXPECT_TRUE(notif1->Equals(*notif2)); - } - EXPECT_TRUE(model()->GetNotification(n1->extension_id(), n1->guid())); - EXPECT_TRUE(model()->GetNotification(n2->extension_id(), n2->guid())); - EXPECT_TRUE(model()->GetNotification(n3->extension_id(), n3->guid())); - - EXPECT_EQ(2, processor()->change_list_size()); - EXPECT_TRUE(processor()->ContainsGuid(n1->guid())); - EXPECT_EQ(SyncChange::ACTION_ADD, - processor()->GetChangeByGuid(n1->guid()).change_type()); - EXPECT_FALSE(processor()->ContainsGuid(n2->guid())); - EXPECT_TRUE(processor()->ContainsGuid(n3->guid())); - EXPECT_EQ(SyncChange::ACTION_ADD, - processor()->GetChangeByGuid(n3->guid()).change_type()); -} - -// Model has some notifications, some of them are local only. Sync has some -// notifications. Some items match up. -TEST_F(AppNotificationManagerSyncTest, ModelAssocBothNonEmptySomeOverlap) { - AppNotification* n1 = CreateNotification(1); - model()->Add(n1); - AppNotification* n2 = CreateNotification(true, 2); - model()->Add(n2); - AppNotification* n3 = CreateNotification(3); - model()->Add(n3); - AppNotification* n4 = CreateNotification(4); - model()->Add(n4); - - SyncDataList initial_data; - initial_data.push_back(CreateSyncData(5)); - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n1)); - initial_data.push_back(CreateSyncData(6)); - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n4)); - initial_data.push_back(CreateSyncData(7)); - - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - initial_data, - processor()); - - EXPECT_EQ(6U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size()); - for (SyncDataList::const_iterator iter = initial_data.begin(); - iter != initial_data.end(); ++iter) { - scoped_ptr<AppNotification> notif1( - AppNotificationManager::CreateNotificationFromSyncData(*iter)); - const std::string& ext_id = notif1->extension_id(); - const std::string& guid = notif1->guid(); - const AppNotification* notif2 = model()->GetNotification(ext_id, guid); - EXPECT_TRUE(notif2); - EXPECT_TRUE(notif1->Equals(*notif2)); - } - EXPECT_TRUE(model()->GetNotification(n1->extension_id(), n1->guid())); - EXPECT_TRUE(model()->GetNotification(n2->extension_id(), n2->guid())); - EXPECT_TRUE(model()->GetNotification(n3->extension_id(), n3->guid())); - EXPECT_TRUE(model()->GetNotification(n4->extension_id(), n4->guid())); - - EXPECT_EQ(1, processor()->change_list_size()); - EXPECT_FALSE(processor()->ContainsGuid(n1->guid())); - EXPECT_FALSE(processor()->ContainsGuid(n2->guid())); - EXPECT_TRUE(processor()->ContainsGuid(n3->guid())); - EXPECT_EQ(SyncChange::ACTION_ADD, - processor()->GetChangeByGuid(n3->guid()).change_type()); - EXPECT_FALSE(processor()->ContainsGuid(n4->guid())); -} - -// When an item that matches up in model and sync is different, an error -// should be returned. -TEST_F(AppNotificationManagerSyncTest, ModelAssocBothNonEmptyTitleMismatch) { - AppNotification* n1 = CreateNotification(1); - model()->Add(n1); - AppNotification* n2 = CreateNotification(true, 2); - model()->Add(n2); - - SyncDataList initial_data; - initial_data.push_back(CreateSyncData(1)); - scoped_ptr<AppNotification> n1_a(CreateNotification( - n1->is_local(), n1->guid(), n1->extension_id(), - n1->title() + "_changed", // different title - n1->body(), n1->link_url().spec(), n1->link_text())); - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n1_a)); - - SyncError sync_error = model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - initial_data, - processor()); - - EXPECT_TRUE(sync_error.IsSet()); - EXPECT_EQ(syncable::APP_NOTIFICATIONS, sync_error.type()); - EXPECT_EQ(0, processor()->change_list_size()); -} - -// When an item in sync matches with a local-only item in model, an error -// should be returned. -TEST_F(AppNotificationManagerSyncTest, ModelAssocBothNonEmptyMatchesLocal) { - AppNotification* n1 = CreateNotification(1); - model()->Add(n1); - AppNotification* n2 = CreateNotification(true, 2); - model()->Add(n2); - - SyncDataList initial_data; - initial_data.push_back(CreateSyncData(1)); - scoped_ptr<AppNotification> n2_a(CreateNotification(2)); - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n2_a)); - - SyncError sync_error = model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - initial_data, - processor()); - - EXPECT_TRUE(sync_error.IsSet()); - EXPECT_EQ(syncable::APP_NOTIFICATIONS, sync_error.type()); - EXPECT_EQ(0, processor()->change_list_size()); -} - -// Process sync changes when model is empty. -TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesEmptyModel) { - // We initially have no data. - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - SyncDataList(), - processor()); - - // Set up a bunch of ADDs. - SyncChangeList changes; - changes.push_back(CreateSyncChange( - SyncChange::ACTION_ADD, CreateNotification(1))); - changes.push_back(CreateSyncChange( - SyncChange::ACTION_ADD, CreateNotification(2))); - changes.push_back(CreateSyncChange( - SyncChange::ACTION_ADD, CreateNotification(3))); - - model()->ProcessSyncChanges(FROM_HERE, changes); - - EXPECT_EQ(3U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size()); - EXPECT_EQ(0, processor()->change_list_size()); -} - -// Process sync changes when model is not empty. -TEST_F(AppNotificationManagerSyncTest, ProcessSyncChangesNonEmptyModel) { - AppNotification* n1 = CreateNotification(1); - model()->Add(n1); - AppNotification* n2 = CreateNotification(2); - model()->Add(n2); - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - SyncDataList(), - processor()); - - // Some adds and some deletes. - SyncChangeList changes; - changes.push_back(CreateSyncChange( - SyncChange::ACTION_ADD, CreateNotification(3))); - changes.push_back(CreateSyncChange( - SyncChange::ACTION_DELETE, CreateNotification(*n1))); - changes.push_back(CreateSyncChange( - SyncChange::ACTION_ADD, CreateNotification(4))); - - model()->ProcessSyncChanges(FROM_HERE, changes); - - EXPECT_EQ(3U, model()->GetAllSyncData(syncable::APP_NOTIFICATIONS).size()); - EXPECT_EQ(2, processor()->change_list_size()); -} - -// Process sync changes should return error if model association is not done. -TEST_F(AppNotificationManagerSyncTest, - ProcessSyncChangesErrorModelAssocNotDone) { - SyncChangeList changes; - - SyncError sync_error = model()->ProcessSyncChanges(FROM_HERE, changes); - EXPECT_TRUE(sync_error.IsSet()); - EXPECT_EQ(syncable::APP_NOTIFICATIONS, sync_error.type()); - EXPECT_EQ(0, processor()->change_list_size()); -} - -// Stop syncing sets state correctly. -TEST_F(AppNotificationManagerSyncTest, StopSyncing) { - EXPECT_FALSE(model()->sync_processor_); - EXPECT_FALSE(model()->models_associated_); - - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - SyncDataList(), - processor()); - - EXPECT_TRUE(model()->sync_processor_); - EXPECT_TRUE(model()->models_associated_); - - model()->StopSyncing(syncable::APP_NOTIFICATIONS); - EXPECT_FALSE(model()->sync_processor_); - EXPECT_FALSE(model()->models_associated_); -} - -// Adds get pushed to sync but local only are skipped. -TEST_F(AppNotificationManagerSyncTest, AddsGetsSynced) { - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - SyncDataList(), - processor()); - - AppNotification* n1 = CreateNotification(1); - model()->Add(n1); - AppNotification* n2 = CreateNotification(2); - model()->Add(n2); - AppNotification* n3 = CreateNotification(true, 2); - model()->Add(n3); - - EXPECT_EQ(2, processor()->change_list_size()); - EXPECT_TRUE(processor()->ContainsGuid(n1->guid())); - SyncChange c1 = processor()->GetChangeByGuid(n1->guid()); - AssertSyncChange(c1, SyncChange::ACTION_ADD, *n1); - SyncChange c2 = processor()->GetChangeByGuid(n2->guid()); - AssertSyncChange(c2, SyncChange::ACTION_ADD, *n2); -} - -// Clear all gets pushed to sync. -TEST_F(AppNotificationManagerSyncTest, ClearAllGetsSynced) { - const std::string& ext_id = "e1"; - scoped_ptr<AppNotification> n1(CreateNotification(false, 1, ext_id)); - scoped_ptr<AppNotification> n2(CreateNotification(false, 2, ext_id)); - scoped_ptr<AppNotification> n3(CreateNotification(false, 3, ext_id)); - scoped_ptr<AppNotification> n4(CreateNotification(4)); - - SyncDataList initial_data; - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n1)); - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n2)); - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n3)); - initial_data.push_back( - AppNotificationManager::CreateSyncDataFromNotification(*n4)); - model()->MergeDataAndStartSyncing( - syncable::APP_NOTIFICATIONS, - initial_data, - processor()); - - model()->ClearAll(ext_id); - - EXPECT_EQ(3, processor()->change_list_size()); - EXPECT_TRUE(processor()->ContainsGuid(n1->guid())); - SyncChange c1 = processor()->GetChangeByGuid(n1->guid()); - AssertSyncChange(c1, SyncChange::ACTION_DELETE, *n1); - SyncChange c2 = processor()->GetChangeByGuid(n2->guid()); - AssertSyncChange(c2, SyncChange::ACTION_DELETE, *n2); - SyncChange c3 = processor()->GetChangeByGuid(n3->guid()); - AssertSyncChange(c3, SyncChange::ACTION_DELETE, *n3); -} diff --git a/chrome/browser/extensions/app_notification_manager_unittest.cc b/chrome/browser/extensions/app_notification_manager_unittest.cc index cdc2ea0..37aa6c6 100644 --- a/chrome/browser/extensions/app_notification_manager_unittest.cc +++ b/chrome/browser/extensions/app_notification_manager_unittest.cc @@ -71,18 +71,14 @@ class AppNotificationManagerTest : public testing::Test { TEST_F(AppNotificationManagerTest, Simple) { std::string id = extension_test_util::MakeId("whatever"); AppNotificationList list; - util::AddNotifications(&list, id, 2, "foo"); - EXPECT_TRUE(util::AddCopiesFromList(mgr_.get(), list)); + util::AddNotifications(&list, 2, "foo"); + util::AddCopiesFromList(mgr_.get(), id, list); // Cause |mgr_| to be recreated, re-reading from its storage. InitializeManager(); const AppNotification* tmp = mgr_->GetLast(id); - ASSERT_TRUE(tmp); - EXPECT_EQ(list[1]->guid(), tmp->guid()); - EXPECT_EQ(list[1]->extension_id(), tmp->extension_id()); - EXPECT_EQ(list[1]->is_local(), tmp->is_local()); - EXPECT_TRUE(list[1]->Equals(*tmp)); + EXPECT_TRUE(tmp && list[1]->Equals(*tmp)); const AppNotificationList* tmp_list = mgr_->GetAll(id); ASSERT_TRUE(tmp_list != NULL); util::ExpectListsEqual(list, *tmp_list); @@ -105,10 +101,10 @@ TEST_F(AppNotificationManagerTest, MAYBE_ExtensionUninstall) { std::string id2 = extension_test_util::MakeId("id2"); AppNotificationList list1; AppNotificationList list2; - util::AddNotifications(&list1, id1, 5, "foo1"); - util::AddNotifications(&list2, id2, 3, "foo2"); - util::AddCopiesFromList(mgr_.get(), list1); - util::AddCopiesFromList(mgr_.get(), list2); + util::AddNotifications(&list1, 5, "foo1"); + util::AddNotifications(&list2, 3, "foo2"); + util::AddCopiesFromList(mgr_.get(), id1, list1); + util::AddCopiesFromList(mgr_.get(), id2, list2); util::ExpectListsEqual(list1, *mgr_->GetAll(id1)); util::ExpectListsEqual(list2, *mgr_->GetAll(id2)); diff --git a/chrome/browser/extensions/app_notification_storage_unittest.cc b/chrome/browser/extensions/app_notification_storage_unittest.cc index 2fcb251..84b836f 100644 --- a/chrome/browser/extensions/app_notification_storage_unittest.cc +++ b/chrome/browser/extensions/app_notification_storage_unittest.cc @@ -62,7 +62,7 @@ TEST_F(AppNotificationStorageTest, Basics) { // Add some items. AppNotificationList list; - util::AddNotifications(&list, id1, 2, "whatever"); + util::AddNotifications(&list, 2, "whatever"); EXPECT_TRUE(storage_->Set(id1, list)); EXPECT_TRUE(DatabaseExistsOnDisk()); @@ -82,12 +82,12 @@ TEST_F(AppNotificationStorageTest, MultipleExtensions) { // Add items for id1. AppNotificationList list1; - util::AddNotifications(&list1, id1, 2, "one"); + util::AddNotifications(&list1, 2, "one"); EXPECT_TRUE(storage_->Set(id1, list1)); // Add items for id2. AppNotificationList list2; - util::AddNotifications(&list2, id2, 3, "two"); + util::AddNotifications(&list2, 3, "two"); EXPECT_TRUE(storage_->Set(id2, list2)); // Verify the items are present @@ -124,8 +124,8 @@ TEST_F(AppNotificationStorageTest, ReplaceExisting) { std::string id = extension_test_util::MakeId("1"); AppNotificationList list1; AppNotificationList list2; - util::AddNotifications(&list1, id, 5, "one"); - util::AddNotifications(&list2, id, 7, "two"); + util::AddNotifications(&list1, 5, "one"); + util::AddNotifications(&list2, 7, "two"); // Put list1 in, then replace with list2 and verify we get list2 back. EXPECT_TRUE(storage_->Set(id, list1)); diff --git a/chrome/browser/extensions/app_notification_test_util.cc b/chrome/browser/extensions/app_notification_test_util.cc index acef4e1..a148e50 100644 --- a/chrome/browser/extensions/app_notification_test_util.cc +++ b/chrome/browser/extensions/app_notification_test_util.cc @@ -21,15 +21,12 @@ void ExpectListsEqual(const AppNotificationList& one, } void AddNotifications(AppNotificationList* list, - const std::string& extension_id, int count, - const std::string& prefix) { + std::string prefix) { for (int i = 0; i < count; i++) { - std::string guid = prefix + "_guid_" + IntToString(i); std::string title = prefix + "_title_" + IntToString(i); std::string body = prefix + "_body_" + IntToString(i); - AppNotification* item = new AppNotification( - true, guid, extension_id, title, body); + AppNotification* item = new AppNotification(title, body); if (i % 2 == 0) { item->set_link_url(GURL("http://www.example.com/" + prefix)); item->set_link_text(prefix + "_link_" + IntToString(i)); @@ -39,23 +36,20 @@ void AddNotifications(AppNotificationList* list, } AppNotification* CopyAppNotification(const AppNotification& source) { - AppNotification* copy = new AppNotification( - source.is_local(), source.guid(), source.extension_id(), - source.title(), source.body()); + AppNotification* copy = new AppNotification(source.title(), source.body()); copy->set_link_url(source.link_url()); copy->set_link_text(source.link_text()); return copy; } -bool AddCopiesFromList(AppNotificationManager* manager, +void AddCopiesFromList(AppNotificationManager* manager, + const std::string& extension_id, const AppNotificationList& list) { - bool result = true; for (AppNotificationList::const_iterator i = list.begin(); i != list.end(); ++i) { - result = result && manager->Add(CopyAppNotification(*(i->get()))); + manager->Add(extension_id, CopyAppNotification(*(i->get()))); } - return result; } } // namespace app_notification_test_util diff --git a/chrome/browser/extensions/app_notification_test_util.h b/chrome/browser/extensions/app_notification_test_util.h index d17d0b6..0f38e5a 100644 --- a/chrome/browser/extensions/app_notification_test_util.h +++ b/chrome/browser/extensions/app_notification_test_util.h @@ -20,15 +20,16 @@ void ExpectListsEqual(const AppNotificationList& one, // Helper for inserting |count| dummy notifications with |prefix| in their // title and body into |list|. void AddNotifications(AppNotificationList* list, - const std::string& extension_id, int count, - const std::string& prefix); + std::string prefix); // Copy the contents of |source| into a new object. AppNotification* CopyAppNotification(const AppNotification& source); -// Adds a copy of each item in |list| to |manager|. -bool AddCopiesFromList(AppNotificationManager* manager, +// Adds a copy of each item in |list| to |manager| using |extension_id| as the +// extension id. +void AddCopiesFromList(AppNotificationManager* manager, + const std::string& extension_id, const AppNotificationList& list); } // namespace app_notification_test_util diff --git a/chrome/browser/extensions/extension_app_api.cc b/chrome/browser/extensions/extension_app_api.cc index 45e666a..d89526c 100644 --- a/chrome/browser/extensions/extension_app_api.cc +++ b/chrome/browser/extensions/extension_app_api.cc @@ -45,8 +45,7 @@ bool AppNotifyFunction::RunImpl() { if (details->HasKey(kBodyTextKey)) EXTENSION_FUNCTION_VALIDATE(details->GetString(kBodyTextKey, &body)); - scoped_ptr<AppNotification> item(new AppNotification( - true, "", id, title, body)); + scoped_ptr<AppNotification> item(new AppNotification(title, body)); if (details->HasKey(kLinkUrlKey)) { std::string link_url; @@ -69,7 +68,7 @@ bool AppNotifyFunction::RunImpl() { AppNotificationManager* manager = profile()->GetExtensionService()->app_notification_manager(); - manager->Add(item.release()); + manager->Add(id, item.release()); NotificationService::current()->Notify( chrome::NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED, diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 4a71347..715b7b8 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -1273,7 +1273,6 @@ 'browser/event_disposition.cc', 'browser/event_disposition.h', 'browser/extensions/app_notification_manager_unittest.cc', - 'browser/extensions/app_notification_manager_sync_unittest.cc', 'browser/extensions/app_notification_storage_unittest.cc', 'browser/extensions/app_notification_test_util.cc', 'browser/extensions/app_notify_channel_setup_unittest.cc', diff --git a/chrome/common/chrome_notification_types.h b/chrome/common/chrome_notification_types.h index 4dfcd1b..e6cee1b 100644 --- a/chrome/common/chrome_notification_types.h +++ b/chrome/common/chrome_notification_types.h @@ -538,10 +538,6 @@ enum NotificationType { // with the extension id of the app. NOTIFICATION_APP_NOTIFICATION_STATE_CHANGED, - // Finished loading app notification manager. - // The source is AppNotificationManager, and the details are NoDetails. - NOTIFICATION_APP_NOTIFICATION_MANAGER_LOADED, - // Component Updater ------------------------------------------------------- // Sent when the component updater starts doing update checks. If no |