Synced Notification Sync Change Processor

chrome/browser/notifier/synced_notification.cc

+// 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.
+
+#include "chrome/browser/notifier/synced_notification.h"
+
+#include "sync/protocol/sync.pb.h"
+#include "sync/protocol/synced_notification_specifics.pb.h"
+
+using namespace sync_pb;
+
+namespace {
+const char kExtensionScheme[] = "chrome-extension://";
+}  // namespace
+
+SyncedNotification::SyncedNotification(const syncer::SyncData sync_data)
+    : sync_data_(sync_data), has_local_changes_(false) {
+}
+
+SyncedNotification::~SyncedNotification() {}
+
+// TODO(petewil): Consider the timestamp too once it gets added to the protobuf.
+bool SyncedNotification::Equals(const SyncedNotification& other) const {
+  // Two notifications are equal if the <appId/coalescingKey> pair matches.
+  return (get_notification_id() == other.get_notification_id());
+}
+
+// Set the read state on the notification, returns true for success.
+bool SyncedNotification::SetReadState(
+    SyncedNotificationCoalescedNotification_ReadState readState) {
+  if (!sync_data_.GetSpecifics().synced_notification().
+      has_coalesced_notification())
+    return false;
+
+  // TODO(petewil): implement
+  return true;
+}
+
+// Mark this notification as having been read locally.
+void SyncedNotification::NotificationHasBeenRead() {
+  // We set the is_local_ flag to true since we modified it locally
+  // then we create a sync change object to pass back up.
+  has_local_changes_ = true;
+
+  bool success = SetReadState(
+      SyncedNotificationCoalescedNotification_ReadState_READ);
+  DCHECK(success);
+}
+
+// mark this notification as having been dismissed locally
+void SyncedNotification::NotificationHasBeenDeleted() {
+  // We set the is_deleted_ flag to true since we modified it locally
+  // then we create a sync change object to pass back up.
+  has_local_changes_ = true;
+
+  bool success = SetReadState(
+      SyncedNotificationCoalescedNotification_ReadState_DISMISSED);
+  DCHECK(success);
+}
+
+// TODO(petewil): Consider whether the repeated code below can be re-used.
+// A first attempt to do so failed.
+
+const SyncedNotificationSpecifics*
+SyncedNotification::GetSyncedNotificationSpecifics() {
+  return &(sync_data_.GetSpecifics().synced_notification());
+}
+
+const std::string SyncedNotification::get_first_external_id() const {
+  if (!sync_data_.GetSpecifics().synced_notification().
+      has_coalesced_notification())
+    return "";
+  if (sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().notification_size() < 1)
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().notification(0).has_external_id())
+    return "";
+
+  return sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().notification(0).external_id();
+}
+
+const std::string SyncedNotification::get_title() const {
+  if (!sync_data_.GetSpecifics().synced_notification().

[Nicolas Zea, 2013/01/18 23:09:25]

I think you might be able to get away with just checking
sync_data_.GetSpecifics().synced_notification().
    coalesced_notification().render_info().layout().has_layout_type()

If I recall correctly, protocol buffers will return a "default" empty message if
the has_x is false. The empty message's fields will all return false for has_y.

So even if there is no coalesced notification, you can still access its
contents. I don't think this applies to repeated fields, for which you should do
bounds checking.

[Pete Williamson, 2013/01/23 01:45:55]

Done.

+      has_coalesced_notification())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().has_render_info())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().has_layout())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().layout().has_layout_type())
+    return "";
+
+  const SyncedNotificationRenderInfo_Layout_LayoutType layout_type =
+      sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().layout().layout_type();
+
+  // Depending on the layout type, get the proper title.
+  if (SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT

[dcheng, 2013/01/18 21:56:13]

switch () { }?

It may be helpful in other areas to use switch() as well so it's obvious what
cases you're purposely ignoring.

[Pete Williamson, 2013/01/23 01:45:55]

changed here.  In the other places I left the if statement in because only one
type actually has the data we are looking for.

+      == layout_type) {
+
+    // If we have title and subtext, get that title.
+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        has_title_and_subtext_data())
+      return "";
+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_subtext_data().has_title())
+      return "";
+
+    // We have a title after all, return it.
+    return sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_subtext_data().title();
+
+  } else if (SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_IMAGE
+             == layout_type) {
+
+    // If we have title and image, get that title.
+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        has_title_and_image_data())
+      return "";
+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_image_data().has_title())
+      return "";
+
+    // We have a title after all, return it.
+    return sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_image_data().title();
+  } else {
+    // 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.
+    DCHECK(false);
+    return "";
+  }
+}
+
+const std::string SyncedNotification::get_app_id() const {
+  if (!sync_data_.GetSpecifics().synced_notification().
+      has_coalesced_notification())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().has_id())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().id().
+      has_app_id())
+    return "";
+  return sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().id().app_id();
+}
+
+const std::string SyncedNotification::get_coalescing_key() const {
+
+  if (!sync_data_.GetSpecifics().synced_notification().
+      has_coalesced_notification())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().has_id())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().id().
+      has_coalescing_key())
+    return "";
+  return sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().id().coalescing_key();
+}
+
+const std::string SyncedNotification::get_origin_url() const {

[dcheng, 2013/01/18 21:56:13]

Return GURL?

[Pete Williamson, 2013/01/23 01:45:55]

Done.

+  std::string origin_url(kExtensionScheme);
+  origin_url += get_app_id();
+  return origin_url;
+}
+
+const std::string SyncedNotification::get_icon_url() const {

[dcheng, 2013/01/18 21:56:13]

Return GURL?

[Pete Williamson, 2013/01/23 01:45:55]

Done.

+  if (!sync_data_.GetSpecifics().synced_notification().
+      has_coalesced_notification())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().has_render_info())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().has_layout())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().layout().has_layout_type())
+    return "";
+
+  const SyncedNotificationRenderInfo_Layout_LayoutType layout_type =
+      sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().layout().layout_type();
+
+  // Depending on the layout type, get the icon.
+  if (SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT
+      == layout_type) {
+
+    // If we have title and subtext, get that icon.
+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        has_title_and_subtext_data())
+      return "";
+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_subtext_data().has_icon())
+      return "";
+
+    // We have an icon after all, get the URL as a string.

