Processing groups with nextPollSeconds === undefined

Processing groups with nextPollSeconds === undefined.

In theory, the server can generate a group with requested == true, but nextPollSeconds === undefined.

This change makes the client correctly process this.

BUG=164227
TEST=No

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=230333

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

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:112: * (1) never sent
'nextPollSeconds' for the group or (2) didn't send
Linebreak (2)

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:438: if (group.nextPollTime
!== undefined) {
!= for consistency. It would be nice to start using the === and !== forms
though...

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:597: if (group.nextPollTime
!== undefined && group.nextPollTime <= now)
!=

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

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:112: * (1) never sent
'nextPollSeconds' for the group or (2) didn't send
On 2013/10/21 21:42:21, Robert Liao wrote:
> Linebreak (2)

Done.

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:438: if (group.nextPollTime
!== undefined) {
On 2013/10/21 21:42:21, Robert Liao wrote:
> != for consistency. It would be nice to start using the === and !== forms
> though...

!= 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...

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:597: if (group.nextPollTime
!== undefined && group.nextPollTime <= now)
On 2013/10/21 21:42:21, Robert Liao wrote:
> !=

See above.

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

LGTM

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/33433003/diff/1/chrome/browser/resources/goog...
chrome/browser/resources/google_now/background.js:438: if (group.nextPollTime
!== undefined) {
=== 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:
> On 2013/10/21 21:42:21, Robert Liao wrote:
> > != for consistency. It would be nice to start using the === and !== forms
> > though...
> 
> != 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...

[skare_, 2013-10-22 17:16:03]

lgtm

[commit-bot: I haz the power, 2013-10-22 17:32:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/33433003/60001

[rgustafson, 2013-10-22 17:36:49]

lgtm

[commit-bot: I haz the power, 2013-10-23 02:40:06]

Retried try job too often on win7_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-10-23 02:42:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/vadimt@chromium.org/33433003/60001

[commit-bot: I haz the power, 2013-10-23 05:16:57]

Change committed as 230333