Makes the BattOrConnection read messages instead of bytes

Makes the BattOrConnection read messages instead of bytes

This was quickly going to become a problem as we implemented
StopTracing. When the BattOr finishes tracing and starts streaming back
the samples that it recorded, it sends back frames with varying message
lengths.

With this change, we can say "give me the next message up to 50kB"
rather than having to specify an exact number of bytes that we expect.
It also gets rid of escape byte counting, which was awkward and
error-prone anyway.

BUG=542837
Committed: https://crrev.com/19bf12bc8a89b38e1bcdd2a767e722209903d738
Cr-Commit-Position: refs/heads/master@{#368091}

[charliea (OOO until 10-5), 2016-01-06 18:58:38]

Description was changed from

==========
Makes the BattOrConnection read messages instead of bytes

This was quickly going to become a problem as we implemented
StopTracing. When the BattOr finishes tracing and starts streaming back
the samples that it recorded, it sends back frames with a varying
number of bytes between the start and end bytes.

With this change, we can say "give me the next message up to 50kB"
rather than having to specify an exact number of bytes that we expect.
It also gets rid of escape byte counting, which was awkward and
error-prone anyway.

BUG=542837
==========

to

==========
Makes the BattOrConnection read messages instead of bytes

This was quickly going to become a problem as we implemented
StopTracing. When the BattOr finishes tracing and starts streaming back
the samples that it recorded, it sends back frames with varying message
lengths.

With this change, we can say "give me the next message up to 50kB"
rather than having to specify an exact number of bytes that we expect.
It also gets rid of escape byte counting, which was awkward and
error-prone anyway.

BUG=542837
==========

[charliea (OOO until 10-5), 2016-01-06 18:58:56]

The CQ bit was checked by charliea@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-01-06 18:59:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1567683002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1567683002/1

[charliea (OOO until 10-5), 2016-01-06 18:59:43]

Patchset #1 (id:1) has been deleted

[charliea (OOO until 10-5), 2016-01-06 19:03:26]

Patchset #1 (id:20001) has been deleted

[charliea (OOO until 10-5), 2016-01-06 19:03:32]

The CQ bit was checked by charliea@chromium.org to run a CQ dry run

[charliea (OOO until 10-5), 2016-01-06 19:03:39]

charliea@chromium.org changed reviewers:
+ nednguyen@google.com

[commit-bot: I haz the power, 2016-01-06 19:03:58]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1567683002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1567683002/40001

[commit-bot: I haz the power, 2016-01-06 19:54:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-01-06 19:54:53]

Dry run: This issue passed the CQ dry run.

[nednguyen, 2016-01-06 22:03:13]

lgtm with nits

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
File tools/battor_agent/battor_agent.cc (right):

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
tools/battor_agent/battor_agent.cc:185:
connection_->ReadMessage(sizeof(BattOrControlMessageAck));
Can yo abstract this to ReadMessage(message_type), and ReadMessage knows how to
compute the message size depending on the message type?
This way, call site do not have to worry about the detail of 
max_data_bytes_to_read not including any start, end, or escape bytes.

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
File tools/battor_agent/battor_connection_impl.cc (right):

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
tools/battor_agent/battor_connection_impl.cc:204: if (i >= kMaxMessageSize)  // 
|| i >= pending_read_max_length_)  // )
What is the comment on the right for?

[charliea (OOO until 10-5), 2016-01-07 16:00:12]

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
File tools/battor_agent/battor_agent.cc (right):

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
tools/battor_agent/battor_agent.cc:185:
connection_->ReadMessage(sizeof(BattOrControlMessageAck));
On 2016/01/06 22:03:13, nednguyen wrote:
> Can yo abstract this to ReadMessage(message_type), and ReadMessage knows how
to
> compute the message size depending on the message type?
> This way, call site do not have to worry about the detail of 
> max_data_bytes_to_read not including any start, end, or escape bytes.

I like this idea a lot. Done.

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
File tools/battor_agent/battor_connection_impl.cc (right):

https://codereview.chromium.org/1567683002/diff/40001/tools/battor_agent/batt...
tools/battor_agent/battor_connection_impl.cc:204: if (i >= kMaxMessageSize)  // 
|| i >= pending_read_max_length_)  // )
On 2016/01/06 22:03:13, nednguyen wrote:
> What is the comment on the right for?

Doh, sorry. This is just crud left over from coding that I didn't catch when I
prereviewed.

[charliea (OOO until 10-5), 2016-01-07 16:03:35]

Patchset #2 (id:60001) has been deleted

[charliea (OOO until 10-5), 2016-01-07 16:03:46]

Patchset #2 (id:80001) has been deleted

[charliea (OOO until 10-5), 2016-01-07 16:03:59]

The CQ bit was checked by charliea@chromium.org

[charliea (OOO until 10-5), 2016-01-07 16:03:59]

The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/1567683002/#ps100001
(title: "Code review")

[commit-bot: I haz the power, 2016-01-07 16:04:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1567683002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1567683002/100001

[commit-bot: I haz the power, 2016-01-07 16:40:23]

Committed patchset #2 (id:100001)

[commit-bot: I haz the power, 2016-01-07 16:41:29]

Description was changed from

==========
Makes the BattOrConnection read messages instead of bytes

This was quickly going to become a problem as we implemented
StopTracing. When the BattOr finishes tracing and starts streaming back
the samples that it recorded, it sends back frames with varying message
lengths.

With this change, we can say "give me the next message up to 50kB"
rather than having to specify an exact number of bytes that we expect.
It also gets rid of escape byte counting, which was awkward and
error-prone anyway.

BUG=542837
==========

to

==========
Makes the BattOrConnection read messages instead of bytes

This was quickly going to become a problem as we implemented
StopTracing. When the BattOr finishes tracing and starts streaming back
the samples that it recorded, it sends back frames with varying message
lengths.

With this change, we can say "give me the next message up to 50kB"
rather than having to specify an exact number of bytes that we expect.
It also gets rid of escape byte counting, which was awkward and
error-prone anyway.

BUG=542837

Committed: https://crrev.com/19bf12bc8a89b38e1bcdd2a767e722209903d738
Cr-Commit-Position: refs/heads/master@{#368091}
==========

[commit-bot: I haz the power, 2016-01-07 16:41:30]

Patchset 2 (id:??) landed as
https://crrev.com/19bf12bc8a89b38e1bcdd2a767e722209903d738
Cr-Commit-Position: refs/heads/master@{#368091}