Combining cards instead of merging

chrome/browser/resources/google_now/cards.js

Index: chrome/browser/resources/google_now/cards.js
diff --git a/chrome/browser/resources/google_now/cards.js b/chrome/browser/resources/google_now/cards.js
index 865db8a36b649f97608886f4f016e306370326e7..371ede0fca621095f0c5c2a65db2f7bb5da4bbc6 100644
--- a/chrome/browser/resources/google_now/cards.js
+++ b/chrome/browser/resources/google_now/cards.js
@@ -8,237 +8,332 @@
  * Show/hide trigger in a card.
  *
  * @typedef {{
- *   showTime: number=,
- *   hideTime: number=
+ *   showTimeSec: (string|undefined),
+ *   hideTimeSec: string
  * }}
  */
 var Trigger;
 
 /**
+ * ID of an undividual (uncombined) notification.

[rgustafson, 2013/12/06 20:01:17]

individual

[vadimt, 2013/12/06 20:57:15]

Fixed, but the old one was better.

+ *
+ * @typedef {string}
+ */
+var NotificationId;
+
+/**
  * Data to build a dismissal request for a card from a specific group.
  *
  * @typedef {{
- *   notificationId: string,
+ *   notificationId: NotificationId,
  *   parameters: Object
  * }}
  */
 var DismissalData;
 
 /**
- * Card merged from potentially multiple groups.
+ * Urls that need to be opened when clicking at notification or its buttons.

[rgustafson, 2013/12/06 20:01:17]

clicking a

[vadimt, 2013/12/06 20:57:15]

Done.

  *
  * @typedef {{
+ *   messageUrl: (string|undefined),
+ *   buttonUrls: (Array.<string>|undefined)
+ * }}
+ */
+var ActionUrls;
+
+/**
+ * ID of a combined notification. This is the ID used with chrome.notifications
+ * API.
+ *
+ * @typedef {string}
+ */
+var ChromeNotificationId;
+
+/**
+ * Notification as it's sent by the server.
+ *
+ * @typedef {{
+ *   notificationId: NotificationId,
+ *   chromeNotificationId: ChromeNotificationId,
  *   trigger: Trigger,
  *   version: number,
- *   timestamp: number,
- *   notification: Object,
- *   actionUrls: Object=,
- *   groupRank: number,
- *   dismissals: Array.<DismissalData>,
- *   locationBased: boolean=
+ *   chromeNotificationOptions: Object,
+ *   actionUrls: (ActionUrls|undefined),
+ *   dismissal: Object,
+ *   locationBased: (boolean|undefined),
+ *   groupName: string
  * }}
  */
-var MergedCard;
+var ReceivedNotification;
 
 /**
- * Set of parameters for creating card notification.
+ * Received notification, and absolute show/hide times.

[rgustafson, 2013/12/06 20:01:17]

Describe what the actual object's purpose is instead of what's inside that we
can see right below.

[vadimt, 2013/12/06 20:57:15]

Done.

  *
  * @typedef {{
- *   notification: Object,
- *   hideTime: number=,
- *   version: number,
- *   previousVersion: number=,
- *   locationBased: boolean=
+ *   receivedNotification: ReceivedNotification,
+ *   showTime: (number|undefined),
+ *   hideTime: number
  * }}
  */
-var CardCreateInfo;
+var UncombinedNotification;
+
+/**
+ * Card combined from potentially multiple groups.
+ *
+ * @typedef {Array.<UncombinedNotification>}
+ */
+var CombinedCard;
+
+/**
+ * Data entry that we store for every Chrome notification.
+ * |timestamp| is the time when corresponding Chrome notification was created or
+ * updated last time by cardSet.update().
+ *
+ * @typedef {{
+ *   actionUrls: (ActionUrls|undefined),
+ *   timestamp: number,
+ *   combinedCard: CombinedCard
+ * }}
+ *
+ */
+ var NotificationDataEntry;
 
 /**
  * Names for tasks that can be created by the this file.
  */
