Widen Data Pipes and newer protobufs

chrome/browser/notifications/sync_notifier/synced_notification.h

 // Copyright (c) 2012 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.
 
 // This class represents the data for a single Synced Notification.
 // It should map 1-1 to all the data in synced_notification_sepcifics.proto,
 // and the data and render protobufs that the specifics protobuf contains.
 
 #ifndef CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_SYNCED_NOTIFICATION_H_
 #define CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_SYNCED_NOTIFICATION_H_
@@ skipping 20 matching lines @@
   enum ReadState {
     kUnread = 1,
     kRead = 2,
     kDismissed = 3,
   };
 
   void Update(const syncer::SyncData& sync_data);
 
   // Here are some helper functions to get individual data parts out of a
   // 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 key() const;
   GURL origin_url() const;
   GURL app_icon_url() const;
   GURL image_url() const;
-  std::string first_external_id() const;
-  std::string notification_id() const;
   std::string text() const;
   ReadState read_state() const;
+  double creation_time() const;

[dcheng, 2013/03/26 07:48:50]

It seems wasteful to continue adding two accessors for each new property.
Shouldn't we just combine them and call it GetFoo()?

[Pete Williamson, 2013/03/26 17:18:30]

I think the class is nicer to use having accessors named after the property, and
with inlining, we should not end up paying extra for the outer wrapper. There's
enough line wrapping going on already, and I'd prefer not to cause more. I'd
prefer to leave it as is here.

[dcheng, 2013/03/26 17:49:29]

This may be nice but it's a violation of the style guide.
lowercase_named_accessors() are typically simple accessors that are inlined into
the headers; other members should be named in CamelCase(). It's something we
should address at some point, and since this class is gaining a bunch of new
members...

[Pete Williamson, 2013/03/27 17:07:53]

Done.

+  int priority() const;
+  int notification_count() const;
+  int button_count() const;
+  std::string default_destination_title() const;
+  std::string default_destination_icon_url() const;
+  std::string default_destination_url() const;
+  std::string button_one_title() const;
+  std::string button_one_icon_url() const;
+  std::string button_one_url() const;
+  std::string button_two_title() const;
+  std::string button_two_icon_url() const;
+  std::string button_two_url() const;
+  std::string contained_notification_title(int index) const;
+  std::string contained_notification_message(int index) const;
 
   bool EqualsIgnoringReadState(const SyncedNotification& other) const;
-  bool IdMatches(const SyncedNotification& other) const;
+  bool KeyMatches(const SyncedNotification& other) const;

[dcheng, 2013/03/26 07:48:50]

This is still unused outside of unit tests. If there's no plan to use it, we
should just remove it.

[Pete Williamson, 2013/03/26 17:18:30]

Done.

 
-  void NotificationHasBeenRead();
   void NotificationHasBeenDismissed();
 
   // This gets a pointer to the SyncedNotificationSpecifics part
   // of the sync data.
   sync_pb::EntitySpecifics GetEntitySpecifics() const;
 
  private:
   // Helper function to mark a notification as read or dismissed.
   void SetReadState(const ReadState& read_state);
 
   // 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 ExtractKey() const;
   GURL ExtractOriginUrl() const;
   GURL ExtractAppIconUrl() const;
   GURL ExtractImageUrl() const;
-  std::string ExtractFirstExternalId() const;
-  std::string ExtractNotificationId() const;
   std::string ExtractText() const;
   ReadState ExtractReadState() const;
+  double ExtractCreationTime() const;
+  int ExtractPriority() const;
+  std::string ExtractDefaultDestinationTitle() const;
+  std::string ExtractDefaultDestinationIconUrl() const;
+  std::string ExtractDefaultDestinationUrl() const;
+  std::string ExtractButtonOneTitle() const;
+  std::string ExtractButtonOneIconUrl() const;
+  std::string ExtractButtonOneUrl() const;
+  std::string ExtractButtonTwoTitle() const;
+  std::string ExtractButtonTwoIconUrl() const;
+  std::string ExtractButtonTwoUrl() const;
+  int ExtractNotificationCount() const;
+  int ExtractButtonCount() const;
+  std::string ExtractContainedNotificationTitle(int index) const;
+  std::string ExtractContainedNotificationMessage(int index) const;
 
   sync_pb::SyncedNotificationSpecifics specifics_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncedNotification);
 };
 
 }  // namespace notifier
 
 #endif  // CHROME_BROWSER_NOTIFICATIONS_SYNC_NOTIFIER_SYNCED_NOTIFICATION_H_