[dcheng, 2013/01/18 21:56:13]

Some of these comments seem misplaced and/or unnecessary. Similar remarks apply
to some of the other comments in this file.

[Pete Williamson, 2013/01/23 01:45:55]

Done.

+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_subtext_data().icon().has_url())
+      return "";
+
+    return sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_subtext_data().icon().url();
+
+  }
+  return "";
+}
+
+const std::string SyncedNotification::get_body() const {
+  // If we have subtext data, concat the text lines and return it.

[dcheng, 2013/01/18 21:56:13]

concatenate.

[Pete Williamson, 2013/01/23 01:45:55]

Done.

+  if (!sync_data_.GetSpecifics().synced_notification().
+      has_coalesced_notification())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().has_render_info())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().has_layout())
+    return "";
+  if (!sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().layout().has_layout_type())
+    return "";
+
+  const SyncedNotificationRenderInfo_Layout_LayoutType layout_type =
+      sync_data_.GetSpecifics().synced_notification().
+      coalesced_notification().render_info().layout().layout_type();
+
+  // Check if this layout type includes body text.
+  if (SyncedNotificationRenderInfo_Layout_LayoutType_TITLE_AND_SUBTEXT
+      == layout_type) {
+
+    // If we have title and subtext, get the text.
+    if (!sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        has_title_and_subtext_data())
+      return "";
+    int subtext_lines = sync_data_.GetSpecifics().synced_notification().
+        coalesced_notification().render_info().layout().
+        title_and_subtext_data().subtext_size();
+    if (subtext_lines < 1)
+      return "";
+
+    // We have a text after all, get it, separate with newlines.
+    std::string subtext;
+    for (int ii = 0; ii < subtext_lines; ++ii) {
+      subtext += sync_data_.GetSpecifics().synced_notification().
+          coalesced_notification().render_info().layout().
+          title_and_subtext_data().subtext(ii);
+      if ( ii < subtext_lines - 1)
+        subtext += '\n';
+    }
+    return subtext;
+  }
+  return "";
+}
+
+const std::string SyncedNotification::get_notification_id() const {
+  // Append the coalescing key to the app id to get the unique id.
+  std::string id = get_app_id();
+  id += "/";
+  id += get_coalescing_key();
+
+  return id;
+}