Chromium Code Reviews
Help | Chromium Project | Sign in
(107)

Issue 11300020: Add QuicStream and friends to QUIC code. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by Ryan Hamilton
Modified:
1 year, 5 months ago
Reviewers:
jar
CC:
chromium-reviews_chromium.org, pam+watch_chromium.org, cbentzel+watch_chromium.org, darin-cc_chromium.org
Visibility:
Public.

Description

Add QuicStream and friends to QUIC code.


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

Patch Set 1 #

Patch Set 2 : Rebase #

Patch Set 3 : License #

Total comments: 71

Patch Set 4 : Rebase #

Patch Set 5 : Remove iovec code, which is not supported on windows #

Patch Set 6 : NET_EXPORT_PRIVATE #

Patch Set 7 : size_t -> int #

Patch Set 8 : smaller char constant #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1714 lines, -2 lines) Lint Patch
M net/net.gyp View 1 2 3 3 chunks +11 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_crypto_stream.h View 1 2 3 4 5 1 chunk +59 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_crypto_stream.cc View 1 2 1 chunk +46 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_crypto_stream_test.cc View 1 2 3 4 5 6 7 1 chunk +100 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_session.h View 1 2 3 4 5 6 1 chunk +117 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_session.cc View 1 2 3 4 5 6 1 chunk +195 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_session_test.cc View 1 2 1 chunk +158 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_stream_sequencer.h View 1 2 3 4 5 1 chunk +90 lines, -0 lines 0 comments 2 errors Download
A net/quic/quic_stream_sequencer.cc View 1 2 3 4 1 chunk +203 lines, -0 lines 0 comments ? errors Download
A net/quic/quic_stream_sequencer_test.cc View 1 2 3 4 1 chunk +413 lines, -0 lines 0 comments ? errors Download
A net/quic/reliable_quic_stream.h View 1 2 3 4 5 6 1 chunk +80 lines, -0 lines 0 comments 1 errors Download
A net/quic/reliable_quic_stream.cc View 1 2 3 4 5 6 1 chunk +126 lines, -0 lines 0 comments 1 errors Download
M net/quic/test_tools/quic_test_utils.h View 1 2 3 4 5 6 4 chunks +75 lines, -2 lines 0 comments ? errors Download
M net/quic/test_tools/quic_test_utils.cc View 1 2 3 chunks +41 lines, -0 lines 0 comments ? errors Download
Commit:

Messages

Total messages: 13
Ryan Hamilton
friendly ping
1 year, 5 months ago #1
jar
I haven't finished reviewing... and am only up to quic_stream_sequencer_test.cc, but since I had some ...
1 year, 5 months ago #2
alyssar
https://codereview.chromium.org/11300020/diff/5001/net/quic/quic_stream_sequencer.cc File net/quic/quic_stream_sequencer.cc (right): https://codereview.chromium.org/11300020/diff/5001/net/quic/quic_stream_sequencer.cc#newcode148 net/quic/quic_stream_sequencer.cc:148: const_cast<char*>(it->second.c_str())); +1 to data() just for clarity https://codereview.chromium.org/11300020/diff/5001/net/quic/quic_stream_sequencer.cc#newcode195 net/quic/quic_stream_sequencer.cc:195: ...
1 year, 5 months ago #3
jar
This is the second half of the comments on this patch set. As per discussion, ...
1 year, 5 months ago #4
Ryan Hamilton
All but a few of these comments will be addressed when internal change 37570979 lands. ...
1 year, 5 months ago #5
jar
https://codereview.chromium.org/11300020/diff/5001/net/quic/quic_stream_sequencer.cc File net/quic/quic_stream_sequencer.cc (right): https://codereview.chromium.org/11300020/diff/5001/net/quic/quic_stream_sequencer.cc#newcode45 net/quic/quic_stream_sequencer.cc:45: size_t data_len = frame.data.size(); I think my comment was ...
1 year, 5 months ago #6
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/11300020/10002
1 year, 5 months ago #7
I haz the power (commit-bot)
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
1 year, 5 months ago #8
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/11300020/4002
1 year, 5 months ago #9
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/11300020/4004
1 year, 5 months ago #10
I haz the power (commit-bot)
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
1 year, 5 months ago #11
I haz the power (commit-bot)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rch@chromium.org/11300020/3019
1 year, 5 months ago #12
I haz the power (commit-bot)
1 year, 5 months ago #13
Change committed as 165858
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6