Makes QuicChromiumPacketWriter use the delegate for interacting with the connection.

Makes QuicChromiumPacketWriter use the delegate for interacting with the connection.

R=rch@chromium.org
CC=jokulik@chromium.org,fayang@chromium.org
BUG=

Committed: https://crrev.com/bce3eecc933d327f990916aed9c593c838c3ec5d
Cr-Commit-Position: refs/heads/master@{#417510}

[Ryan Hamilton, 2016-09-09 02:56:24]

LGTM modulo a few nits.

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
File net/quic/chromium/quic_chromium_packet_writer.cc (right):

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.cc:94: DCHECK(delegate_) <<
"Uninitialized observer.";
nit: here and elsewhere s/observer/delegate/

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.cc:109:
delegate_->OnWriteUnblocked();
this is actually a functional change, right? probably fine I guess since all the
tests pass?

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
File net/quic/chromium/quic_chromium_packet_writer.h (right):

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.h:42: explicit
QuicChromiumPacketWriter(Socket* socket);
I assume socket must outlive the writer? If so, please comment.

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.h:45: void set_delegate(Delegate*
delegate) { delegate_ = delegate; }
nit: Can you comment "// Must outlive the writer."

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.h:68: Delegate* delegate_;
 // Unowned. 
(And above, assuming that's true)

[Ryan Hamilton, 2016-09-09 02:56:42]

We probably should have some unit tests for the writer, but we don't need to do
that now.

[Jana, 2016-09-09 04:19:34]

Thanks much, Ryan.

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
File net/quic/chromium/quic_chromium_packet_writer.cc (right):

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.cc:94: DCHECK(delegate_) <<
"Uninitialized observer.";
On 2016/09/09 02:56:23, Ryan Hamilton wrote:
> nit: here and elsewhere s/observer/delegate/

Done.

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.cc:109:
delegate_->OnWriteUnblocked();
On 2016/09/09 02:56:23, Ryan Hamilton wrote:
> this is actually a functional change, right? probably fine I guess since all
the
> tests pass?

Yes, this is a functional change, but it's a relatively benign one... and tests
pass.

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
File net/quic/chromium/quic_chromium_packet_writer.h (right):

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.h:42: explicit
QuicChromiumPacketWriter(Socket* socket);
On 2016/09/09 02:56:23, Ryan Hamilton wrote:
> I assume socket must outlive the writer? If so, please comment.

Yes. The socket that's passed in is owned by QuicChromiumClientSession and dies
with the session.

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.h:45: void set_delegate(Delegate*
delegate) { delegate_ = delegate; }
On 2016/09/09 02:56:23, Ryan Hamilton wrote:
> nit: Can you comment "// Must outlive the writer."

Done.

https://codereview.chromium.org/2325733004/diff/1/net/quic/chromium/quic_chro...
net/quic/chromium/quic_chromium_packet_writer.h:68: Delegate* delegate_;
On 2016/09/09 02:56:23, Ryan Hamilton wrote:
>  // Unowned. 
> (And above, assuming that's true)

Done.

[Jana, 2016-09-09 04:25:23]

The CQ bit was checked by jri@chromium.org

[Jana, 2016-09-09 04:25:24]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2325733004/#ps20001
(title: "Nits fixed.")

[commit-bot: I haz the power, 2016-09-09 04:25:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 05:42:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-09 05:44:51]

Description was changed from

==========
Makes QuicChromiumPacketWriter use the delegate for interacting with the
connection.

R=rch@chromium.org
CC=jokulik@chromium.org,fayang@chromium.org
BUG=
==========

to

==========
Makes QuicChromiumPacketWriter use the delegate for interacting with the
connection.

R=rch@chromium.org
CC=jokulik@chromium.org,fayang@chromium.org
BUG=

Committed: https://crrev.com/bce3eecc933d327f990916aed9c593c838c3ec5d
Cr-Commit-Position: refs/heads/master@{#417510}
==========

[commit-bot: I haz the power, 2016-09-09 05:44:52]

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