Add serialization support to the JS DataSender and DataReceiver.

extensions/test/data/data_sender_unittest.js

Index: extensions/test/data/data_sender_unittest.js
diff --git a/extensions/test/data/data_sender_unittest.js b/extensions/test/data/data_sender_unittest.js
index 78568c90740baa68a8c0035ce3be8c7e6cbd7470..7b892a9b605d3d255c6b32e8a3e3637db612c14b 100644
--- a/extensions/test/data/data_sender_unittest.js
+++ b/extensions/test/data/data_sender_unittest.js
@@ -26,7 +26,7 @@ function createSender() {
     requireAsync('content/public/renderer/service_provider'),
     requireAsync('data_sender'),
     requireAsync('device/serial/data_stream.mojom'),
-  ]).then(test.callbackPass(function(modules) {
+  ]).then(function(modules) {
     var serviceProvider = modules[0];
     var dataSender = modules[1];
     var dataStream = modules[2];
@@ -34,185 +34,280 @@ function createSender() {
         serviceProvider.connectToService(dataStream.DataSinkProxy.NAME_),
         BUFFER_SIZE,
         FATAL_ERROR);
-  }));
+  });
+}
+
+function sendAndExpectSuccess(data, expectedBytesSent) {

[raymes, 2014/09/18 03:16:07]

Please add comments for the helpers here too

[Sam McNally, 2014/09/19 04:58:48]

Done.

+  return function(sender) {
+    return sender.send(data).then(function(bytesSent) {
+      test.assertEq(expectedBytesSent, bytesSent);
+      return sender;
+    });
+  };
+}
+
+function sendAndExpectError(data, expectedError, expectedBytesSent) {
+  return function(sender) {
+    return sender.send(data).catch(function(result) {
+      test.assertEq(expectedError, result.error);
+      test.assertEq(expectedBytesSent, result.bytesSent);
+      return sender;
+    });
+  };
+}
+
+function cancelSend(cancelReason) {
+  return function(sender) {
+    return sender.cancel(cancelReason).then(function() {
+      return sender;
+    });
+  };
+}
+
+function expectOrder(promises) {
+  var nextIndex = 0;
+  function createOrderChecker(promise, expectedIndex) {
+    return promise.then(function(sender) {
+      test.assertEq(nextIndex, expectedIndex);
+      nextIndex++;
+      return sender;
+    });
+  }
+  var wrappedPromises = [];
+  for (var i = 0; i < promises.length; i++) {
+    wrappedPromises.push(createOrderChecker(promises[i], i));
+  }
+  return Promise.all(wrappedPromises).then(function(results) {
+    return results[0];
+  });
+}
+
+function serializeRoundTrip(sender) {
+  return Promise.all([
+    sender.serialize(),
+    requireAsync('data_sender'),
+  ]).then(function(promises) {
+    var serialized = promises[0];
+    var dataSender = promises[1];

[raymes, 2014/09/18 03:16:07]

dataSenderModule

[Sam McNally, 2014/09/19 04:58:47]

Done.

+    return promises[1].DataSender.deserialize(promises[0]);

[raymes, 2014/09/18 03:16:07]

same here, use the variable names :)

[Sam McNally, 2014/09/19 04:58:48]

Done.

+  });
+}
+
+function closeSender(sender) {
+  sender.close();
+  return sender;
 }
 
 unittestBindings.exportTests([
   function testSend() {
-    createSender().then(test.callbackPass(function(sender) {
-      var seen = null;
-      sender.send(generateData(1)).then(test.callbackPass(function(bytesSent) {
-        test.assertEq(1, bytesSent);
-        test.assertEq(null, seen);
-        seen = 'first';
-      }));
-      sender.send(generateData(1)).then(test.callbackPass(function(bytesSent) {
-        sender.close();
-        test.assertEq(1, bytesSent);
-        test.assertEq('first', seen);
-        seen = 'second';
-      }));
-    }));
+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectSuccess(generateData(1), 1)),
+        sender.then(sendAndExpectSuccess(generateData(1), 1)),
+    ]).then(closeSender).then(test.succeed, test.fail);
   },
 
   function testLargeSend() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.send(generateData(BUFFER_SIZE * 3, '123')).then(
-          test.callbackPass(function(bytesSent) {
-        test.assertEq(BUFFER_SIZE * 3, bytesSent);
-        sender.close();
-      }));
-    }));
+    createSender()
+        .then(sendAndExpectSuccess(generateData(BUFFER_SIZE * 3, '123'),
+                                     BUFFER_SIZE * 3))

