Remove QuicSentPacketManager::DebugVisitor's OnSentPacket, since it's

Remove QuicSentPacketManager::DebugVisitor's OnSentPacket, since it's
redundant to QuicConnection::DebugVisitor's OnPacketSent.

Refactored QuicConnection::DebugVisitor's OnPacketSent in order to add
necessary fields for the trace... visitor.

Merge internal change: 76595983

R=rch@chromium.org

[ramant (doing other things), 2014-10-03 03:40:55]

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
File net/quic/quic_connection_logger.cc (right):

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
net/quic/quic_connection_logger.cc:60:
base::Uint64ToString(serialized_packet.sequence_number));
Hi Ryan and Ian,
  Should we log more info from serialized_packet to net-internals? Logged
sent_time's ToDebuggingValue(). Is that ok?

thanks
raman

[Ian Swett, 2014-10-03 17:50:30]

ianswett@google.com changed reviewers:
+ ianswett@google.com

[Ian Swett, 2014-10-03 17:50:30]

Hi Raman, did we recently remove any logging assocated with OnSerializedPacket
or OnPacketRetransmitted?  If so, we'd want to restore that information in the
new OnPacketSent method.

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
File net/quic/quic_connection_logger.cc (right):

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
net/quic/quic_connection_logger.cc:60:
base::Uint64ToString(serialized_packet.sequence_number));
On 2014/10/03 03:40:55, ramant wrote:
> Hi Ryan and Ian,
>   Should we log more info from serialized_packet to net-internals? Logged
> sent_time's ToDebuggingValue(). Is that ok?
> 
> thanks
> raman

I would think we'd want to log the original sequence number for retransmissions,
unless that's logged elsewhere? 

The time sounds like a good addition, though we do have it in the net-internals
itself as well.  Ryan, your call.

[ramant (doing other things), 2014-10-03 22:05:44]

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
File net/quic/quic_connection_logger.cc (right):

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
net/quic/quic_connection_logger.cc:60:
base::Uint64ToString(serialized_packet.sequence_number));
On 2014/10/03 17:50:30, Ian Swett wrote:
> On 2014/10/03 03:40:55, ramant wrote:
> > Hi Ryan and Ian,
> >   Should we log more info from serialized_packet to net-internals? Logged
> > sent_time's ToDebuggingValue(). Is that ok?
> > 
> > thanks
> > raman
> 
> I would think we'd want to log the original sequence number for
retransmissions,
> unless that's logged elsewhere? 

We do. It is logged in the next line (61-62). Is there another
original_sequence_number?

> 
> The time sounds like a good addition, though we do have it in the
net-internals
> itself as well.  Ryan, your call.

thanks Ian.

[ramant (doing other things), 2014-10-03 22:32:09]

On 2014/10/03 17:50:30, Ian Swett wrote:
> Hi Raman, did we recently remove any logging assocated with OnSerializedPacket
> or OnPacketRetransmitted? 

I don't remember deleting in logging code from quic_connection_logger.cc. Will
go through the merges and see if we had done it.

> If so, we'd want to restore that information in the  new OnPacketSent method.

Will check and will do in another CL.

thanks
raman

> 
>
https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
> File net/quic/quic_connection_logger.cc (right):
> 
>
https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
> net/quic/quic_connection_logger.cc:60:
> base::Uint64ToString(serialized_packet.sequence_number));
> On 2014/10/03 03:40:55, ramant wrote:
> > Hi Ryan and Ian,
> >   Should we log more info from serialized_packet to net-internals? Logged
> > sent_time's ToDebuggingValue(). Is that ok?
> > 
> > thanks
> > raman
> 
> I would think we'd want to log the original sequence number for
retransmissions,
> unless that's logged elsewhere? 
> 
> The time sounds like a good addition, though we do have it in the
net-internals
> itself as well.  Ryan, your call.

[Ian Swett, 2014-10-05 17:51:03]

The CQ bit was checked by ianswett@google.com

[Ian Swett, 2014-10-05 17:51:03]

lgtm

Thanks Raman, I'm not as familiar with the QuicConnectionLogger, but this looks
good.

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
File net/quic/quic_connection_logger.cc (right):

https://codereview.chromium.org/627583002/diff/1/net/quic/quic_connection_log...
net/quic/quic_connection_logger.cc:60:
base::Uint64ToString(serialized_packet.sequence_number));
On 2014/10/03 22:05:43, ramant wrote:
> On 2014/10/03 17:50:30, Ian Swett wrote:
> > On 2014/10/03 03:40:55, ramant wrote:
> > > Hi Ryan and Ian,
> > >   Should we log more info from serialized_packet to net-internals? Logged
> > > sent_time's ToDebuggingValue(). Is that ok?
> > > 
> > > thanks
> > > raman
> > 
> > I would think we'd want to log the original sequence number for
> retransmissions,
> > unless that's logged elsewhere? 
> 
> We do. It is logged in the next line (61-62). Is there another
> original_sequence_number?
> 
> > 
> > The time sounds like a good addition, though we do have it in the
> net-internals
> > itself as well.  Ryan, your call.
> 
> thanks Ian.

Nope, I just missed that.  Thanks!

[commit-bot: I haz the power, 2014-10-05 17:52:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/627583002/1

[commit-bot: I haz the power, 2014-10-05 17:52:15]

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

[commit-bot: I haz the power, 2014-10-05 17:52:16]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[Ryan Hamilton, 2014-10-10 22:48:58]

lgtm