Implement more of chrome.serial on the Mojo SerialConnection.

Implement more of chrome.serial on the Mojo SerialConnection.

This implements connect, disconnect, getInfo, update, getControlSignals,
setControlSignals, flush, setPaused and getConnections.

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

[Ken Rockot(use gerrit already), 2014-07-31 16:28:33]

lgtm. May want a mojo reviewer too?

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/api/...
File extensions/renderer/api/serial/serial_api_unittest.cc (right):

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/api/...
extensions/renderer/api/serial/serial_api_unittest.cc:156: signals->dcd =
num_calls() & 1;
clever

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
File extensions/renderer/resources/serial_custom_bindings.js (right):

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
extensions/renderer/resources/serial_custom_bindings.js:15: }
nit: missing ;

I realize it's a legal omission in this case, but the style guide doesn't make
any exceptions to the "always use a semicolon" rule, so I'm mentioning it here.

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
extensions/renderer/resources/serial_custom_bindings.js:33: }
nit: missing ;

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
File extensions/renderer/resources/serial_service.js (right):

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
extensions/renderer/resources/serial_service.js:78: 
I'm guessing this conversion stuff is some of the unfortunate boilerplate that
was mentioned in the meeting yesterday.

[Sam McNally, 2014-08-01 01:10:13]

darin: do you want to take a look?

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
File extensions/renderer/resources/serial_custom_bindings.js (right):

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
extensions/renderer/resources/serial_custom_bindings.js:15: }
On 2014/07/31 16:28:33, Ken Rockot wrote:
> nit: missing ;
> 
> I realize it's a legal omission in this case, but the style guide doesn't make
> any exceptions to the "always use a semicolon" rule, so I'm mentioning it
here.

Done.

https://codereview.chromium.org/423403002/diff/80001/extensions/renderer/reso...
extensions/renderer/resources/serial_custom_bindings.js:33: }
On 2014/07/31 16:28:33, Ken Rockot wrote:
> nit: missing ;

Done.

[raymes, 2014-08-01 02:16:28]

The tests here are really cool! Very excited about it.

Could you explain a bit why we need serial_custom_bindings.js and
serial_service.js?  Couldn't we expose the API that extensions needs directly
from serial_service.js?

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
File extensions/renderer/resources/serial_service.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
extensions/renderer/resources/serial_service.js:131: return Promise.all([info,
allocateConnectionId()]);
the indentation here looks odd but I see it throughout the file? I would expect
to indent 2 spaces from the above line?

[Sam McNally, 2014-08-01 06:39:24]

On 2014/08/01 02:16:28, raymes wrote:
> The tests here are really cool! Very excited about it.
> 
> Could you explain a bit why we need serial_custom_bindings.js and
> serial_service.js?  Couldn't we expose the API that extensions needs directly
> from serial_service.js?

The split is because the JS implementation has dependencies that must be loaded
asynchronously (e.g. mojo bindings modules), but the user-facing API must be
available synchronously. It's possible to manage this in a single file, but it
requires using promises for each access to any dependencies and that ends up
being quite messy. With it split into two files, serial_service.js can access
its dependencies synchronously, while serial_custom_bindings.js provides
synchronous access to functions that handle both getting serial_service.js as
well as calling the appropriate function that it provides.

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
File extensions/renderer/resources/serial_service.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
extensions/renderer/resources/serial_service.js:131: return Promise.all([info,
allocateConnectionId()]);
On 2014/08/01 02:16:28, raymes wrote:
> the indentation here looks odd but I see it throughout the file? I would
expect
> to indent 2 spaces from the above line?

It's to maintain 2 space indent per scope.

[raymes, 2014-08-14 02:58:05]

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/api...
File extensions/renderer/api/serial/serial_api_unittest.cc (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/api...
extensions/renderer/api/serial/serial_api_unittest.cc:98: class
TestIoHandlerBase : public device::TestSerialIoHandler {
Consider merging these classes together into a single TestIOHandler.

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
File extensions/renderer/resources/serial_custom_bindings.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
extensions/renderer/resources/serial_custom_bindings.js:6: 
Please add a comment to this file explaining what it's doing.

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
File extensions/renderer/resources/serial_service.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
extensions/renderer/resources/serial_service.js:4: 
Please add a comment to this file explaining what it's doing.

https://codereview.chromium.org/423403002/diff/100001/extensions/test/data/se...
File extensions/test/data/serial_unittest.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/test/data/se...
extensions/test/data/serial_unittest.js:8: 
Please add a comment to this file explaining what it's doing.

[raymes, 2014-08-14 02:59:11]

lgtm

[Sam McNally, 2014-08-14 04:33:51]

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/api...
File extensions/renderer/api/serial/serial_api_unittest.cc (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/api...
extensions/renderer/api/serial/serial_api_unittest.cc:98: class
TestIoHandlerBase : public device::TestSerialIoHandler {
On 2014/08/14 02:58:04, raymes wrote:
> Consider merging these classes together into a single TestIOHandler.

Acknowledged.

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
File extensions/renderer/resources/serial_custom_bindings.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
extensions/renderer/resources/serial_custom_bindings.js:6: 
On 2014/08/14 02:58:04, raymes wrote:
> Please add a comment to this file explaining what it's doing.

Done.

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
File extensions/renderer/resources/serial_service.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/renderer/res...
extensions/renderer/resources/serial_service.js:4: 
On 2014/08/14 02:58:05, raymes wrote:
> Please add a comment to this file explaining what it's doing.

Done.

https://codereview.chromium.org/423403002/diff/100001/extensions/test/data/se...
File extensions/test/data/serial_unittest.js (right):

https://codereview.chromium.org/423403002/diff/100001/extensions/test/data/se...
extensions/test/data/serial_unittest.js:8: 
On 2014/08/14 02:58:05, raymes wrote:
> Please add a comment to this file explaining what it's doing.

Done.

[Sam McNally, 2014-08-14 04:34:34]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2014-08-14 04:38:32]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sammc@chromium.org/423403002/140001

[Sam McNally, 2014-08-14 08:03:59]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2014-08-14 08:07:11]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sammc@chromium.org/423403002/160001

[commit-bot: I haz the power, 2014-08-14 15:05:22]

Committed patchset #5 (160001) as 289578