[raymes, 2014/09/18 03:16:07]

I think the indentation should be 2 spaces less here.

[Sam McNally, 2014/09/19 04:58:47]

Done.

+        .then(closeSender).then(test.succeed, test.fail);
   },
 
   function testSendError() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.send(generateData(BUFFER_SIZE * 3, 'b')).catch(test.callbackPass(
-          function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(0, e.bytesSent);
-        sender.send(generateData(1)).then(test.callbackPass(
-            function(bytesSent) {
-          test.assertEq(1, bytesSent);
-          sender.close();
-        }));
-      }));
-    }));
+    createSender()
+        .then(sendAndExpectError(generateData(BUFFER_SIZE * 3, 'b'), 1, 0))
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(closeSender).then(test.succeed, test.fail);
   },
 
   function testSendErrorPartialSuccess() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.send(generateData(BUFFER_SIZE * 3, 'b')).catch(test.callbackPass(
-          function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(5, e.bytesSent);
-        sender.send(generateData(1)).then(test.callbackPass(
-            function(bytesSent) {
-          test.assertEq(1, bytesSent);
-          sender.close();
-        }));
-      }));
-    }));
+    createSender()
+        .then(sendAndExpectError(generateData(BUFFER_SIZE * 3, 'b'), 1, 5))
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(closeSender).then(test.succeed, test.fail);
   },
 
   function testSendErrorBetweenPackets() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.send(generateData(2, 'b')).catch(test.callbackPass(function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(2, e.bytesSent);
-      }));
-      // After an error, all sends in progress will be cancelled.
-      sender.send(generateData(2, 'b')).catch(test.callbackPass(function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(0, e.bytesSent);
-        sender.send(generateData(1)).then(test.callbackPass(
-            function(bytesSent) {
-          test.assertEq(1, bytesSent);
-          sender.close();
-        }));
-      }));
-    }));
+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(2, 'b'), 1, 2)),
+        sender.then(sendAndExpectError(generateData(2, 'b'), 1, 0)),
+    ])
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(closeSender).then(test.succeed, test.fail);
   },
 
   function testSendErrorInSecondPacket() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.send(generateData(2, 'b')).then(test.callbackPass(
-          function(bytesSent) {
-        test.assertEq(2, bytesSent);
-      }));
-      sender.send(generateData(2, 'b')).catch(test.callbackPass(function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(1, e.bytesSent);
-        sender.send(generateData(1)).then(test.callbackPass(
-            function(bytesSent) {
-          test.assertEq(1, bytesSent);
-          sender.close();
-        }));
-      }));
-    }));
+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectSuccess(generateData(2, 'b'), 2)),
+        sender.then(sendAndExpectError(generateData(2, 'b'), 1, 1)),
+    ])
+      .then(sendAndExpectSuccess(generateData(1), 1))
+      .then(closeSender).then(test.succeed, test.fail);

[raymes, 2014/09/18 03:16:07]

I think the indentation is wrong here - it looks different to the above.

[Sam McNally, 2014/09/19 04:58:48]

