Chrome Now notificationGroups Storage Race Condition Fix

chrome/browser/resources/google_now/background.js

 // Copyright (c) 2013 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.
 
 'use strict';
 
 /**
  * @fileoverview The event page for Google Now for Chrome implementation.
  * The Google Now event page gets Google Now cards from the server and shows
  * them as Chrome notifications.
@@ skipping 275 matching lines @@
         originalOnLoadEnd(event);
       }
     });
 
     callbackBoolean(true);
   });
 }
 
 /**
  * Shows parsed and combined cards as notifications.
+ * @param {Object.<string, StoredNotificationGroup>} notificationGroups Map from
+ *     group name to group information.
  * @param {Object.<ChromeNotificationId, CombinedCard>} cards Map from
  *     chromeNotificationId to the combined card, containing cards to show.
+ * @param {function()} onSuccess Called on success.
  * @param {function(ReceivedNotification)=} onCardShown Optional parameter
  *     called when each card is shown.
  */
-function showNotificationCards(cards, onCardShown) {
+function showNotificationCards(
+    notificationGroups, cards, onSuccess, onCardShown) {
   console.log('showNotificationCards ' + JSON.stringify(cards));
 
   instrumented.notifications.getAll(function(notifications) {
     console.log('showNotificationCards-getAll ' +
         JSON.stringify(notifications));
     notifications = notifications || {};
 
     // Mark notifications that didn't receive an update as having received
     // an empty update.
     for (var chromeNotificationId in notifications) {
       cards[chromeNotificationId] = cards[chromeNotificationId] || [];
     }
 
     /** @type {Object.<string, NotificationDataEntry>} */
     var notificationsData = {};
 
     // Create/update/delete notifications.
     for (var chromeNotificationId in cards) {
       notificationsData[chromeNotificationId] = cardSet.update(
           chromeNotificationId,
           cards[chromeNotificationId],
+          notificationGroups,
           onCardShown);
     }
     chrome.storage.local.set({notificationsData: notificationsData});
+    onSuccess();
   });
 }
 
 /**
  * Removes all cards and card state on Google Now close down.
  * For example, this occurs when the geolocation preference is unchecked in the
  * content settings.
  */
 function removeAllCards() {
   console.log('removeAllCards');
@@ skipping 74 matching lines @@
       updateCardsAttempts.start(optinPollPeriodSeconds);
     });
   }
 }
 
 /**
  * Combines notification groups into a set of Chrome notifications and shows
  * them.
  * @param {Object.<string, StoredNotificationGroup>} notificationGroups Map from
  *     group name to group information.
+ * @param {function()} onSuccess Called on success.
  * @param {function(ReceivedNotification)=} onCardShown Optional parameter
  *     called when each card is shown.
  */
