Push API: Replace buggy FutureData test helper with ResultQueue

chrome/test/data/push_messaging/push_test.js

Index: chrome/test/data/push_messaging/push_test.js
diff --git a/chrome/test/data/push_messaging/push_test.js b/chrome/test/data/push_messaging/push_test.js
index a02f3aea88cb33d0c9c3a13a5bfa8066daa1d9d1..410a839996cd23f2943e19346f93e01e16080331 100644
--- a/chrome/test/data/push_messaging/push_test.js
+++ b/chrome/test/data/push_messaging/push_test.js
@@ -4,7 +4,7 @@
 
 'use strict';
 
-var pushData = new FutureData();
+var resultQueue = new ResultQueue();

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

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.

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

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)

 var pushRegistration = null;
 
 // Sends data back to the test. This must be in response to an earlier
@@ -21,36 +21,39 @@ function sendErrorToTest(error) {
   sendResultToTest(error.name + ' - ' + error.message);
 }
 
-// A container for a single piece of data. The data does not have to be
-// available yet when the getter is called, as all responses to the test are
-// asynchronous.
-function FutureData() {
-  this.data = null;
-  this.waiting = false;
+// Queue storing asynchronous results received from the Service Worker. Results
+// are sent to the test when requested.
+function ResultQueue() {

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

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

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

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.

+  // Invariant: this.queue.length == 0 || this.pendingGets == 0
+  this.queue = [];
+  this.pendingGets = 0;
 }
 
-// Sends the data to the test if it is available. Otherwise sets the
-// |waiting| flag.
-FutureData.prototype.get = function() {
-  if (this.data) {
-    sendResultToTest(this.data);
+// Adds a data item to the queue. Will be sent to the test if there are
+// pendingGets.
+ResultQueue.prototype.push = function(data) {
+  if (this.pendingGets > 0) {
+    this.pendingGets--;
+    sendResultToTest(data);
   } else {
-    this.waiting = true;
+    this.queue.unshift(data);
   }
 };
 
-// Sets a new data value. If the |waiting| flag is on, it is turned off and
-// the data is sent to the test.
-FutureData.prototype.set = function(data) {
-  this.data = data;
-  if (this.waiting) {
-    sendResultToTest(data);
-    this.waiting = false;
+// Called by native. Sends the next data item to the test if it is available.
+// Otherwise increments pendingGets so it will be delivered when received.
+ResultQueue.prototype.pop = function() {
+  if (this.queue.length) {
+    sendResultToTest(this.queue.pop());
+  } else {
+    this.pendingGets++;
   }
 };
 
-FutureData.prototype.getImmediately = function() {
-  sendResultToTest(this.data);
+// Called by native. Immediately sends the next data item to the test if it is
+// available, otherwise sends null.
+ResultQueue.prototype.popImmediately = function() {
+  sendResultToTest(this.queue.length ? this.queue.pop() : null);
 };
 
 // Notification permission has been coalesced with Push permission. After
@@ -132,7 +135,6 @@ function unregister() {
 
 addEventListener('message', function(event) {
   var message = JSON.parse(event.data);
-  if (message.type == 'push') {
-    pushData.set(message.data);
-  }
+  if (message.type == 'push')
+    resultQueue.push(message.data);
 }, false);