Done.

   },
 
   function testSendErrorInLargeSend() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.send(generateData(BUFFER_SIZE * 3, '1234567890')).catch(
-          test.callbackPass(function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(12, e.bytesSent);
-        sender.send(generateData(1)).then(test.callbackPass(
-            function(bytesSent) {
-          test.assertEq(1, bytesSent);
-          sender.close();
-        }));
-      }));
-    }));
+    createSender()
+        .then(sendAndExpectError(
+            generateData(BUFFER_SIZE * 3, '1234567890'), 1, 12))
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(closeSender).then(test.succeed, test.fail);
   },
 
   function testSendErrorBeforeLargeSend() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.send(generateData(5, 'b')).catch(test.callbackPass(function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(2, e.bytesSent);
-      }));
-      sender.send(generateData(BUFFER_SIZE * 3, '1234567890')).catch(
-          test.callbackPass(function(e) {
-        test.assertEq(1, e.error);
-        test.assertEq(0, e.bytesSent);
-        sender.send(generateData(1)).then(test.callbackPass(
-            function(bytesSent) {
-          test.assertEq(1, bytesSent);
-          sender.close();
-        }));
-      }));
-    }));
+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(5, 'b'), 1, 2)),
+        sender.then(sendAndExpectError(
+            generateData(BUFFER_SIZE * 3, '1234567890'), 1, 0)),
+    ])
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(closeSender).then(test.succeed, test.fail);
   },
 
   function testCancelWithoutSend() {
-    createSender().then(test.callbackPass(function(sender) {
-      sender.cancel(3).then(test.callbackPass(function() {
-        sender.close();
-      }));
-    }));
+    createSender()
+        .then(cancelSend(3))
+        .then(closeSender).then(test.succeed, test.fail);
   },
 
   function testCancel() {
-    createSender().then(test.callbackPass(function(sender) {
-      var seen = null;
-      sender.send(generateData(1, 'b')).catch(test.callbackPass(function(e) {
-        test.assertEq(3, e.error);
-        test.assertEq(0, e.bytesSent);
-        test.assertEq(null, seen);
-        seen = 'send';
-      }));
-      sender.cancel(3).then(test.callbackPass(function() {
-        test.assertEq('send', seen);
-        seen = 'cancel';
-        sender.close();
-      }));
+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(1, 'b'), 3, 0)),
+        sender.then(cancelSend(3)),
+    ])
+        .then(closeSender).then(test.succeed, test.fail);
+    sender.then(function(sender) {
       test.assertThrows(
           sender.cancel, sender, [], 'Cancel already in progress');
       test.assertThrows(sender.send, sender, [], 'Cancel in progress');
-    }));
+    });
   },
 
   function testClose() {
-    createSender().then(test.callbackPass(function(sender) {
-      var seen = null;
-      sender.send(generateData(1, 'b')).catch(test.callbackPass(function(e) {
-        test.assertEq(FATAL_ERROR, e.error);
-        test.assertEq(0, e.bytesSent);
-        test.assertEq(null, seen);
-        seen = 'send';
-      }));
-      sender.cancel(3).then(test.callbackPass(function() {
-        test.assertEq('send', seen);
-        seen = 'cancel';
-        sender.close();
-      }));
-      sender.close();
+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(1, 'b'), FATAL_ERROR, 0)),
+        sender.then(cancelSend(3)),
+    ]);
+    sender.then(closeSender).then(test.succeed, test.fail)

[raymes, 2014/09/18 03:16:07]

I like having every .then on a separate line :)

[Sam McNally, 2014/09/19 04:58:47]

Done.