-function combineAndShowNotificationCards(notificationGroups, onCardShown) {
+function combineAndShowNotificationCards(
+    notificationGroups, onSuccess, onCardShown) {
   console.log('combineAndShowNotificationCards ' +
       JSON.stringify(notificationGroups));
   /** @type {Object.<ChromeNotificationId, CombinedCard>} */
   var combinedCards = {};
 
   for (var groupName in notificationGroups)
     combineGroup(combinedCards, notificationGroups[groupName]);
 
-  showNotificationCards(combinedCards, onCardShown);
+  showNotificationCards(
+      notificationGroups, combinedCards, onSuccess, onCardShown);
 }
 
 /**
  * Parses JSON response from the notification server, shows notifications and
  * schedules next update.
  * @param {string} response Server response.
  * @param {function(ReceivedNotification)=} onCardShown Optional parameter
  *     called when each card is shown.
  */
 function parseAndShowNotificationCards(response, onCardShown) {
@@ skipping 81 matching lines @@
           // cards updates.
           if (receivedGroup.nextPollSeconds !== undefined) {
             storedGroup.nextPollTime =
                 now + receivedGroup.nextPollSeconds * MS_IN_SECOND;
           }
 
           updatedGroups[groupName] = storedGroup;
         }
 
         scheduleNextPoll(updatedGroups, !parsedResponse.googleNowDisabled);
-        chrome.storage.local.set({
-          notificationGroups: updatedGroups,
-          recentDismissals: updatedRecentDismissals
-        });
-        combineAndShowNotificationCards(updatedGroups, onCardShown);
-        recordEvent(GoogleNowEvent.CARDS_PARSE_SUCCESS);
+        combineAndShowNotificationCards(updatedGroups,

[vadimt, 2013/12/13 20:07:40]

nit: please start "updatedGroups," from new line

[robliao, 2013/12/13 20:38:17]

Done.

+            function() {
+              chrome.storage.local.set({
+                notificationGroups: updatedGroups,
+                recentDismissals: updatedRecentDismissals
+              });
+              recordEvent(GoogleNowEvent.CARDS_PARSE_SUCCESS);
+            },
+            onCardShown);
       });
 }
 
 /**
  * Update Location Cards Shown Count.
  * @param {ReceivedNotification} receivedNotification Notification as it was
  *     received from the server.
  */
 function countLocationCard(receivedNotification) {
   if (receivedNotification.locationBased) {
@@ skipping 348 matching lines @@
   if (!byUser)
     return;
 
   // At this point we are guaranteed that the notification is a now card.
   chrome.metricsPrivate.recordUserAction('GoogleNow.Dismissed');
 
   tasks.add(DISMISS_CARD_TASK_NAME, function() {
     dismissalAttempts.start();
 
     instrumented.storage.local.get(
-        ['pendingDismissals', 'notificationsData'], function(items) {
-      items = items || {};
-      /** @type {Array.<PendingDismissal>} */
-      items.pendingDismissals = items.pendingDismissals || [];
-      /** @type {Object.<string, NotificationDataEntry>} */
-      items.notificationsData = items.notificationsData || {};
+        ['pendingDismissals', 'notificationsData', 'notificationGroups'],
+        function(items) {
+          items = items || {};
+          /** @type {Array.<PendingDismissal>} */
+          items.pendingDismissals = items.pendingDismissals || [];
+          /** @type {Object.<string, NotificationDataEntry>} */
+          items.notificationsData = items.notificationsData || {};
+          /** @type {Object.<string, StoredNotificationGroup>} */
+          items.notificationGroups = items.notificationGroups || {};
 
-      /** @type {NotificationDataEntry} */
-      var notificationData = items.notificationsData[chromeNotificationId] || {
-        timestamp: Date.now(),
-        combinedCard: []
-      };
+          /** @type {NotificationDataEntry} */
+          var notificationData =
+              items.notificationsData[chromeNotificationId] ||
+              {
+                timestamp: Date.now(),
+                combinedCard: []
+              };
 
-      var dismissalResult =
-          cardSet.onDismissal(chromeNotificationId, notificationData);
+          var dismissalResult =
+              cardSet.onDismissal(
+                  chromeNotificationId,
+                  notificationData,
+                  items.notificationGroups);
 
-      for (var i = 0; i < dismissalResult.dismissals.length; i++) {
-        /** @type {PendingDismissal} */
-        var dismissal = {
-          chromeNotificationId: chromeNotificationId,
-          time: Date.now(),
-          dismissalData: dismissalResult.dismissals[i]
-        };
-        items.pendingDismissals.push(dismissal);
-      }
+          for (var i = 0; i < dismissalResult.dismissals.length; i++) {
+            /** @type {PendingDismissal} */
+            var dismissal = {
+              chromeNotificationId: chromeNotificationId,
+              time: Date.now(),
+              dismissalData: dismissalResult.dismissals[i]
+            };
+            items.pendingDismissals.push(dismissal);
+          }
 
-      items.notificationsData[chromeNotificationId] =
-          dismissalResult.notificationData;
+          items.notificationsData[chromeNotificationId] =
+              dismissalResult.notificationData;
 
-      chrome.storage.local.set({
-        pendingDismissals: items.pendingDismissals,
-        notificationsData: items.notificationsData
-      });
+          chrome.storage.local.set(items);
 
-      processPendingDismissals(function(success) {});
-    });
+          processPendingDismissals(function(success) {});
+        });
   });
 }
 
 /**
  * Initializes the polling system to start monitoring location and fetching
  * cards.
  */
 function startPollingCards() {
   // Create an update timer for a case when for some reason location request
   // gets stuck.
@@ skipping 164 matching lines @@
   // Show notifications received by earlier polls. Doing this as early as
   // possible to reduce latency of showing first notifications. This mimics how
   // persistent notifications will work.
   tasks.add(SHOW_ON_START_TASK_NAME, function() {
     instrumented.storage.local.get('notificationGroups', function(items) {
       console.log('onStartup-get ' + JSON.stringify(items));
       items = items || {};
       /** @type {Object.<string, StoredNotificationGroup>} */
       items.notificationGroups = items.notificationGroups || {};
 
-      combineAndShowNotificationCards(items.notificationGroups);
+      combineAndShowNotificationCards(items.notificationGroups, function() {});

[vadimt, 2013/12/13 20:07:40]

We need to store notificationGroups here too, in case when all notifications in
the combined card have expired, and get deleted from groups.

[robliao, 2013/12/13 20:38:17]

Ah. I for some reason thought we didn't need this. That's easy enough.
On 2013/12/13 20:07:40, vadimt wrote:

     });
   });
 
   initialize();
 });
 
 instrumented.
     preferencesPrivate.
     googleGeolocationAccessEnabled.
     onChange.
@@ skipping 77 matching lines @@
             lastPollNowPayloads: items.lastPollNowPayloads,
             notificationGroups: items.notificationGroups
           });
 
           updateNotificationsCards();
         }
       });
     });
   }
 });