Processing groups with nextPollSeconds === undefined

chrome/browser/resources/google_now/background.js

Index: chrome/browser/resources/google_now/background.js
diff --git a/chrome/browser/resources/google_now/background.js b/chrome/browser/resources/google_now/background.js
index 0d8de446b052525c584f95529db8ccefd22915d8..2a5507d50147b805bb1a40cc29d592a1adeaf20d 100644
--- a/chrome/browser/resources/google_now/background.js
+++ b/chrome/browser/resources/google_now/background.js
@@ -108,12 +108,15 @@ var UnmergedNotification;
 
 /**
  * Notification group as the client stores it. |cardsTimestamp| and |rank| are
- * defined if |cards| is non-empty.
+ * defined if |cards| is non-empty. |nextPollTime| is undefined if the server
+ * (1) never sent 'nextPollSeconds' for the group or (2) didn't send

[robliao, 2013/10/21 21:42:21]

Linebreak (2)

[vadimt, 2013/10/21 22:37:03]

Done.

+ * 'nextPollSeconds' with the last group update containing a cards update and
+ * all the times after that.
  *
  * @typedef {{
  *   cards: Array.<UnmergedNotification>,
  *   cardsTimestamp: number=,
- *   nextPollTime: number,
+ *   nextPollTime: number=,
  *   rank: number=
  * }}
  */
@@ -432,10 +435,13 @@ function scheduleNextPoll(groups) {
 
   for (var groupName in groups) {
     var group = groups[groupName];
-    nextPollTime = nextPollTime == null ?
-        group.nextPollTime : Math.min(group.nextPollTime, nextPollTime);
+    if (group.nextPollTime !== undefined) {

[robliao, 2013/10/21 21:42:21]

!= for consistency. It would be nice to start using the === and !== forms
though...

[vadimt, 2013/10/21 22:37:03]

!= with undefined has a complicated semantics, which I don't mean here.
What I want to express is typeof(nextPollTime) == 'undefined'. This is exactly
what is meant by !==.
I consistently use !== and === in such cases.

In other places, where types of arguments are same, I use == and !=.
I believe that given the JS ugliness, this is quite adequate...

[robliao, 2013/10/21 22:57:05]

=== and !== are easier to understand (and likely what we really want).

Maybe we should look to a cleanup when things stabilize.
On 2013/10/21 22:37:03, vadimt wrote:

+      nextPollTime = nextPollTime == null ?
+          group.nextPollTime : Math.min(group.nextPollTime, nextPollTime);
+    }
   }
 
+  // At least one of the groups must have nextPollTime.
   verify(nextPollTime != null, 'scheduleNextPoll: nextPollTime is null');
 
   var nextPollDelaySeconds = Math.max(
@@ -494,7 +500,7 @@ function parseAndShowNotificationCards(response) {
       var storageGroup = items.notificationGroups[groupName] || {
         cards: [],
         cardsTimestamp: undefined,
-        nextPollTime: now,
+        nextPollTime: undefined,
         rank: undefined
       };
 
@@ -502,11 +508,20 @@ function parseAndShowNotificationCards(response) {
         receivedGroup.cards = receivedGroup.cards || [];
 
       if (receivedGroup.cards) {
+        // If the group contains a cards update, all its fields will get new
+        // values.
         storageGroup.cards = receivedGroup.cards;
         storageGroup.cardsTimestamp = now;
         storageGroup.rank = receivedGroup.rank;
+        storageGroup.nextPollTime = undefined;
+        // The code below assigns nextPollTime a defined value if
+        // nextPollSeconds is specified in the received group.
+        // If the group's cards are not updated, and nextPollSeconds is
+        // unspecified, this method doesn't change group's nextPollTime.
       }
 
+      // 'nextPollSeconds' may be sent even for groups that don't contain cards
+      // updates.
       if (receivedGroup.nextPollSeconds !== undefined) {
         storageGroup.nextPollTime =
             now + receivedGroup.nextPollSeconds * MS_IN_SECOND;
@@ -579,7 +594,7 @@ function requestNotificationCards(position) {
 
       for (var groupName in items.notificationGroups) {
         var group = items.notificationGroups[groupName];
-        if (group.nextPollTime <= now)
+        if (group.nextPollTime !== undefined && group.nextPollTime <= now)

[robliao, 2013/10/21 21:42:21]

!=

[vadimt, 2013/10/21 22:37:03]

See above.

           groupsToRequest.push(groupName);
       }
     }