Push API: Replace buggy FutureData test helper with ResultQueue

Push API: Replace buggy FutureData test helper with ResultQueue

Since FutureData didn't properly clear its state after a get(), it was possible
a piece of data provided via set() to be sent back twice to the test code!

Replaced it with a queue data structure that doesn't have these problems (and
also won't lose data in the event that test code causes two or more results to
be asynchronously generated before getting the results, though so far I think
we've been lucky on that front).

BUG=437277
Committed: https://crrev.com/8863b7da7faae03cc65dd1076c36278e8a6831e9
Cr-Commit-Position: refs/heads/master@{#311304}

[johnme, 2015-01-13 11:06:12]

johnme@chromium.org changed reviewers:
+ mvanouwerkerk@chromium.org

[Michael van Ouwerkerk, 2015-01-13 11:14:51]

https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
File chrome/test/data/push_messaging/push_test.js (right):

https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
chrome/test/data/push_messaging/push_test.js:26: function ResultQueue() {
I don't mind a more powerful class here, but in the past Peter did [1], so
please get him to review this patch.

[1]
https://codereview.chromium.org/673783003/diff/40001/chrome/test/data/push_me...

[johnme, 2015-01-13 11:16:18]

johnme@chromium.org changed reviewers:
+ peter@chromium.org

[Peter Beverloo, 2015-01-13 16:53:38]

lgtm

https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
File chrome/test/data/push_messaging/push_test.js (right):

https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
chrome/test/data/push_messaging/push_test.js:7: var resultQueue = new
ResultQueue();
nit: since this is Chromium, variables should be named "result_queue". While
you're just updating this one, you're also adding pendingGets and it'd be nice
to get this fixed.

https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
chrome/test/data/push_messaging/push_test.js:26: function ResultQueue() {
On 2015/01/13 11:14:51, Michael van Ouwerkerk wrote:
> I don't mind a more powerful class here, but in the past Peter did [1], so
> please get him to review this patch.
> 
> [1]
>
https://codereview.chromium.org/673783003/diff/40001/chrome/test/data/push_me...

Thanks for checking!

I think that the additional complexity introduced by this patch is OK given that
we gain queue functionality. It takes an approach very similar to the FutureData
class you wrote, albeit with a queue, whereas AsyncData supported named data. I
do hope situations in which we'll be waiting for more than one message will be
rare -- otherwise we may need named data after all, in the event message B
arrives before message A, causing the items to be queued in the wrong order.

[johnme, 2015-01-13 18:03:04]

Thanks for review both

https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
File chrome/test/data/push_messaging/push_test.js (right):

https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
chrome/test/data/push_messaging/push_test.js:7: var resultQueue = new
ResultQueue();
On 2015/01/13 16:53:38, Peter Beverloo wrote:
> nit: since this is Chromium, variables should be named "result_queue". While
> you're just updating this one, you're also adding pendingGets and it'd be nice
> to get this fixed.

Actually,
http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml?showone...
says variableNamesLikeThis and it is indeed 5 times more common in
chrome/test/data (excluding third_party)

[johnme, 2015-01-13 18:03:08]

The CQ bit was checked by johnme@chromium.org

[commit-bot: I haz the power, 2015-01-13 18:04:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/822203006/1

[Peter Beverloo, 2015-01-13 18:06:11]

On 2015/01/13 18:03:04, johnme wrote:
> Thanks for review both
> 
>
https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
> File chrome/test/data/push_messaging/push_test.js (right):
> 
>
https://codereview.chromium.org/822203006/diff/1/chrome/test/data/push_messag...
> chrome/test/data/push_messaging/push_test.js:7: var resultQueue = new
> ResultQueue();
> On 2015/01/13 16:53:38, Peter Beverloo wrote:
> > nit: since this is Chromium, variables should be named "result_queue". While
> > you're just updating this one, you're also adding pendingGets and it'd be
nice
> > to get this fixed.
> 
> Actually,
>
http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml?showone...
> says variableNamesLikeThis and it is indeed 5 times more common in
> chrome/test/data (excluding third_party)

Yup, thanks :).

[commit-bot: I haz the power, 2015-01-13 19:21:25]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-01-13 19:22:13]

Patchset 1 (id:??) landed as
https://crrev.com/8863b7da7faae03cc65dd1076c36278e8a6831e9
Cr-Commit-Position: refs/heads/master@{#311304}