Implemented SpdyHttpStream::GetTotalSentBytes.

Implemented SpdyHttpStream::GetTotalSentBytes.

Based on https://codereview.chromium.org/1314783007/.

This CL implements SpdyHttpStream::GetTotalSentBytes, including SPDY
frame overhead.

BUG=518897
Committed: https://crrev.com/c8db2ff24e80abbaa099af0f35fe402a702e2c25
Cr-Commit-Position: refs/heads/master@{#347038}

[sclittle, 2015-08-31 22:46:35]

sclittle@chromium.org changed reviewers:
+ bengr@chromium.org, bnc@chromium.org, mmenke@chromium.org

[sclittle, 2015-08-31 22:46:35]

bengr, bnc: *

mmenke added for net/ ownership.

Thanks in advance!

[bengr, 2015-09-01 16:22:21]

This is much more manageable that the monolithic CL you had yesterday. Thanks
for breaking it up!

Would you please document what is included and not included in the sent/received
counts?

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_http_stream_u...
File net/spdy/spdy_http_stream_unittest.cc (right):

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_http_stream_u...
net/spdy/spdy_http_stream_unittest.cc:285: http_stream1->GetTotalSentBytes());
This is the number of bytes after header compression, correct?

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_session.cc
File net/spdy/spdy_session.cc (right):

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_session.cc#ne...
net/spdy/spdy_session.cc:1622: in_flight_write_stream_->IncrementRawSentBytes(
Could you rename, because "Increment" means increase by a fixed size, not an
arbitrary one.

[mmenke, 2015-09-01 19:20:16]

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_stream.h
File net/spdy/spdy_stream.h (right):

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_stream.h#newc...
net/spdy/spdy_stream.h:430: void IncrementRawSentBytes(size_t sent_bytes) {
This should be named increment_raw_sent_bytes, or not inlined.  Same goes for
the above method (Think it's fine to do the rename in this CL)

[sclittle, 2015-09-01 23:22:41]

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_http_stream_u...
File net/spdy/spdy_http_stream_unittest.cc (right):

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_http_stream_u...
net/spdy/spdy_http_stream_unittest.cc:285: http_stream1->GetTotalSentBytes());
On 2015/09/01 16:22:21, bengr wrote:
> This is the number of bytes after header compression, correct?

Yes. This is the raw size of the frame that was written to the socket. These
tests don't actually enable header compression, since it makes it harder to test
the different SPDY/HTTP2 protocols, but we don't really need to here since that
would just send a frame with a different size payload, and all we really need to
test here is that the whole frame is accounted for in
GetTotalSent/ReceivedBytes.

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_session.cc
File net/spdy/spdy_session.cc (right):

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_session.cc#ne...
net/spdy/spdy_session.cc:1622: in_flight_write_stream_->IncrementRawSentBytes(
On 2015/09/01 16:22:21, bengr wrote:
> Could you rename, because "Increment" means increase by a fixed size, not an
> arbitrary one.

Done.

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_stream.h
File net/spdy/spdy_stream.h (right):

https://codereview.chromium.org/1309663003/diff/1/net/spdy/spdy_stream.h#newc...
net/spdy/spdy_stream.h:430: void IncrementRawSentBytes(size_t sent_bytes) {
On 2015/09/01 19:20:16, mmenke wrote:
> This should be named increment_raw_sent_bytes, or not inlined.  Same goes for
> the above method (Think it's fine to do the rename in this CL)

Done.

[mmenke, 2015-09-02 17:33:13]

This LGTM, modulo comments.

https://codereview.chromium.org/1309663003/diff/20001/net/spdy/spdy_http_stre...
File net/spdy/spdy_http_stream.h (right):

https://codereview.chromium.org/1309663003/diff/20001/net/spdy/spdy_http_stre...
net/spdy/spdy_http_stream.h:71: // push_promise frames for this stream,
including the size of frame headers.
Not quite accurate.  It's the number of bytes after SSL decryption and proxy
overhead that are associated with this stream.  Same goes for GetTotalSentBytes.

https://codereview.chromium.org/1309663003/diff/20001/net/spdy/spdy_http_stre...
net/spdy/spdy_http_stream.h:132: int64_t closed_stream_sent_bytes_;
Think these need a comment.  Maybe something like "Track the bytes sent/received
after |stream_| has been closed.".

[sclittle, 2015-09-02 18:59:56]

bnc: PTAL

https://codereview.chromium.org/1309663003/diff/20001/net/spdy/spdy_http_stre...
File net/spdy/spdy_http_stream.h (right):

https://codereview.chromium.org/1309663003/diff/20001/net/spdy/spdy_http_stre...
net/spdy/spdy_http_stream.h:71: // push_promise frames for this stream,
including the size of frame headers.
On 2015/09/02 17:33:13, mmenke wrote:
> Not quite accurate.  It's the number of bytes after SSL decryption and proxy
> overhead that are associated with this stream.  Same goes for
GetTotalSentBytes.

Done.

https://codereview.chromium.org/1309663003/diff/20001/net/spdy/spdy_http_stre...
net/spdy/spdy_http_stream.h:132: int64_t closed_stream_sent_bytes_;
On 2015/09/02 17:33:13, mmenke wrote:
> Think these need a comment.  Maybe something like "Track the bytes
sent/received
> after |stream_| has been closed.".

Done. Had to reword it so that it didn't imply that new bytes are received/sent
after |stream_| is already closed.

[Bence, 2015-09-02 19:44:20]

LGTM.

[sclittle, 2015-09-02 20:14:05]

The CQ bit was checked by sclittle@chromium.org

[sclittle, 2015-09-02 20:14:06]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/1309663003/#ps40001
(title: "Addressed comments")

[commit-bot: I haz the power, 2015-09-02 20:14:47]

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

[bengr, 2015-09-02 20:35:45]

lgtm

[commit-bot: I haz the power, 2015-09-02 22:02:44]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-09-02 22:03:49]

Patchset 3 (id:??) landed as
https://crrev.com/c8db2ff24e80abbaa099af0f35fe402a702e2c25
Cr-Commit-Position: refs/heads/master@{#347038}