diff options
10 files changed, 459 insertions, 353 deletions
diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.cc b/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.cc index ef368a6..1348d17 100644 --- a/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.cc +++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.cc @@ -4,9 +4,12 @@ #include "chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.h" +#include "chrome/browser/notifications/sync_notifier/chrome_notifier_service.h" + namespace notifier { -ChromeNotifierDelegate::ChromeNotifierDelegate(const std::string& id) - : id_(id) {} +ChromeNotifierDelegate::ChromeNotifierDelegate(const std::string& id, + ChromeNotifierService* notifier) + : id_(id), chrome_notifier_(notifier) {} ChromeNotifierDelegate::~ChromeNotifierDelegate() {} @@ -19,7 +22,7 @@ content::RenderViewHost* ChromeNotifierDelegate::GetRenderViewHost() const { } void ChromeNotifierDelegate::Close(bool by_user) { - // TODO(petewil): implement + chrome_notifier_->MarkNotificationAsDismissed(id_); } } // namespace notifier diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.h b/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.h index 6d28ee8..ad462b8 100644 --- a/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.h +++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_delegate.h @@ -11,13 +11,16 @@ namespace notifier { +class ChromeNotifierService; + // ChromeNotifierDelegate is a NotificationDelegate which catches // responses from the NotificationUIManager when a notification // has been closed. class ChromeNotifierDelegate : public NotificationDelegate { public: - explicit ChromeNotifierDelegate(const std::string& id); + explicit ChromeNotifierDelegate(const std::string& id, + ChromeNotifierService* notifier); // NotificationDelegate interface. virtual void Display() OVERRIDE {} @@ -31,6 +34,7 @@ class ChromeNotifierDelegate : public NotificationDelegate { virtual ~ChromeNotifierDelegate(); const std::string id_; + ChromeNotifierService* const chrome_notifier_; DISALLOW_COPY_AND_ASSIGN(ChromeNotifierDelegate); }; diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc index 194ce87..c58e0e6 100644 --- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc +++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.cc @@ -48,6 +48,7 @@ syncer::SyncMergeResult ChromeNotifierService::MergeDataAndStartSyncing( syncer::SyncMergeResult merge_result(syncer::SYNCED_NOTIFICATIONS); // A list of local changes to send up to the sync server. syncer::SyncChangeList new_changes; + sync_processor_ = sync_processor.Pass(); for (syncer::SyncDataList::const_iterator it = initial_sync_data.begin(); it != initial_sync_data.end(); ++it) { @@ -77,9 +78,9 @@ syncer::SyncMergeResult ChromeNotifierService::MergeDataAndStartSyncing( continue; } else { // If the read state is different, read wins for both places. - if (incoming->read_state() == SyncedNotification::kRead) { + if (incoming->read_state() == SyncedNotification::kDismissed) { // If it is marked as read on the server, but not the client. - found->NotificationHasBeenRead(); + found->NotificationHasBeenDismissed(); // TODO(petewil): Tell the Notification UI Manager to mark it read. } else { // If it is marked as read on the client, but not the server. @@ -103,8 +104,8 @@ syncer::SyncMergeResult ChromeNotifierService::MergeDataAndStartSyncing( // Send up the changes that were made locally. if (new_changes.size() > 0) { - merge_result.set_error( - sync_processor.get()->ProcessSyncChanges(FROM_HERE, new_changes)); + merge_result.set_error(sync_processor_->ProcessSyncChanges( + FROM_HERE, new_changes)); } return merge_result; @@ -188,14 +189,27 @@ scoped_ptr<SyncedNotification> // Check for mandatory fields in the sync_data object. if (!specifics.has_coalesced_notification() || - !specifics.coalesced_notification().has_id() || - !specifics.coalesced_notification().id().has_app_id() || - specifics.coalesced_notification().id().app_id().empty() || - !specifics.coalesced_notification().has_render_info() || - !specifics.coalesced_notification().has_creation_time_msec()) { + !specifics.coalesced_notification().has_key() || + !specifics.coalesced_notification().has_read_state()) { return scoped_ptr<SyncedNotification>(); } + // TODO(petewil): Is this the right set? Should I add more? + bool is_well_formed_unread_notification = + (static_cast<SyncedNotification::ReadState>( + specifics.coalesced_notification().read_state()) == + SyncedNotification::kUnread && + specifics.coalesced_notification().has_render_info()); + bool is_well_formed_dismissed_notification = + (static_cast<SyncedNotification::ReadState>( + specifics.coalesced_notification().read_state()) == + SyncedNotification::kDismissed); + + // if the notification is poorly formed, return a null pointer + if (!is_well_formed_unread_notification && + !is_well_formed_dismissed_notification) + return scoped_ptr<SyncedNotification>(); + // Create a new notification object based on the supplied sync_data. scoped_ptr<SyncedNotification> notification( new SyncedNotification(sync_data)); @@ -210,7 +224,7 @@ SyncedNotification* ChromeNotifierService::FindNotificationById( const std::string& id) { // TODO(petewil): We can make a performance trade off here. // While the vector has good locality of reference, a map has faster lookup. - // Based on how bit we expect this to get, maybe change this to a map. + // Based on how big we expect this to get, maybe change this to a map. for (std::vector<SyncedNotification*>::const_iterator it = notification_data_.begin(); it != notification_data_.end(); @@ -223,6 +237,23 @@ SyncedNotification* ChromeNotifierService::FindNotificationById( return NULL; } +void ChromeNotifierService::MarkNotificationAsDismissed(const std::string& id) { + SyncedNotification* notification = FindNotificationById(id); + CHECK(notification != NULL); + + notification->NotificationHasBeenDismissed(); + syncer::SyncChangeList new_changes; + + syncer::SyncData sync_data = CreateSyncDataFromNotification(*notification); + new_changes.push_back( + syncer::SyncChange(FROM_HERE, + syncer::SyncChange::ACTION_UPDATE, + sync_data)); + + // Send up the changes that were made locally. + sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes); +} + // Add a new notification to our data structure. This takes ownership // of the passed in pointer. void ChromeNotifierService::Add(scoped_ptr<SyncedNotification> notification) { @@ -238,25 +269,39 @@ void ChromeNotifierService::Add(scoped_ptr<SyncedNotification> notification) { void ChromeNotifierService::Show(SyncedNotification* notification) { // Set up the fields we need to send and create a Notification object. GURL origin_url(notification->origin_url()); - GURL icon_url(notification->icon_url()); + GURL app_icon_url(notification->app_icon_url()); string16 title = UTF8ToUTF16(notification->title()); - string16 body = UTF8ToUTF16(notification->body()); + string16 text = UTF8ToUTF16(notification->text()); + string16 heading = UTF8ToUTF16(notification->heading()); + string16 description = UTF8ToUTF16(notification->description()); + // TODO(petewil): What goes in the display source, is empty OK? string16 display_source; string16 replace_id = UTF8ToUTF16(notification->notification_id()); + + // TODO(petewil): For now, just punt on dismissed notifications until + // I change the interface to let NotificationUIManager know the right way. + if (SyncedNotification::kRead == notification->read_state() || + SyncedNotification::kDismissed == notification->read_state() ) { + DVLOG(2) << "Not showing dismissed notification" + << notification->title() << " " << notification->text(); + return; + } + // The delegate will eventually catch calls that the notification // was read or deleted, and send the changes back to the server. scoped_refptr<NotificationDelegate> delegate = - new ChromeNotifierDelegate(notification->notification_id()); + new ChromeNotifierDelegate(notification->notification_id(), this); - Notification ui_notification(origin_url, icon_url, title, body, + Notification ui_notification(origin_url, app_icon_url, heading, description, WebKit::WebTextDirectionDefault, display_source, replace_id, delegate); notification_manager_->Add(ui_notification, profile_); - DVLOG(1) << "Synced Notification arrived! " << title << " " << body - << " " << icon_url << " " << replace_id; + DVLOG(1) << "Synced Notification arrived! " << title << " " << text + << " " << app_icon_url << " " << replace_id << " " + << notification->read_state(); return; } diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h index 58add25..4332e51 100644 --- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h +++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service.h @@ -56,6 +56,9 @@ class ChromeNotifierService : public syncer::SyncableService, // The caller must not free it. notifier::SyncedNotification* FindNotificationById(const std::string& id); + // Called when we dismiss a notification. + void MarkNotificationAsDismissed(const std::string& id); + // functions for test void AddForTest(scoped_ptr<notifier::SyncedNotification> notification) { Add(notification.Pass()); @@ -65,11 +68,13 @@ class ChromeNotifierService : public syncer::SyncableService, // Add a notification to our list. This takes ownership of the pointer. void Add(scoped_ptr<notifier::SyncedNotification> notification); + // Display a notification in the notification center. void Show(notifier::SyncedNotification* notification); // Back pointer to the owning profile. Profile* const profile_; NotificationUIManager* const notification_manager_; + scoped_ptr<syncer::SyncChangeProcessor> sync_processor_; // TODO(petewil): consider whether a map would better suit our data. // If there are many entries, lookup time may trump locality of reference. diff --git a/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc b/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc index 095ca35..f9322e2 100644 --- a/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc +++ b/chrome/browser/notifications/sync_notifier/chrome_notifier_service_unittest.cc @@ -18,7 +18,6 @@ using sync_pb::SyncedNotificationSpecifics; using sync_pb::EntitySpecifics; -using sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT; using syncer::SyncData; using syncer::SyncChange; using syncer::SyncChangeList; @@ -36,23 +35,16 @@ const char kAppId4[] = "fbcmoldooppoahjhfflnmljoanccek44"; const char kAppId5[] = "fbcmoldooppoahjhfflnmljoanccek55"; const char kAppId6[] = "fbcmoldooppoahjhfflnmljoanccek66"; const char kAppId7[] = "fbcmoldooppoahjhfflnmljoanccek77"; -const char kCoalescingKey1[] = "foo"; -const char kCoalescingKey2[] = "bar"; -const char kCoalescingKey3[] = "bat"; -const char kCoalescingKey4[] = "baz"; -const char kCoalescingKey5[] = "foobar"; -const char kCoalescingKey6[] = "fu"; -const char kCoalescingKey7[] = "meta"; -const char kNotificationId1[] = "fboilmbenheemaomgaeehigklolhkhnf/foo"; -const char kNotificationId2[] = "fbcmoldooppoahjhfflnmljoanccekpf/bar"; -const char kNotificationId3[] = "fbcmoldooppoahjhfflnmljoanccek33/bat"; -const char kNotificationId4[] = "fbcmoldooppoahjhfflnmljoanccek44/baz"; -const char kNotificationId5[] = "fbcmoldooppoahjhfflnmljoanccek55/foobar"; -const char kNotificationId6[] = "fbcmoldooppoahjhfflnmljoanccek66/fu"; -const char kNotificationId7[] = "fbcmoldooppoahjhfflnmljoanccek77/meta"; +const char kKey1[] = "foo"; +const char kKey2[] = "bar"; +const char kKey3[] = "bat"; +const char kKey4[] = "baz"; +const char kKey5[] = "foobar"; +const char kKey6[] = "fu"; +const char kKey7[] = "meta"; const int64 kFakeCreationTime = 42; -const sync_pb::CoalescedSyncedNotification_ReadState kRead = - sync_pb::CoalescedSyncedNotification_ReadState_READ; +const sync_pb::CoalescedSyncedNotification_ReadState kDismissed = + sync_pb::CoalescedSyncedNotification_ReadState_DISMISSED; const sync_pb::CoalescedSyncedNotification_ReadState kUnread = sync_pb::CoalescedSyncedNotification_ReadState_UNREAD; @@ -60,11 +52,7 @@ const sync_pb::CoalescedSyncedNotification_ReadState kUnread = std::string GetNotificationId(const SyncData& sync_data) { SyncedNotificationSpecifics specifics = sync_data.GetSpecifics(). synced_notification(); - std::string notification_id = specifics. - coalesced_notification().id().app_id(); - notification_id += "/"; - notification_id += specifics.coalesced_notification().id().coalescing_key(); - return notification_id; + return specifics.coalesced_notification().key(); } // Stub out the NotificationUIManager for unit testing. @@ -194,10 +182,10 @@ class ChromeNotifierServiceTest : public testing::Test { SyncedNotification* CreateNotification( const std::string& message, const std::string& app_id, - const std::string& coalescing_key, + const std::string& key, const std::string& external_id, sync_pb::CoalescedSyncedNotification_ReadState read_state) { - SyncData sync_data = CreateSyncData(message, app_id, coalescing_key, + SyncData sync_data = CreateSyncData(message, app_id, key, external_id, read_state); // Set enough fields in sync_data, including specifics, for our tests // to pass. @@ -221,7 +209,7 @@ class ChromeNotifierServiceTest : public testing::Test { static SyncData CreateSyncData( const std::string& message, const std::string& app_id, - const std::string& coalescing_key, + const std::string& key, const std::string& external_id, sync_pb::CoalescedSyncedNotification_ReadState read_state) { // CreateLocalData makes a copy of this, so this can safely live @@ -232,34 +220,31 @@ class ChromeNotifierServiceTest : public testing::Test { entity_specifics.mutable_synced_notification(); specifics->mutable_coalesced_notification()-> - mutable_render_info()-> - mutable_layout()-> - set_layout_type( - SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT); - - specifics->mutable_coalesced_notification()-> - mutable_id()-> set_app_id(app_id); specifics->mutable_coalesced_notification()-> - mutable_id()-> - set_coalescing_key(coalescing_key); + set_key(key); specifics->mutable_coalesced_notification()-> mutable_render_info()-> - mutable_layout()-> - mutable_title_and_subtext_data()-> + mutable_expanded_info()-> + mutable_simple_expanded_layout()-> set_title(message); - specifics->mutable_coalesced_notification()-> + specifics-> + mutable_coalesced_notification()-> set_creation_time_msec(kFakeCreationTime); - specifics->mutable_coalesced_notification()-> + specifics-> + mutable_coalesced_notification()-> add_notification(); - specifics->mutable_coalesced_notification()-> + + specifics-> + mutable_coalesced_notification()-> mutable_notification(0)->set_external_id(external_id); - specifics->mutable_coalesced_notification()-> + specifics-> + mutable_coalesced_notification()-> set_read_state(read_state); SyncData sync_data = SyncData::CreateLocalData( @@ -283,7 +268,7 @@ class ChromeNotifierServiceTest : public testing::Test { // Create a Notification, convert it to SyncData and convert it back. TEST_F(ChromeNotifierServiceTest, NotificationToSyncDataToNotification) { scoped_ptr<SyncedNotification> notification1( - CreateNotification("1", kAppId1, kCoalescingKey1, "11", kUnread)); + CreateNotification("1", kAppId1, kKey1, "11", kUnread)); SyncData sync_data = ChromeNotifierService::CreateSyncDataFromNotification(*notification1); scoped_ptr<SyncedNotification> notification2( @@ -324,13 +309,13 @@ TEST_F(ChromeNotifierServiceTest, ProcessSyncChangesEmptyModel) { SyncChangeList changes; changes.push_back(CreateSyncChange( SyncChange::ACTION_ADD, CreateNotification( - "1", kAppId1, kCoalescingKey1, "11", kUnread))); + "1", kAppId1, kKey1, "11", kUnread))); changes.push_back(CreateSyncChange( SyncChange::ACTION_ADD, CreateNotification( - "2", kAppId2, kCoalescingKey2, "22", kUnread))); + "2", kAppId2, kKey2, "22", kUnread))); changes.push_back(CreateSyncChange( SyncChange::ACTION_ADD, CreateNotification( - "3", kAppId3, kCoalescingKey3, "33", kUnread))); + "3", kAppId3, kKey3, "33", kUnread))); notifier.ProcessSyncChanges(FROM_HERE, changes); @@ -347,24 +332,24 @@ TEST_F(ChromeNotifierServiceTest, LocalRemoteBothNonEmptyNoOverlap) { // Create some local fake data. scoped_ptr<SyncedNotification> n1(CreateNotification( - "1", kAppId1, kCoalescingKey1, "11", kUnread)); + "1", kAppId1, kKey1, "11", kUnread)); notifier.AddForTest(n1.Pass()); scoped_ptr<SyncedNotification> n2(CreateNotification( - "2", kAppId2, kCoalescingKey2, "22", kUnread)); + "2", kAppId2, kKey2, "22", kUnread)); notifier.AddForTest(n2.Pass()); scoped_ptr<SyncedNotification> n3(CreateNotification( - "3", kAppId3, kCoalescingKey3, "33", kUnread)); + "3", kAppId3, kKey3, "33", kUnread)); notifier.AddForTest(n3.Pass()); // Create some remote fake data. SyncDataList initial_data; - initial_data.push_back(CreateSyncData("4", kAppId4, kCoalescingKey4, + initial_data.push_back(CreateSyncData("4", kAppId4, kKey4, "44", kUnread)); - initial_data.push_back(CreateSyncData("5", kAppId5, kCoalescingKey5, + initial_data.push_back(CreateSyncData("5", kAppId5, kKey5, "55", kUnread)); - initial_data.push_back(CreateSyncData("6", kAppId6, kCoalescingKey6, + initial_data.push_back(CreateSyncData("6", kAppId6, kKey6, "66", kUnread)); - initial_data.push_back(CreateSyncData("7", kAppId7, kCoalescingKey7, + initial_data.push_back(CreateSyncData("7", kAppId7, kKey7, "77", kUnread)); // Merge the local and remote data. @@ -376,13 +361,13 @@ TEST_F(ChromeNotifierServiceTest, LocalRemoteBothNonEmptyNoOverlap) { // Ensure the local store now has all local and remote notifications. EXPECT_EQ(7U, notifier.GetAllSyncData(SYNCED_NOTIFICATIONS).size()); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId1)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId2)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId3)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId4)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId5)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId6)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId7)); + EXPECT_TRUE(notifier.FindNotificationById(kKey1)); + EXPECT_TRUE(notifier.FindNotificationById(kKey2)); + EXPECT_TRUE(notifier.FindNotificationById(kKey3)); + EXPECT_TRUE(notifier.FindNotificationById(kKey4)); + EXPECT_TRUE(notifier.FindNotificationById(kKey5)); + EXPECT_TRUE(notifier.FindNotificationById(kKey6)); + EXPECT_TRUE(notifier.FindNotificationById(kKey7)); // Test the type conversion and construction functions. for (SyncDataList::const_iterator iter = initial_data.begin(); @@ -396,9 +381,9 @@ TEST_F(ChromeNotifierServiceTest, LocalRemoteBothNonEmptyNoOverlap) { EXPECT_TRUE(notification1->EqualsIgnoringReadState(*notification2)); EXPECT_EQ(notification1->read_state(), notification2->read_state()); } - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId1)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId2)); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId3)); + EXPECT_TRUE(notifier.FindNotificationById(kKey1)); + EXPECT_TRUE(notifier.FindNotificationById(kKey2)); + EXPECT_TRUE(notifier.FindNotificationById(kKey3)); } // Test the local store having the read bit unset, the remote store having @@ -409,16 +394,16 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyReadMismatch1) { // Create some local fake data. scoped_ptr<SyncedNotification> n1(CreateNotification( - "1", kAppId1, kCoalescingKey1, "11", kUnread)); + "1", kAppId1, kKey1, "11", kUnread)); notifier.AddForTest(n1.Pass()); scoped_ptr<SyncedNotification> n2(CreateNotification( - "2", kAppId2, kCoalescingKey2, "22", kUnread)); + "2", kAppId2, kKey2, "22", kUnread)); notifier.AddForTest(n2.Pass()); // Create some remote fake data, item 1 matches except for the read state. syncer::SyncDataList initial_data; - initial_data.push_back(CreateSyncData("1", kAppId1, kCoalescingKey1, - "11", kRead)); + initial_data.push_back(CreateSyncData("1", kAppId1, kKey1, + "11", kDismissed)); // Merge the local and remote data. notifier.MergeDataAndStartSyncing( syncer::SYNCED_NOTIFICATIONS, @@ -430,15 +415,15 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyReadMismatch1) { // state of the first is now read. EXPECT_EQ(2U, notifier.GetAllSyncData(syncer::SYNCED_NOTIFICATIONS).size()); SyncedNotification* notification1 = - notifier.FindNotificationById(kNotificationId1); + notifier.FindNotificationById(kKey1); EXPECT_FALSE(NULL == notification1); - EXPECT_EQ(SyncedNotification::kRead, notification1->read_state()); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId2)); - EXPECT_FALSE(notifier.FindNotificationById(kNotificationId3)); + EXPECT_EQ(SyncedNotification::kDismissed, notification1->read_state()); + EXPECT_TRUE(notifier.FindNotificationById(kKey2)); + EXPECT_FALSE(notifier.FindNotificationById(kKey3)); // Ensure no new data will be sent to the remote store for notification1. EXPECT_EQ(0U, processor()->change_list_size()); - EXPECT_FALSE(processor()->ContainsId(kNotificationId1)); + EXPECT_FALSE(processor()->ContainsId(kKey1)); } // Test when the local store has the read bit set, and the remote store has @@ -449,15 +434,15 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyReadMismatch2) { // Create some local fake data. scoped_ptr<SyncedNotification> n1(CreateNotification( - "1", kAppId1, kCoalescingKey1, "11", kRead)); + "1", kAppId1, kKey1, "11", kDismissed)); notifier.AddForTest(n1.Pass()); scoped_ptr<SyncedNotification> n2(CreateNotification( - "2", kAppId2, kCoalescingKey2, "22", kUnread)); + "2", kAppId2, kKey2, "22", kUnread)); notifier.AddForTest(n2.Pass()); // Create some remote fake data, item 1 matches except for the read state. syncer::SyncDataList initial_data; - initial_data.push_back(CreateSyncData("1", kAppId1, kCoalescingKey1, + initial_data.push_back(CreateSyncData("1", kAppId1, kKey1, "11", kUnread)); // Merge the local and remote data. notifier.MergeDataAndStartSyncing( @@ -470,17 +455,17 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyReadMismatch2) { // state of the first is now read. EXPECT_EQ(2U, notifier.GetAllSyncData(syncer::SYNCED_NOTIFICATIONS).size()); SyncedNotification* notification1 = - notifier.FindNotificationById(kNotificationId1); + notifier.FindNotificationById(kKey1); EXPECT_FALSE(NULL == notification1); - EXPECT_EQ(SyncedNotification::kRead, notification1->read_state()); - EXPECT_TRUE(notifier.FindNotificationById(kNotificationId2)); - EXPECT_FALSE(notifier.FindNotificationById(kNotificationId3)); + EXPECT_EQ(SyncedNotification::kDismissed, notification1->read_state()); + EXPECT_TRUE(notifier.FindNotificationById(kKey2)); + EXPECT_FALSE(notifier.FindNotificationById(kKey3)); // Ensure the new data will be sent to the remote store for notification1. EXPECT_EQ(1U, processor()->change_list_size()); - EXPECT_TRUE(processor()->ContainsId(kNotificationId1)); + EXPECT_TRUE(processor()->ContainsId(kKey1)); EXPECT_EQ(SyncChange::ACTION_UPDATE, processor()->GetChangeById( - kNotificationId1).change_type()); + kKey1).change_type()); } // We have a notification in the local store, we get an updated version @@ -491,13 +476,12 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyWithUpdate) { // Create some local fake data. scoped_ptr<SyncedNotification> n1(CreateNotification( - "1", kAppId1, kCoalescingKey1, "11", kRead)); + "1", kAppId1, kKey1, "11", kDismissed)); notifier.AddForTest(n1.Pass()); - // Create some remote fake data, item 1 matches the ID, but has different data syncer::SyncDataList initial_data; - initial_data.push_back(CreateSyncData("One", kAppId1, kCoalescingKey1, + initial_data.push_back(CreateSyncData("One", kAppId1, kKey1, "Eleven", kUnread)); // Merge the local and remote data. notifier.MergeDataAndStartSyncing( @@ -509,7 +493,7 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyWithUpdate) { // Ensure the local store still has only one notification EXPECT_EQ(1U, notifier.GetAllSyncData(syncer::SYNCED_NOTIFICATIONS).size()); SyncedNotification* notification1 = - notifier.FindNotificationById(kNotificationId1); + notifier.FindNotificationById(kKey1); EXPECT_FALSE(NULL == notification1); EXPECT_EQ(SyncedNotification::kUnread, notification1->read_state()); EXPECT_EQ("One", notification1->title()); @@ -517,7 +501,7 @@ TEST_F(ChromeNotifierServiceTest, ModelAssocBothNonEmptyWithUpdate) { // Ensure no new data will be sent to the remote store for notification1. EXPECT_EQ(0U, processor()->change_list_size()); - EXPECT_FALSE(processor()->ContainsId(kNotificationId1)); + EXPECT_FALSE(processor()->ContainsId(kKey1)); } // TODO(petewil): There are more tests to add, such as when we add an API diff --git a/chrome/browser/notifications/sync_notifier/synced_notification.cc b/chrome/browser/notifications/sync_notifier/synced_notification.cc index e225a47..951a50d 100644 --- a/chrome/browser/notifications/sync_notifier/synced_notification.cc +++ b/chrome/browser/notifications/sync_notifier/synced_notification.cc @@ -21,6 +21,10 @@ COMPILE_ASSERT(static_cast<sync_pb::CoalescedSyncedNotification_ReadState>( SyncedNotification::kRead) == sync_pb::CoalescedSyncedNotification_ReadState_READ, local_enum_must_match_protobuf_enum); +COMPILE_ASSERT(static_cast<sync_pb::CoalescedSyncedNotification_ReadState>( + SyncedNotification::kDismissed) == + sync_pb::CoalescedSyncedNotification_ReadState_DISMISSED, + local_enum_must_match_protobuf_enum); SyncedNotification::SyncedNotification(const syncer::SyncData& sync_data) { Update(sync_data); @@ -32,32 +36,38 @@ void SyncedNotification::Update(const syncer::SyncData& sync_data) { specifics_.CopyFrom(sync_data.GetSpecifics().synced_notification()); } -sync_pb::EntitySpecifics -SyncedNotification::GetEntitySpecifics() const { +sync_pb::EntitySpecifics SyncedNotification::GetEntitySpecifics() const { sync_pb::EntitySpecifics entity_specifics; entity_specifics.mutable_synced_notification()->CopyFrom(specifics_); return entity_specifics; } - std::string SyncedNotification::title() const { return ExtractTitle(); } +std::string SyncedNotification::heading() const { + return ExtractHeading(); +} + +std::string SyncedNotification::description() const { + return ExtractDescription(); +} + std::string SyncedNotification::app_id() const { return ExtractAppId(); } -std::string SyncedNotification::coalescing_key() const { - return ExtractCoalescingKey(); +std::string SyncedNotification::key() const { + return ExtractKey(); } GURL SyncedNotification::origin_url() const { return ExtractOriginUrl(); } -GURL SyncedNotification::icon_url() const { - return ExtractIconUrl(); +GURL SyncedNotification::app_icon_url() const { + return ExtractAppIconUrl(); } GURL SyncedNotification::image_url() const { @@ -72,8 +82,8 @@ std::string SyncedNotification::notification_id() const { return ExtractNotificationId(); } -std::string SyncedNotification::body() const { - return ExtractBody(); +std::string SyncedNotification::text() const { + return ExtractText(); } SyncedNotification::ReadState SyncedNotification::read_state() const { @@ -85,12 +95,10 @@ bool SyncedNotification::EqualsIgnoringReadState( const SyncedNotification& other) const { return (title() == other.title() && app_id() == other.app_id() && - coalescing_key() == other.coalescing_key() && - first_external_id() == other.first_external_id() && - notification_id() == other.notification_id() && - body() == other.body() && + key() == other.key() && + text() == other.text() && origin_url() == other.origin_url() && - icon_url() == other.icon_url() && + app_icon_url() == other.app_icon_url() && image_url() == other.image_url() ); } @@ -104,21 +112,27 @@ bool SyncedNotification::IdMatches(const SyncedNotification& other) const { void SyncedNotification::SetReadState(const ReadState& read_state) { // convert the read state to the protobuf type for read state - if (kRead == read_state) - specifics_.mutable_coalesced_notification()-> - set_read_state(sync_pb::CoalescedSyncedNotification_ReadState_READ); + if (kDismissed == read_state) + specifics_.mutable_coalesced_notification()->set_read_state( + sync_pb::CoalescedSyncedNotification_ReadState_DISMISSED); else if (kUnread == read_state) - specifics_.mutable_coalesced_notification()-> - set_read_state(sync_pb::CoalescedSyncedNotification_ReadState_UNREAD); + specifics_.mutable_coalesced_notification()->set_read_state( + sync_pb::CoalescedSyncedNotification_ReadState_UNREAD); + else if (kRead == read_state) + specifics_.mutable_coalesced_notification()->set_read_state( + sync_pb::CoalescedSyncedNotification_ReadState_READ); else NOTREACHED(); } -// Mark this notification as having been read locally. void SyncedNotification::NotificationHasBeenRead() { SetReadState(kRead); } +void SyncedNotification::NotificationHasBeenDismissed() { + SetReadState(kDismissed); +} + std::string SyncedNotification::ExtractFirstExternalId() const { if (!specifics_.has_coalesced_notification() || specifics_.coalesced_notification().notification_size() < 1 || @@ -129,56 +143,42 @@ std::string SyncedNotification::ExtractFirstExternalId() const { } std::string SyncedNotification::ExtractTitle() const { - if (!specifics_.coalesced_notification().render_info().layout(). - has_layout_type()) - return std::string(); + if (!specifics_.coalesced_notification().render_info().expanded_info(). + simple_expanded_layout().has_title()) + return ""; - const sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType layout_type = - specifics_.coalesced_notification().render_info().layout(). - layout_type(); - - // Depending on the layout type, get the proper title. - switch (layout_type) { - case sync_pb:: - SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT: { - // If we have title and subtext, get that title. - if (!specifics_.coalesced_notification().render_info().layout(). - title_and_subtext_data().has_title()) - return std::string(); - - return specifics_.coalesced_notification().render_info().layout(). - title_and_subtext_data().title(); - } - - case sync_pb:: - SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_IMAGE: { - // If we have title and image, get that title. - if (!specifics_.coalesced_notification().render_info().layout(). - title_and_image_data().has_title()) - return std::string(); - - return specifics_.coalesced_notification().render_info().layout(). - title_and_image_data().title(); - } - default: { - // This is an error case, we should never get here unless the protobuf - // is bad, or a new type is introduced and this code does not get updated. - NOTREACHED(); - return std::string(); - } - } + return specifics_.coalesced_notification().render_info().expanded_info(). + simple_expanded_layout().title(); +} + +std::string SyncedNotification::ExtractHeading() const { + if (!specifics_.coalesced_notification().render_info().collapsed_info(). + simple_collapsed_layout().has_heading()) + return ""; + + return specifics_.coalesced_notification().render_info().collapsed_info(). + simple_collapsed_layout().heading(); +} + +std::string SyncedNotification::ExtractDescription() const { + if (!specifics_.coalesced_notification().render_info().collapsed_info(). + simple_collapsed_layout().has_description()) + return ""; + + return specifics_.coalesced_notification().render_info().collapsed_info(). + simple_collapsed_layout().description(); } std::string SyncedNotification::ExtractAppId() const { - if (!specifics_.coalesced_notification().id().has_app_id()) - return std::string(); - return specifics_.coalesced_notification().id().app_id(); + if (!specifics_.coalesced_notification().has_app_id()) + return ""; + return specifics_.coalesced_notification().app_id(); } -std::string SyncedNotification::ExtractCoalescingKey() const { - if (!specifics_.coalesced_notification().id().has_coalescing_key()) - return std::string(); - return specifics_.coalesced_notification().id().coalescing_key(); +std::string SyncedNotification::ExtractKey() const { + if (!specifics_.coalesced_notification().has_key()) + return ""; + return specifics_.coalesced_notification().key(); } GURL SyncedNotification::ExtractOriginUrl() const { @@ -187,85 +187,42 @@ GURL SyncedNotification::ExtractOriginUrl() const { return GURL(origin_url); } -GURL SyncedNotification::ExtractIconUrl() const { - if (!specifics_.coalesced_notification().render_info().layout(). - has_layout_type()) +GURL SyncedNotification::ExtractAppIconUrl() const { + if (specifics_.coalesced_notification().render_info().expanded_info(). + collapsed_info_size() == 0) return GURL(); - const sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType layout_type = - specifics_.coalesced_notification().render_info().layout(). - layout_type(); - - // Depending on the layout type, get the icon. - if (sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT - == layout_type) { - // If we have title and subtext, get that icon. - if (!specifics_.coalesced_notification().render_info().layout(). - title_and_subtext_data().icon().has_url()) - return GURL(); + if (!specifics_.coalesced_notification().render_info().expanded_info(). + collapsed_info(0).simple_collapsed_layout().has_app_icon()) + return GURL(); - return GURL(specifics_.coalesced_notification().render_info().layout(). - title_and_subtext_data().icon().url()); - } - return GURL(); + return GURL(specifics_.coalesced_notification().render_info(). + expanded_info().collapsed_info(0).simple_collapsed_layout(). + app_icon().url()); } +// TODO(petewil): This currenly only handles the first image from the first +// collapsed item, someday return all images. GURL SyncedNotification::ExtractImageUrl() const { - if (!specifics_.coalesced_notification().render_info().layout(). - has_layout_type()) + if (specifics_.coalesced_notification().render_info().expanded_info(). + simple_expanded_layout().media_size() == 0) return GURL(); - const sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType layout_type = - specifics_.coalesced_notification().render_info().layout(). - layout_type(); - - // Depending on the layout type, get the image. - if (sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_IMAGE - == layout_type) { - // If we have title and subtext, get that image. - if (!specifics_.coalesced_notification().render_info().layout(). - title_and_image_data().image().has_url()) - return GURL(); + if (!specifics_.coalesced_notification().render_info().expanded_info(). + simple_expanded_layout().media(0).image().has_url()) + return GURL(); - return GURL(specifics_.coalesced_notification().render_info().layout(). - title_and_image_data().image().url()); - } - return GURL(); + return GURL(specifics_.coalesced_notification().render_info(). + expanded_info().simple_expanded_layout().media(0).image().url()); } -std::string SyncedNotification::ExtractBody() const { - // If we have subtext data, concatenate the text lines and return it. - if (!specifics_.coalesced_notification().render_info().layout(). - has_layout_type()) - return std::string(); +std::string SyncedNotification::ExtractText() const { + if (!specifics_.coalesced_notification().render_info().expanded_info(). + simple_expanded_layout().has_text()) + return ""; - const sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType layout_type = - specifics_.coalesced_notification().render_info().layout(). - layout_type(); - - // Check if this layout type includes body text. - if (sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT - == layout_type) { - // If we have title and subtext, get the text. - if (!specifics_.coalesced_notification().render_info().layout(). - has_title_and_subtext_data()) - return std::string(); - int subtext_lines = specifics_.coalesced_notification().render_info(). - layout().title_and_subtext_data().subtext_size(); - if (subtext_lines < 1) - return std::string(); - - std::string subtext; - for (int ii = 0; ii < subtext_lines; ++ii) { - subtext += specifics_.coalesced_notification().render_info(). - layout(). - title_and_subtext_data().subtext(ii); - if (ii < subtext_lines - 1) - subtext += '\n'; - } - return subtext; - } - return std::string(); + return specifics_.coalesced_notification().render_info().expanded_info(). + simple_expanded_layout().text(); } SyncedNotification::ReadState SyncedNotification::ExtractReadState() const { @@ -274,11 +231,15 @@ SyncedNotification::ReadState SyncedNotification::ExtractReadState() const { sync_pb::CoalescedSyncedNotification_ReadState found_read_state = specifics_.coalesced_notification().read_state(); - if (found_read_state == sync_pb::CoalescedSyncedNotification_ReadState_READ) { - return kRead; + if (found_read_state == + sync_pb::CoalescedSyncedNotification_ReadState_DISMISSED) { + return kDismissed; } else if (found_read_state == sync_pb::CoalescedSyncedNotification_ReadState_UNREAD) { return kUnread; + } else if (found_read_state == + sync_pb::CoalescedSyncedNotification_ReadState_READ) { + return kRead; } else { NOTREACHED(); return static_cast<SyncedNotification::ReadState>(found_read_state); @@ -286,12 +247,7 @@ SyncedNotification::ReadState SyncedNotification::ExtractReadState() const { } std::string SyncedNotification::ExtractNotificationId() const { - // Append the coalescing key to the app id to get the unique id. - std::string id = app_id(); - id += "/"; - id += coalescing_key(); - - return id; + return key(); } } // namespace notifier diff --git a/chrome/browser/notifications/sync_notifier/synced_notification.h b/chrome/browser/notifications/sync_notifier/synced_notification.h index 90e2185..b7eb805 100644 --- a/chrome/browser/notifications/sync_notifier/synced_notification.h +++ b/chrome/browser/notifications/sync_notifier/synced_notification.h @@ -31,6 +31,7 @@ class SyncedNotification { enum ReadState { kUnread = 1, kRead = 2, + kDismissed = 3, }; void Update(const syncer::SyncData& sync_data); @@ -39,20 +40,23 @@ class SyncedNotification { // SyncedNotification. // TODO(petewil): Add more types as we expand support for the protobuf. std::string title() const; + std::string heading() const; + std::string description() const; std::string app_id() const; - std::string coalescing_key() const; + std::string key() const; GURL origin_url() const; - GURL icon_url() const; + GURL app_icon_url() const; GURL image_url() const; std::string first_external_id() const; std::string notification_id() const; - std::string body() const; + std::string text() const; ReadState read_state() const; bool EqualsIgnoringReadState(const SyncedNotification& other) const; bool IdMatches(const SyncedNotification& other) const; void NotificationHasBeenRead(); + void NotificationHasBeenDismissed(); // This gets a pointer to the SyncedNotificationSpecifics part // of the sync data. @@ -65,14 +69,16 @@ class SyncedNotification { // Parsing functions to get this information out of the sync_data and into // our local variables. std::string ExtractTitle() const; + std::string ExtractHeading() const; + std::string ExtractDescription() const; std::string ExtractAppId() const; - std::string ExtractCoalescingKey() const; + std::string ExtractKey() const; GURL ExtractOriginUrl() const; - GURL ExtractIconUrl() const; + GURL ExtractAppIconUrl() const; GURL ExtractImageUrl() const; std::string ExtractFirstExternalId() const; std::string ExtractNotificationId() const; - std::string ExtractBody() const; + std::string ExtractText() const; ReadState ExtractReadState() const; sync_pb::SyncedNotificationSpecifics specifics_; diff --git a/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc b/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc index 4b928d3..f77af5e 100644 --- a/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc +++ b/chrome/browser/notifications/sync_notifier/synced_notification_unittest.cc @@ -15,7 +15,6 @@ using syncer::SyncData; using notifier::SyncedNotification; using sync_pb::EntitySpecifics; using sync_pb::SyncedNotificationSpecifics; -using sync_pb::SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT; namespace { @@ -26,11 +25,11 @@ const char kTitle2[] = "Email from Mark: Upcoming Ski trip"; const char kTitle3[] = "Weather alert - light rain tonight."; const char kAppId1[] = "fboilmbenheemaomgaeehigklolhkhnf"; const char kAppId2[] = "fbcmoldooppoahjhfflnmljoanccekpf"; -const char kCoalescingKey1[] = "foo"; -const char kCoalescingKey2[] = "bar"; -const char kBody1[] = "Space Needle, 12:00 pm"; -const char kBody2[] = "Stevens Pass is our first choice."; -const char kBody3[] = "More rain expected in the Seattle area tonight."; +const char kKey1[] = "foo"; +const char kKey2[] = "bar"; +const char kText1[] = "Space Needle, 12:00 pm"; +const char kText2[] = "Stevens Pass is our first choice."; +const char kText3[] = "More rain expected in the Seattle area tonight."; const char kIconUrl1[] = "http://www.google.com/icon1.jpg"; const char kIconUrl2[] = "http://www.google.com/icon2.jpg"; const char kIconUrl3[] = "http://www.google.com/icon3.jpg"; @@ -41,6 +40,8 @@ const sync_pb::CoalescedSyncedNotification_ReadState kRead = sync_pb::CoalescedSyncedNotification_ReadState_READ; const sync_pb::CoalescedSyncedNotification_ReadState kUnread = sync_pb::CoalescedSyncedNotification_ReadState_UNREAD; +const sync_pb::CoalescedSyncedNotification_ReadState kDismissed = + sync_pb::CoalescedSyncedNotification_ReadState_DISMISSED; } // namespace class SyncedNotificationTest : public testing::Test { @@ -51,17 +52,17 @@ class SyncedNotificationTest : public testing::Test { // Methods from testing::Test. virtual void SetUp() OVERRIDE { - sync_data1_ = CreateSyncData( - kTitle1, kBody1, kIconUrl1, kAppId1, kCoalescingKey1, kUnread); - sync_data2_ = CreateSyncData( - kTitle2, kBody2, kIconUrl2, kAppId2, kCoalescingKey2, kUnread); + sync_data1_ = CreateSyncData(kTitle1, kText1, kIconUrl1, kImageUrl1, + kAppId1, kKey1, kUnread); + sync_data2_ = CreateSyncData(kTitle2, kText2, kIconUrl2, kImageUrl1, + kAppId2, kKey2, kUnread); // Notification 3 will have the same ID as notification1, but different // data inside. - sync_data3_ = CreateSyncData( - kTitle3, kBody3, kIconUrl3, kAppId1, kCoalescingKey1, kUnread); + sync_data3_ = CreateSyncData(kTitle3, kText3, kIconUrl3, kImageUrl1, + kAppId1, kKey1, kUnread); // Notification 4 will be the same as 1, but the read state will be 'read'. - sync_data4_ = CreateSyncData( - kTitle1, kBody1, kIconUrl1, kAppId1, kCoalescingKey1, kRead); + sync_data4_ = CreateSyncData(kTitle1, kText1, kIconUrl1, kImageUrl1, + kAppId1, kKey1, kDismissed); notification1_.reset(new SyncedNotification(sync_data1_)); notification2_.reset(new SyncedNotification(sync_data2_)); @@ -85,10 +86,11 @@ class SyncedNotificationTest : public testing::Test { // Helper to create syncer::SyncData. static SyncData CreateSyncData( const std::string& title, - const std::string& body, - const std::string& icon_url, + const std::string& text, + const std::string& app_icon_url, + const std::string& image_url, const std::string& app_id, - const std::string& coalescing_key, + const std::string& key, const sync_pb::CoalescedSyncedNotification_ReadState read_state) { // CreateLocalData makes a copy of this, so this can safely live // on the stack. @@ -100,37 +102,47 @@ class SyncedNotificationTest : public testing::Test { entity_specifics.mutable_synced_notification(); specifics->mutable_coalesced_notification()-> - mutable_render_info()-> - mutable_layout()-> - set_layout_type( - SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT); - - specifics->mutable_coalesced_notification()-> - mutable_id()-> set_app_id(app_id); specifics->mutable_coalesced_notification()-> - mutable_id()-> - set_coalescing_key(coalescing_key); + set_key(key); specifics->mutable_coalesced_notification()-> mutable_render_info()-> - mutable_layout()-> - mutable_title_and_subtext_data()-> + mutable_expanded_info()-> + mutable_simple_expanded_layout()-> set_title(title); specifics->mutable_coalesced_notification()-> mutable_render_info()-> - mutable_layout()-> - mutable_title_and_subtext_data()-> - add_subtext(body); + mutable_expanded_info()-> + mutable_simple_expanded_layout()-> + set_text(text); + + specifics->mutable_coalesced_notification()-> + mutable_render_info()-> + mutable_expanded_info()-> + add_collapsed_info(); + specifics->mutable_coalesced_notification()-> + mutable_render_info()-> + mutable_expanded_info()-> + mutable_collapsed_info(0)-> + mutable_simple_collapsed_layout()-> + mutable_app_icon()-> + set_url(app_icon_url); specifics->mutable_coalesced_notification()-> mutable_render_info()-> - mutable_layout()-> - mutable_title_and_subtext_data()-> - mutable_icon()-> - set_url(icon_url); + mutable_expanded_info()-> + mutable_simple_expanded_layout()-> + add_media(); + specifics->mutable_coalesced_notification()-> + mutable_render_info()-> + mutable_expanded_info()-> + mutable_simple_expanded_layout()-> + mutable_media(0)-> + mutable_image()-> + set_url(image_url); specifics->mutable_coalesced_notification()-> set_creation_time_msec(kFakeCreationTime); @@ -164,9 +176,9 @@ TEST_F(SyncedNotificationTest, GetAppIdTest) { EXPECT_EQ(found_app_id, expected_app_id); } -TEST_F(SyncedNotificationTest, GetCoalescingKeyTest) { - std::string found_key = notification1_->coalescing_key(); - std::string expected_key(kCoalescingKey1); +TEST_F(SyncedNotificationTest, GetKeyTest) { + std::string found_key = notification1_->key(); + std::string expected_key(kKey1); EXPECT_EQ(expected_key, found_key); } @@ -179,7 +191,7 @@ TEST_F(SyncedNotificationTest, GetTitleTest) { } TEST_F(SyncedNotificationTest, GetIconURLTest) { - std::string found_icon_url = notification1_->icon_url().spec(); + std::string found_icon_url = notification1_->app_icon_url().spec(); std::string expected_icon_url(kIconUrl1); EXPECT_EQ(expected_icon_url, found_icon_url); @@ -194,7 +206,7 @@ TEST_F(SyncedNotificationTest, GetReadStateTest) { SyncedNotification::ReadState found_state2 = notification4_->read_state(); - SyncedNotification::ReadState expected_state2(SyncedNotification::kRead); + SyncedNotification::ReadState expected_state2(SyncedNotification::kDismissed); EXPECT_EQ(expected_state2, found_state2); } @@ -203,24 +215,22 @@ TEST_F(SyncedNotificationTest, GetReadStateTest) { // pass on actual data. TEST_F(SyncedNotificationTest, GetImageURLTest) { std::string found_image_url = notification1_->image_url().spec(); - std::string expected_image_url; // TODO(petewil): (kImageUrl1) + std::string expected_image_url = kImageUrl1; EXPECT_EQ(expected_image_url, found_image_url); } // TODO(petewil): test with a multi-line body -TEST_F(SyncedNotificationTest, GetBodyTest) { - std::string found_body = notification1_->body(); - std::string expected_body(kBody1); +TEST_F(SyncedNotificationTest, GetTextTest) { + std::string found_text = notification1_->text(); + std::string expected_text(kText1); - EXPECT_EQ(expected_body, found_body); + EXPECT_EQ(expected_text, found_text); } TEST_F(SyncedNotificationTest, GetNotificationIdTest) { std::string found_id = notification1_->notification_id(); - std::string expected_id(kAppId1); - expected_id += "/"; - expected_id += kCoalescingKey1; + std::string expected_id(kKey1); EXPECT_EQ(expected_id, found_id); } diff --git a/sync/protocol/synced_notification_data.proto b/sync/protocol/synced_notification_data.proto index 1b8112e..fed7ee7 100644 --- a/sync/protocol/synced_notification_data.proto +++ b/sync/protocol/synced_notification_data.proto @@ -26,31 +26,59 @@ message SyncedNotificationIdentifier { optional string coalescing_key = 2; } -message SyncedNotification { - // The unique identifier of the notification. - optional SyncedNotificationIdentifier id = 1; +message SyncedNotificationCreator { + // The gaia id of the creator. If a notification does not have a clear + // creator, skip this and follow the directions below to use a system creator. + optional int64 gaia_id = 1; + + // Indicates that the creator is a "system" creator. Example of these are + // notifications sent to the user where the addressee is "Google", such as the + // "You have violated our TOS, and have 3 days to fix it or you'll lose your + // account" notifications. If is_system is set, gaia_id must not be set and + // instead the app_id field must be set. + optional bool is_system = 2; + + // Only set this in the system-creator case. + optional string app_id = 3; +} + +message SyncedNotificationRecipients { + repeated int64 gaia_id = 1; + + // For now, only support gaia id recipients. Add more recipient types via + // 'repeated Type other_type = X' when necessary. +} +message SyncedNotification { // A secondary type that is isolated within the same app_id. // // NOTE: For ASBE support purposes this must be in the format [A-Za-z_]+. - optional string type = 2; + optional string type = 1; // Whatever string the client entered during creation. If no external_id is // specified, the notification can no longer be identified individually for // fetching/deleting, etc... - optional string external_id = 3; + optional string external_id = 2; + + // The creator of the notification. + optional SyncedNotificationCreator creator = 3; + + // TODO(petewil): This won't build. Import the relevant protobuf. + // optional MapData client_data = 4; } message CoalescedSyncedNotification { - // The identifier used to identify individual coalesced notifications. - optional SyncedNotificationIdentifier id = 1; + // An opaque string key used to identify individual coalesced notifications. + optional string key = 1; + + optional string app_id = 2; // All the notifications that are grouped together. - repeated SyncedNotification notification = 2; + repeated SyncedNotification notification = 3; // Data that is used directly by endpoints to render notifications in the case // where no "native" app can handle the notification. - optional SyncedNotificationRenderInfo render_info = 3; + optional SyncedNotificationRenderInfo render_info = 4; // Read state will be per coalesced notification. enum ReadState { @@ -58,9 +86,21 @@ message CoalescedSyncedNotification { READ = 2; DISMISSED = 3; } - optional ReadState read_state = 4; + optional ReadState read_state = 5; // The time when the LATEST notification of the coalesced notification is // created (in milliseconds since the linux epoch). - optional uint64 creation_time_msec = 5; + optional uint64 creation_time_msec = 6; + + enum Priority { + LOW = 1; + STANDARD = 2; + HIGH = 3; + // We will most likely add at least one more priority in the near future. + }; + optional Priority priority = 7; } + +message SyncedNotificationList { + repeated CoalescedSyncedNotification coalesced_notification = 1; +}
\ No newline at end of file diff --git a/sync/protocol/synced_notification_render.proto b/sync/protocol/synced_notification_render.proto index 47dd6c8..ce5a721 100644 --- a/sync/protocol/synced_notification_render.proto +++ b/sync/protocol/synced_notification_render.proto @@ -16,68 +16,121 @@ package sync_pb; // Data that is used directly by endpoints to render notifications in the case // where no "native" app can handle the notification. message SyncedNotificationRenderInfo { - message Layout { - enum LayoutType { - TITLE_AND_SUBTEXT = 1; - TITLE_AND_IMAGE = 2; + + // Render information for the collapsed (summary) view of a coalesced + // notification. + message CollapsedInfo { + message SimpleCollapsedLayout { + // Application icon. + optional SyncedNotificationImage app_icon = 1; + + // Profile image(s) of the notification creator(s) to show in the + // collapsed UI. + repeated SyncedNotificationProfileImage profile_image = 2; + + // Heading - often the name(s) of the notification creator(s). + optional string heading = 3; + + // Description - often the action that generated the notification. + optional string description = 4; } - optional LayoutType layout_type = 1; + optional SimpleCollapsedLayout simple_collapsed_layout = 1; - message TitleAndSubtextData { - optional string title = 1; - // The icon to show on the left of the notification. - optional SyncedNotificationIcon icon = 2; - // Multiple lines of the sub-text. - repeated string subtext = 3; + // The creation time of the notification in microseconds since the UNIX + // epoch. + optional uint64 creation_timestamp_usec = 2; + + // The default destination target. + optional SyncedNotificationDestination default_destination = 3; + + // Secondary destinations and actions grouped into a message to account for + // ordering. + message Target { + optional SyncedNotificationDestination destination = 1; + optional SyncedNotificationAction action = 2; } - optional TitleAndSubtextData title_and_subtext_data = 3; + repeated Target target = 4; + } + optional CollapsedInfo collapsed_info = 1; - message TitleAndImageData { + // Render information for the expanded (detail) view of a coalesced + // notification. + message ExpandedInfo { + message SimpleExpandedLayout { + // Title - often the title of the underlying entity referred to by the + // notification(s). optional string title = 1; - optional SyncedNotificationImage image = 2; + + // Text content - often a snippet of text from the underlying entity + // reference or the notification. + optional string text = 2; + + // Media. + message Media { + // TOOD(jro): Do we need other media types? + optional SyncedNotificationImage image = 1; + } + repeated Media media = 3; } - optional TitleAndImageData title_and_image_data = 4; - } - optional Layout layout = 1; + optional SimpleExpandedLayout simple_expanded_layout = 1; - // An Action encapsulates an UI component that trigger certain programmable - // actions. Depending on the endpoint, this may show up as a html button, - // "quick actions" in the Android notification drawer, a link, or even the - // notification card itself (the "default" action case). - message Action { - // The description for the Action. - optional string text = 1; + // Collapsed information for each notification in the coalesced group. + repeated CollapsedInfo collapsed_info = 2; + } + optional ExpandedInfo expanded_info = 2; +} - // The icon to use for the Action. - optional SyncedNotificationIcon icon = 2; +// A Destination is a target URL that the user can be taken to by clicking on or +// selecting the notification or part thereof. +message SyncedNotificationDestination { + // The description for the link to the destination. + optional string text = 1; - // Specify whether the action should be a background action or - // should open up a web page with the specified URL. - optional bool is_background = 3; + // The icon to use for the link to the destination. + optional SyncedNotificationImage icon = 2; - // The url to send users to when they trigger the action. - optional string action_url = 4; + // The destination URL. + optional string url = 3; - // If post_data is populated, this indicates that action_url should be - // contacted via a POST rather than a GET. - optional string post_data = 5; - } - // All the actions that can be triggered from this (coalesced) notification. - // This is sorted by importance so depending on the end point, the first N - // actions should be used (and the first action is the "default" action). - // For default actions, the icon and text params are ignored. - repeated Action action = 2; + // Optional label to aid accessibility. + optional string accessibility_label = 4; +} + +// An Action encapsulates an UI component that trigger certain programmable +// actions. Depending on the endpoint, this may show up as a HTML button, an +// action button associated with the notification on native mobile, a link, or +// even the notification card itself. +message SyncedNotificationAction { + // The description for the Action. + optional string text = 1; + + // The icon to use for the Action. + optional SyncedNotificationImage icon = 2; + + // The URL that performs the action. + optional string url = 3; + + // Additional request data. + optional string request_data = 4; + + // Optional label to aid accessibility. + optional string accessibility_label= 5; } message SyncedNotificationImage { + // Note that the image may be from any source. Clients wishing to resize the + // image should ensure the image is proxied appropriately. optional string url = 1; - // This is somewhat made up - not sure what else apart from url do we need - // about an image. optional string alt_text = 2; optional int32 preferred_width = 3; optional int32 preferred_height = 4; } -message SyncedNotificationIcon { - optional string url = 1; -} +message SyncedNotificationProfileImage { + // Url for the image. + optional string image_url = 1; + // Object id for the image. + optional string oid = 2; + // Name to display for this image. + optional string display_name = 3; +}
\ No newline at end of file |