-var SHOW_CARD_TASK_NAME = 'show-card';
-var CLEAR_CARD_TASK_NAME = 'clear-card';
+var UPDATE_CARD_TASK_NAME = 'update-card';
 
 /**
  * Builds an object to manage notification card set.
  * @return {Object} Card set interface.
  */
 function buildCardSet() {
-  var cardShowPrefix = 'card-show-';
-  var cardHidePrefix = 'card-hide-';
+  var alarmPrefix = 'card-';
 
   /**
-   * Schedules hiding a notification.
-   * @param {string} cardId Card ID.
-   * @param {number=} opt_timeHide If specified, epoch time to hide the card. If
-   *      undefined, the card will be kept shown at least until next update.
+   * Creates/updates/deletes a Chrome notification.
+   * @param {ChromeNotificationId} cardId Card ID.
+   * @param {?ReceivedNotification} receivedNotification Google Now card
+   *     represented as a set of parameters for showing a Chrome notification,
+   *     or null if the notification needs to be deleted.
+   * @param {function(ReceivedNotification)=} onCardShown Optional parameter
+   *     called when each card is shown.
    */
-  function scheduleHiding(cardId, opt_timeHide) {
-    if (opt_timeHide === undefined)
+  function updateNotification(cardId, receivedNotification, onCardShown) {
+    console.log('cardManager.updateNotification ' + cardId + ' ' +
+                JSON.stringify(receivedNotification));
+
+    if (!receivedNotification) {
+      instrumented.notifications.clear(cardId, function() {});
       return;
+    }
 
-    var alarmName = cardHidePrefix + cardId;
-    var alarmInfo = {when: opt_timeHide};
-    chrome.alarms.create(alarmName, alarmInfo);
+    // Try updating the notification.
+    instrumented.notifications.update(
+        cardId,
+        receivedNotification.chromeNotificationOptions,
+        function(wasUpdated) {
+          if (!wasUpdated) {
+            // If the notification wasn't updated, it probably didn't exist.
+            // Create it.
+            console.log('cardManager.updateNotification ' + cardId +
+                        ' failed to update, creating');
+            instrumented.notifications.create(
+                cardId,
+                receivedNotification.chromeNotificationOptions,
+                function(newNotificationId) {
+                  if (!newNotificationId || chrome.runtime.lastError) {
+                    var errorMessage = chrome.runtime.lastError &&
+                                       chrome.runtime.lastError.message;
+                    console.error('notifications.create: ID=' +
+                        newNotificationId + ', ERROR=' + errorMessage);
+                    return;
+                  }
+
+                  if (onCardShown !== undefined)
+                    onCardShown(receivedNotification);
+                });
+          }
+        });
   }
 
   /**
-   * Shows a notification.
-   * @param {string} cardId Card ID.
-   * @param {CardCreateInfo} cardCreateInfo Google Now card represented as a set
-   *     of parameters for showing a Chrome notification.
-   * @param {function(CardCreateInfo)=} onCardShown Optional parameter called
-   *     when each card is shown.
+   * Enumerates uncombined notifications in a combined card, determining for
+   * each whether it's visible at the specified moment.
+   * @param {CombinedCard} combinedCard The combined card in question.
+   * @param {number} timestamp Time for which to calculate visibility.
+   * @param {function(UncombinedNotification, boolean)} callback Function
+   *     invoked for every uncombined notification in |combinedCard|.
+   *     The boolean parameter indicates whether the uncombined notification is
+   *     visible at |timestamp|.
    */
-  function showNotification(cardId, cardCreateInfo, onCardShown) {
-    console.log('cardManager.showNotification ' + cardId + ' ' +
-                JSON.stringify(cardCreateInfo));
-
-    if (cardCreateInfo.hideTime <= Date.now()) {
-      console.log('cardManager.showNotification ' + cardId + ': expired');
-      // Card has expired. Schedule hiding to delete asociated information.
-      scheduleHiding(cardId, cardCreateInfo.hideTime);
-      return;
-    }
+  function enumerateUncombinedNotifications(combinedCard, timestamp, callback) {
+    for (var i = 0; i != combinedCard.length; ++i) {
+      var uncombinedNotification = combinedCard[i];
+      var shouldShow = !uncombinedNotification.showTime ||
+          uncombinedNotification.showTime <= timestamp;
+      var shouldHide = uncombinedNotification.hideTime <= timestamp;
 
-    if (cardCreateInfo.previousVersion !== cardCreateInfo.version) {
-      // Delete a notification with the specified id if it already exists, and
-      // then create a notification.
-      instrumented.notifications.create(
-          cardId,
-          cardCreateInfo.notification,
-          function(newNotificationId) {
-            if (!newNotificationId || chrome.runtime.lastError) {
-              var errorMessage = chrome.runtime.lastError &&
-                                 chrome.runtime.lastError.message;
-              console.error('notifications.create: ID=' + newNotificationId +
-                            ', ERROR=' + errorMessage);
-              return;
-            }
-
-            if (onCardShown !== undefined)
-              onCardShown(cardCreateInfo);
-
-            scheduleHiding(cardId, cardCreateInfo.hideTime);
-          });
-    } else {
-      // Update existing notification.
-      instrumented.notifications.update(
-          cardId,
-          cardCreateInfo.notification,
-          function(wasUpdated) {
-            if (!wasUpdated || chrome.runtime.lastError) {
-              var errorMessage = chrome.runtime.lastError &&
-                                 chrome.runtime.lastError.message;
-              console.error('notifications.update: UPDATED=' + wasUpdated +
-                            ', ERROR=' + errorMessage);
-              return;
-            }
-
-            scheduleHiding(cardId, cardCreateInfo.hideTime);
-          });
+      callback(uncombinedNotification, shouldShow && !shouldHide);
     }
   }
 
   /**
-   * Updates/creates a card notification with new data.
-   * @param {string} cardId Card ID.
-   * @param {MergedCard} card Google Now card from the server.
-   * @param {number=} previousVersion The version of the shown card with
-   *     this id, if it exists, undefined otherwise.
-   * @param {function(CardCreateInfo)=} onCardShown Optional parameter called
-   *     when each card is shown.
-   * @return {Object} Notification data entry for this card.
+   * Refreshes (shows/hides) the notification corresponding to the combined card
+   * based on the current time and show-hide intervals in the combined card.
+   * @param {ChromeNotificationId} cardId Card ID.
+   * @param {CombinedCard} combinedCard Combined cards with |cardId|.
+   * @param {function(ReceivedNotification)=} onCardShown Optional parameter
+   *     called when each card is shown.
+   * @return {(NotificationDataEntry|undefined)} Notification data entry for
+   *     this card. It's 'undefined' if the card's life is over.
    */
-  function update(cardId, card, previousVersion, onCardShown) {
-    console.log('cardManager.update ' + JSON.stringify(card) + ' ' +
-        previousVersion);
-
-    chrome.alarms.clear(cardHidePrefix + cardId);
-
-    var cardCreateInfo = {
-      notification: card.notification,
-      hideTime: card.trigger.hideTime,
-      version: card.version,
-      previousVersion: previousVersion,
-      locationBased: card.locationBased
-    };
+  function update(cardId, combinedCard, onCardShown) {
+    console.log('cardManager.update ' + JSON.stringify(combinedCard));
+
+    chrome.alarms.clear(alarmPrefix + cardId);
+    var now = Date.now();
+    /** @type {?UncombinedNotification} */
+    var winningCard = null;
+    // Next moment of time when winning notification selection algotithm can
+    // potentially return a different notification.
+    /** @type {?number} */
+    var nextEventTime = null;
+
+    // Find a winning uncombined notification: a highest-priority notification
+    // that needs to be shown now.
+    enumerateUncombinedNotifications(
+        combinedCard, now, function(uncombinedCard, visible) {
+      // If the uncombined notification is visible now and set the winning card
+      // to it if its priority is higher.
+      if (visible) {
+        if (!winningCard ||
+            uncombinedCard.receivedNotification.chromeNotificationOptions.
+                priority >
+            winningCard.receivedNotification.chromeNotificationOptions.
+                priority) {
+          winningCard = uncombinedCard;
+        }
+      }
+
+      // Next event time is the closest hide or show event.
+      if (uncombinedCard.showTime && uncombinedCard.showTime > now) {
+        if (!nextEventTime || nextEventTime > uncombinedCard.showTime)
+          nextEventTime = uncombinedCard.showTime;
+      }
+      if (uncombinedCard.hideTime > now) {
+        if (!nextEventTime || nextEventTime > uncombinedCard.hideTime)
+          nextEventTime = uncombinedCard.hideTime;
+      }
+    });
+
+    // Show/hide the winning card.
+    updateNotification(
+        cardId, winningCard && winningCard.receivedNotification, onCardShown);
+
+    if (nextEventTime) {
+      // If we expect more events, create an alarm for the next one.
+      chrome.alarms.create(alarmPrefix + cardId, {when: nextEventTime});
+
+      // The trick with stringify/parse is to create a copy of action URLs,
+      // otherwise notifications data with 2 pointers to the same object won't
+      // be stored correctly to chrome.storage.

[skare_, 2013/12/06 03:44:00]

side question: is this known? intended? if no, worth filing a bug for?

[vadimt, 2013/12/06 18:58:49]

This behavior makes sense to me. This is a good way to avoid infinite
serialization if the data structure contains loop. It seems to work like: if you
serialize same object second time, serialize it as null.

[robliao, 2013/12/07 00:51:29]

This sounds like a gotcha of the storage system and should be documented. I
suspect they're storing objects as JSON underneath, which doesn't do so well
with circular structures.

Alternatively, they can simply just handle loops (costs a bit more in the
serialized representation, but very easy to store).

[vadimt, 2013/12/07 01:04:24]

I'll let them know.

+      var winningActionUrls = winningCard &&
+          JSON.parse(JSON.stringify(
+              winningCard.receivedNotification.actionUrls));
 
-    var shownImmediately = false;
-    var cardShowAlarmName = cardShowPrefix + cardId;
-    if (card.trigger.showTime && card.trigger.showTime > Date.now()) {
-      // Card needs to be shown later.
-      console.log('cardManager.update: postponed');
-      var alarmInfo = {
-        when: card.trigger.showTime
+      return {
+        actionUrls: winningActionUrls,
+        timestamp: now,
+        combinedCard: combinedCard
       };
-      chrome.alarms.create(cardShowAlarmName, alarmInfo);
     } else {
-      // Card needs to be shown immediately.
-      console.log('cardManager.update: immediate');
-      chrome.alarms.clear(cardShowAlarmName);
-      showNotification(cardId, cardCreateInfo, onCardShown);
+      // If there are no more events, we are done with this card.
+      verify(!winningCard, 'No events left, but card is shown.');
+      clearCardFromGroups(cardId);
+      return undefined;
     }
+  }
+
+  /**
+   * Removes dismissed part of a card and refreshes the card. Returns remaining
+   * dismissals for the combined card and updated notification data.
+   * @param {ChromeNotificationId} cardId Card ID.
+   * @param {NotificationDataEntry} notificationData Stored notification entry
+   *     for this card.
+   * @return {{
+   *   dismissals: Array.<DismissalData>,
+   *   notificationData: (NotificationDataEntry|undefined)
+   * }}
+   */
+  function onDismissal(cardId, notificationData) {
+    var dismissals = [];
+    var newCombinedCard = [];
+
+    // Determine which parts of the combined card need to be dismissed or to be
+    // preserved. We dismiss parts that were visible at the moment when the card
+    // was last time updated.
+    enumerateUncombinedNotifications(
+      notificationData.combinedCard,
+      notificationData.timestamp,
+      function(uncombinedCard, visible) {
+        if (visible) {
+          dismissals.push({
+            notificationId: uncombinedCard.receivedNotification.notificationId,
+            parameters: uncombinedCard.receivedNotification.dismissal
+          });
+        } else {
+          newCombinedCard.push(uncombinedCard);
+        }
+      });
 
     return {
-      actionUrls: card.actionUrls,
-      cardCreateInfo: cardCreateInfo,
-      dismissals: card.dismissals
+      dismissals: dismissals,
+      notificationData: update(cardId, newCombinedCard)
     };
   }
 
   /**
-   * Removes a card notification.
-   * @param {string} cardId Card ID.
-   * @param {boolean} clearStorage True if the information associated with the
-   *     card should be erased from chrome.storage.
+   * Removes a card information from 'notificationGroups'.
+   * @param {ChromeNotificationId} cardId Card ID.
    */
-  function clear(cardId, clearStorage) {
-    console.log('cardManager.clear ' + cardId);
-
-    chrome.notifications.clear(cardId, function() {});
-    chrome.alarms.clear(cardShowPrefix + cardId);
-    chrome.alarms.clear(cardHidePrefix + cardId);
-
-    if (clearStorage) {
-      instrumented.storage.local.get(
-          ['notificationsData', 'notificationGroups'],
-          function(items) {
-            items = items || {};
-            items.notificationsData = items.notificationsData || {};
-            items.notificationGroups = items.notificationGroups || {};
-
-            delete items.notificationsData[cardId];
-
-            for (var groupName in items.notificationGroups) {
-              var group = items.notificationGroups[groupName];
-              for (var i = 0; i != group.cards.length; ++i) {
-                if (group.cards[i].chromeNotificationId == cardId) {
-                  group.cards.splice(i, 1);
-                  break;
-                }
-              }
-            }
-
-            chrome.storage.local.set(items);
-          });
-    }
+  function clearCardFromGroups(cardId) {
+    console.log('cardManager.clearCardFromGroups ' + cardId);
+
+    instrumented.storage.local.get('notificationGroups', function(items) {
+      items = items || {};
+      /** @type {Object.<string, StoredNotificationGroup>} */
+      items.notificationGroups = items.notificationGroups || {};
+
+      for (var groupName in items.notificationGroups) {
+        var group = items.notificationGroups[groupName];
+        for (var i = 0; i != group.cards.length; ++i) {
+          if (group.cards[i].chromeNotificationId == cardId) {
+            group.cards.splice(i, 1);
+            break;
+          }
+        }
+      }
+
+      chrome.storage.local.set(items);
+    });
   }
 
   instrumented.alarms.onAlarm.addListener(function(alarm) {
     console.log('cardManager.onAlarm ' + JSON.stringify(alarm));
 
-    if (alarm.name.indexOf(cardShowPrefix) == 0) {
+    if (alarm.name.indexOf(alarmPrefix) == 0) {
       // Alarm to show the card.
-      tasks.add(SHOW_CARD_TASK_NAME, function() {
-        var cardId = alarm.name.substring(cardShowPrefix.length);
+      tasks.add(UPDATE_CARD_TASK_NAME, function() {
+        var cardId = alarm.name.substring(alarmPrefix.length);
         instrumented.storage.local.get('notificationsData', function(items) {
           console.log('cardManager.onAlarm.get ' + JSON.stringify(items));
-          if (!items || !items.notificationsData)
-            return;
-          var notificationData = items.notificationsData[cardId];
-          if (!notificationData)
-            return;
+          items = items || {};
+          /** @type {Object.<string, NotificationDataEntry>} */
+          items.notificationsData = items.notificationsData || {};
+          var combinedCard =
+            (items.notificationsData[cardId] &&
+             items.notificationsData[cardId].combinedCard) || [];
 
           var cardShownCallback = undefined;
           if (localStorage['locationCardsShown'] <
@@ -246,21 +341,17 @@ function buildCardSet() {
              cardShownCallback = countLocationCard;
           }
 
-          showNotification(
-              cardId, notificationData.cardCreateInfo, cardShownCallback);
+          items.notificationsData[cardId] =
+              update(cardId, combinedCard, cardShownCallback);
+
+          chrome.storage.local.set(items);
         });
       });
-    } else if (alarm.name.indexOf(cardHidePrefix) == 0) {
-      // Alarm to hide the card.
-      tasks.add(CLEAR_CARD_TASK_NAME, function() {
-        var cardId = alarm.name.substring(cardHidePrefix.length);
-        clear(cardId, true);
-      });
     }
   });
 
   return {
     update: update,
-    clear: clear
+    onDismissal: onDismissal
   };
 }