+          .then(function(sender) {

[raymes, 2014/09/18 03:16:07]

Won't this run after the test has passed or failed =\

[Sam McNally, 2014/09/19 04:58:47]

Done.

       test.assertThrows(
           sender.cancel, sender, [], 'DataSender has been closed');
       test.assertThrows(sender.send, sender, [], 'DataSender has been closed');
-    }));
+    });
+  },
+
+  function testSendWithSerialization() {
+    var sender = createSender()
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(serializeRoundTrip);
+    expectOrder([
+        sender.then(sendAndExpectSuccess(generateData(1), 1)),
+        sender.then(sendAndExpectSuccess(generateData(1), 1)),
+    ]).then(closeSender).then(test.succeed, test.fail);

[raymes, 2014/09/18 03:16:07]

.then on a new line (and lots below)

[Sam McNally, 2014/09/19 04:58:48]

Done.

+  },
+
+  function testSendErrorWithSerialization() {
+    createSender()
+        .then(sendAndExpectError(generateData(BUFFER_SIZE * 3, 'b'), 1, 0))
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(serializeRoundTrip)
+        .then(sendAndExpectError(generateData(BUFFER_SIZE * 3, 'b'), 3, 0))
+        .then(sendAndExpectSuccess(generateData(1, 'c'), 1))
+        .then(closeSender).then(test.succeed, test.fail);
+  },
+
+  function testSendErrorPartialSuccessWithSerialization() {
+    createSender()
+        .then(sendAndExpectError(generateData(BUFFER_SIZE * 3, 'b'), 1, 5))
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(serializeRoundTrip)
+        .then(sendAndExpectError(generateData(BUFFER_SIZE * 3, 'c'), 3, 5))
+        .then(sendAndExpectSuccess(generateData(1, 'd'), 1))
+        .then(closeSender).then(test.succeed, test.fail);
+  },
+
+  function testSendErrorBetweenPacketsWithSerialization() {

[raymes, 2014/09/18 03:16:07]

Do this test and the next one test something substantially different from the
other tests? Is there particular state we expect to be stored in these cases
that isn't tested in the other cases? It seems to me they might be redundant.

If it is testing something different, it would be good to spell out what the
test is doing a bit more. 

[Sam McNally, 2014/09/19 04:58:48]

Removed these.

+    var sender = createSender();
+    sender = expectOrder([
+        sender.then(sendAndExpectError(generateData(2, 'b'), 1, 2)),
+        sender.then(sendAndExpectError(generateData(2, 'b'), 1, 0)),
+    ])
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(serializeRoundTrip);
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(2, 'c'), 3, 2)),
+        sender.then(sendAndExpectError(generateData(2, 'c'), 3, 0)),
+    ])
+        .then(sendAndExpectSuccess(generateData(1, 'd'), 1))
+        .then(closeSender).then(test.succeed, test.fail);
+  },
+
+  function testSendErrorInSecondPacketWithSerialization() {
+    var sender = createSender();
+    sender = expectOrder([
+        sender.then(sendAndExpectSuccess(generateData(2, 'b'), 2)),
+        sender.then(sendAndExpectError(generateData(2, 'b'), 1, 1)),
+    ])
+        .then(sendAndExpectSuccess(generateData(1), 1))
+        .then(serializeRoundTrip);
+    expectOrder([
+        sender.then(sendAndExpectSuccess(generateData(2, 'c'), 2)),
+        sender.then(sendAndExpectError(generateData(2, 'c'), 3, 1)),
+    ])
+        .then(sendAndExpectSuccess(generateData(1, 'd'), 1))
+        .then(closeSender).then(test.succeed, test.fail);
+
+  },
+
+  function testCancelWithSerialization() {

[raymes, 2014/09/18 03:16:07]

does this test actually check anything?

[Sam McNally, 2014/09/19 04:58:47]

Just that cancel still works the same after serialization.

+    var sender = createSender();
+    sender = expectOrder([
+        sender.then(sendAndExpectError(generateData(1, 'b'), 3, 0)),
+        sender.then(cancelSend(3)),
+    ]).then(serializeRoundTrip);
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(1, 'b'), 4, 0)),
+        sender.then(cancelSend(4)),
+    ]).then(closeSender).then(test.succeed, test.fail);
+    sender.then(function(sender) {
+      test.assertThrows(
+          sender.cancel, sender, [], 'Cancel already in progress');
+      test.assertThrows(sender.send, sender, [], 'Cancel in progress');
+    });
+  },
+
+  function testSerializeWithSendsInProgress() {

[raymes, 2014/09/18 03:16:07]

does this test actually check anything?

[Sam McNally, 2014/09/19 04:58:48]

Yes. This checks that serialize() cancels any sends in progress. Renamed to be
clearer.

+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(1, 'b'), FATAL_ERROR, 0)),
+        sender.then(sendAndExpectError(generateData(1, 'b'), FATAL_ERROR, 0)),
+        sender.then(serializeRoundTrip),
+    ]).then(closeSender).then(test.succeed, test.fail);
+  },
+
+  function testSerializeWithSendAndCancelInProgress() {
+    var sender = createSender();
+    expectOrder([
+        sender.then(sendAndExpectError(generateData(1, 'b'), 3, 0)),
+        sender.then(cancelSend(3)),
+        sender.then(serializeRoundTrip),

[raymes, 2014/09/18 03:16:07]

does this test actually check anything?

[Sam McNally, 2014/09/19 04:58:47]

Yes. This checks that serialize() doesn't override the cancel already in
progress. Renamed to be clearer.

+    ]).then(closeSender).then(test.succeed, test.fail);
+  },
+
+  function testSerializeAfterClose() {
+    function sendAfterClose(sender) {
+      test.assertThrows(sender.send, sender, [], 'DataSender has been closed');
+    }
+
+    createSender()
+        .then(closeSender)
+        .then(serializeRoundTrip)
+        .then(sendAfterClose).then(test.succeed, test.fail);
   },
 
 ], test.runTests, exports);