Chrome Now notificationGroups Storage Race Condition Fix

chrome/browser/resources/google_now/cards.js

 // Copyright 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';
 
 /**
  * Show/hide trigger in a card.
  *
  * @typedef {{
@@ skipping 165 matching lines @@
 
       callback(uncombinedNotification, shouldShow && !shouldHide);
     }
   }
 
   /**
    * 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 {Object.<string, StoredNotificationGroup>} notificationGroups
+   *     Map from group name to group information.
    * @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, combinedCard, onCardShown) {
+  function update(cardId, combinedCard, notificationGroups, 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;
@@ skipping 45 matching lines @@
 
       return {
         actionUrls: winningActionUrls,
         timestamp: now,
         combinedCard: combinedCard
       };
     } else {
       // If there are no more events, we are done with this card. Note that all
       // received notifications have hideTime.
       verify(!winningCard, 'No events left, but card is shown.');
-      clearCardFromGroups(cardId);
+      clearCardFromGroups(cardId, notificationGroups);
       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.
@@ skipping 18 matching lines @@
             notificationId: uncombinedCard.receivedNotification.notificationId,
             parameters: uncombinedCard.receivedNotification.dismissal
           });
         } else {
           newCombinedCard.push(uncombinedCard);
         }
       });
 
     return {
       dismissals: dismissals,
       notificationData: update(cardId, newCombinedCard)

[vadimt, 2013/12/13 18:20:54]

Please pass notificationGroups here too.

[robliao, 2013/12/13 19:31:26]

Done.

     };
   }
 
   /**
-   * Removes card information from 'notificationGroups'.
+   * Removes card information from |notificationGroups|.
    * @param {ChromeNotificationId} cardId Card ID.
+   * @param {Object.<string, StoredNotificationGroup>} notificationGroups
+   *     Map from group name to group information.
    */
-  function clearCardFromGroups(cardId) {
+  function clearCardFromGroups(cardId, notificationGroups) {
     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;
-          }
+    for (var groupName in notificationGroups) {
+      var group = 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(alarmPrefix) == 0) {
       // Alarm to show the card.
       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));
           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'] <
               LOCATION_CARDS_LINK_THRESHOLD) {
              cardShownCallback = countLocationCard;
           }
 
           items.notificationsData[cardId] =
               update(cardId, combinedCard, cardShownCallback);

[vadimt, 2013/12/13 18:20:54]

Please pass notificationGroups here too.

[robliao, 2013/12/13 19:31:26]

Done.

 
           chrome.storage.local.set(items);
         });
       });
     }
   });
 
   return {
     update: update,
     onDismissal: onDismissal
   };
 }