Add serial bulk reads.

Add serial bulk reads.

As a curious shortcut, we originally implemented the serial read function to
return exactly one byte at a time. This might have been back when we didn't
yet have ArrayBuffer support, so it actually was easier to deal with a byte
rather than a buffer. Now we read the requested number of bytes or a smaller
number to avoid blocking.

BUG=136897
TEST=changed existing tests to use new API, and replaced the test byte with a test sequence.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149718

[Peng, 2012-08-01 22:14:00]

http://codereview.chromium.org/10827123/diff/2001/chrome/browser/extensions/a...
File chrome/browser/extensions/api/serial/serial_apitest.cc (right):

http://codereview.chromium.org/10827123/diff/2001/chrome/browser/extensions/a...
chrome/browser/extensions/api/serial/serial_apitest.cc:84: if (read_index_ >=
write_index_) {
As it is a ring bufer, I think just testing read_index >= write_index is not
enough.

http://codereview.chromium.org/10827123/diff/2001/chrome/browser/extensions/a...
chrome/browser/extensions/api/serial/serial_apitest.cc:104:
ring_buffer_[write_index_++] = *data++;
I think here may overwrite unread data.

[miket_OOO, 2012-08-01 23:04:27]

> As it is a ring bufer, I think just testing read_index >= write_index is not
> enough.

You're right; I forgot that I never turned this into a real ring buffer. It's
irrelevant at this point (we're writing 8 bytes into a 256-byte ring buffer),
but the code is in an inconsistent state w.r.t. its naming. I'll fix and patch.

[miket_OOO, 2012-08-02 16:17:46]

> You're right; I forgot that I never turned this into a real ring buffer. It's
> irrelevant at this point (we're writing 8 bytes into a 256-byte ring buffer),
> but the code is in an inconsistent state w.r.t. its naming. I'll fix and
patch.

This is done. Please re-review.

[Peng, 2012-08-02 16:31:32]

lgtm, with comments.

BTW, I am not in OWNERS.

http://codereview.chromium.org/10827123/diff/19/chrome/browser/extensions/api...
File chrome/browser/extensions/api/serial/serial_apitest.cc (right):

http://codereview.chromium.org/10827123/diff/19/chrome/browser/extensions/api...
chrome/browser/extensions/api/serial/serial_apitest.cc:89: while
(bytes_to_copy--) {
Maybe test !ring_buffer_.empty() as well, and return the actual read size.

http://codereview.chromium.org/10827123/diff/19/chrome/browser/extensions/api...
chrome/browser/extensions/api/serial/serial_apitest.cc:112: std::deque<char>
ring_buffer_;
Maybe just buffer_ is better.

[commit-bot: I haz the power, 2012-08-02 19:18:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miket@chromium.org/10827123/5003

[commit-bot: I haz the power, 2012-08-02 19:18:37]

Failed to apply patch for chrome/common/extensions/docs/apps/experimental.html:
While running patch -p1 --forward --force;
patching file chrome/common/extensions/docs/apps/experimental.html
Hunk #1 FAILED at 213.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/common/extensions/docs/apps/experimental.html.rej

[commit-bot: I haz the power, 2012-08-02 19:23:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miket@chromium.org/10827123/5003

[commit-bot: I haz the power, 2012-08-02 19:23:14]

Failed to apply patch for chrome/common/extensions/docs/apps/experimental.html:
While running patch -p1 --forward --force;
patching file chrome/common/extensions/docs/apps/experimental.html
Hunk #1 FAILED at 213.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/common/extensions/docs/apps/experimental.html.rej

[asargent_no_longer_on_chrome, 2012-08-02 19:58:07]

LGTM

http://codereview.chromium.org/10827123/diff/5003/chrome/browser/extensions/a...
File chrome/browser/extensions/api/serial/serial_api.cc (right):

http://codereview.chromium.org/10827123/diff/5003/chrome/browser/extensions/a...
chrome/browser/extensions/api/serial/serial_api.cc:213: new
net::IOBufferWithSize(params_->bytes_to_read));
nit: does IOBufferWithSize already properly handle the case of being passed an
absurdly large number of bytes? If not we should impose some reasonable upper
limit (a megabyte maybe?)

[commit-bot: I haz the power, 2012-08-02 20:11:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miket@chromium.org/10827123/3019

[commit-bot: I haz the power, 2012-08-02 20:11:10]

Failed to apply patch for chrome/common/extensions/docs/apps/experimental.html:
While running patch -p1 --forward --force;
patching file chrome/common/extensions/docs/apps/experimental.html
Hunk #1 FAILED at 213.
1 out of 1 hunk FAILED -- saving rejects to file
chrome/common/extensions/docs/apps/experimental.html.rej

[commit-bot: I haz the power, 2012-08-02 20:38:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miket@chromium.org/10827123/12002

[commit-bot: I haz the power, 2012-08-02 22:06:46]

Change committed as 149718