Test code to see if a timer fixes the issue for the majority of cases.

Test code to see if a timer fixes the issue for the majority of cases.

Our Push Messaging test fails a fraction of the time (perhaps 10-20% of the
time).  The way that the push messaging test works is that it is a buildbot
which builds the source and then runs the test, so to test this I need to check
the source code in for the buildbot to run the test for me, even though the code
is not final.

The current theory is that the Push Messaging Canary Test is exiting immediately
after recieving the push message from the wire. The invalidation code (Push
Messaging relies on A sync invalidation service, which has a library that
reports reciept of the message back the server.  It seems that the reports back
to the server are not reliable, so the server thinks the client is flaky.

To investigate a fix to this, I'll add code which delays the time test by two
seconds to allow the ack to propagate back to the invalidation service.

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

[Pete Williamson, 2013-04-08 21:33:05]

Mike - please review

Justin - Adding you so that you can start to get familiar with our Push
Messaging code, please come see me if you have any questions.

DCheng - FYI

Our Push Messaging test fails a fraction of the time (perhaps 10-20% of the
time).  The way that the push messaging test works is that it is a buildbot
which builds the source and then runs the test, so to test this I need to check
the source code in for the buildbot to run the test for me, even though the code
is not final.

The current theory is that the Push Messaging Canary Test is exiting immediately
after recieving the push message from the wire. The invalidation code (Push
Messaging relies on Tango, which is an invalidation service) has a library that
reports reciept of the message back the server.  It seems that the reports back
to the server are not reliable, so the server thinks the client is flaky.

To investigate a fix to this, I'll add code which delays the time test by two
seconds to allow the ack to propagate back to tango.

[dcheng, 2013-04-08 21:38:22]

On 2013/04/08 21:33:05, Pete Williamson wrote:
> Mike - please review
> 
> Justin - Adding you so that you can start to get familiar with our Push
> Messaging code, please come see me if you have any questions.
> 
> DCheng - FYI
> 
> Our Push Messaging test fails a fraction of the time (perhaps 10-20% of the
> time).  The way that the push messaging test works is that it is a buildbot
> which builds the source and then runs the test, so to test this I need to
check
> the source code in for the buildbot to run the test for me, even though the
code
> is not final.
> 
> The current theory is that the Push Messaging Canary Test is exiting
immediately
> after recieving the push message from the wire. The invalidation code (Push
> Messaging relies on Tango, which is an invalidation service) has a library
that
> reports reciept of the message back the server.  It seems that the reports
back
> to the server are not reliable, so the server thinks the client is flaky.
> 
> To investigate a fix to this, I'll add code which delays the time test by two
> seconds to allow the ack to propagate back to tango.

Please make the patch description more... descriptive =)
If this patch came by on build.chromium.org, I would have no idea what it's for.

[dcheng, 2013-04-08 21:38:54]

https://codereview.chromium.org/13483025/diff/1/chrome/test/data/extensions/a...
File
chrome/test/data/extensions/api_test/push_messaging_canary/push_messaging_canary.js
(right):

https://codereview.chromium.org/13483025/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/push_messaging_canary/push_messaging_canary.js:22:
// We need to allow time for tango to ACK before we shutdown
Indent for a {} block is two spaces.

[miket_OOO, 2013-04-08 22:07:20]

LGTM, agreed with dcheng, please improve description if possible.

https://codereview.chromium.org/13483025/diff/1/chrome/test/data/extensions/a...
File
chrome/test/data/extensions/api_test/push_messaging_canary/push_messaging_canary.js
(right):

https://codereview.chromium.org/13483025/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/push_messaging_canary/push_messaging_canary.js:24:
}
JS style -- more common to see }); in this case.

[miket_OOO, 2013-04-08 22:07:49]

Also removed reviewer "Mike Tsao," who is not me.

[commit-bot: I haz the power, 2013-04-08 22:43:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/petewil@chromium.org/13483025/1

[dcheng, 2013-04-08 22:44:56]

On 2013/04/08 22:43:07, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/petewil%40chromium.org/13483025/1

Don't forget to upload an updated snapshot before CQing this. =)

[Pete Williamson, 2013-04-08 23:17:41]

CR fix suggestions per DCheng, DewittJ, and MikeT

https://codereview.chromium.org/13483025/diff/1/chrome/test/data/extensions/a...
File
chrome/test/data/extensions/api_test/push_messaging_canary/push_messaging_canary.js
(right):

https://codereview.chromium.org/13483025/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/push_messaging_canary/push_messaging_canary.js:22:
// We need to allow time for tango to ACK before we shutdown
On 2013/04/08 21:38:54, dcheng wrote:
> Indent for a {} block is two spaces.

Done.

https://codereview.chromium.org/13483025/diff/1/chrome/test/data/extensions/a...
chrome/test/data/extensions/api_test/push_messaging_canary/push_messaging_canary.js:24:
}
On 2013/04/08 22:07:20, miket wrote:
> JS style -- more common to see }); in this case.

Done.

[commit-bot: I haz the power, 2013-04-08 23:18:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/petewil@chromium.org/13483025/20001

[commit-bot: I haz the power, 2013-04-09 00:08:45]

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

[commit-bot: I haz the power, 2013-04-09 00:59:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/petewil@chromium.org/13483025/20001

[commit-bot: I haz the power, 2013-04-09 05:20:41]

Change committed as 193015

[Pete Williamson, 2013-04-09 23:26:57]

2 seconds didn't fix the problem.  Trying again with 20 seconds.

MikeT, please take another look.

[miket_OOO, 2013-04-10 15:35:48]

LGTM, good luck.