Land Recent QUIC changes.

Land Recent QUIC changes.

Fix to change SendAlarm so it does not not try to write if
the connection is write blocked.  Chrome b/303981.

Merge internal change: 53845630


Fixing minor nits (added using std::pair).

Merge internal change: 53257699

Increasing the maximum tcp congestion window to 200 packets from 100.

Merge internal change: 53185276

Add a DCHECK to ensure the sent packet sequence number never goes down
and separate the send alarm into a send alarm and a resume writes
alarm to be used when the socket is still writable an there may be
more pending data to write.

Merge internal change: 53050471

Merged quic_epoll_connection_helper_test.cc from internal code.

Minor nit fixes.

Merge internal change: 53048224

Move QuicAckNotifierManager from QuicConnection to
QuicSentPacketManager.

Merge internal change: 53036185

R=jar@chromium.org, rch@chromium.org, wtc@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226390

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=227396

[wtc, 2013-10-01 14:06:59]

Patch set 1 LGTM.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
File net/quic/quic_ack_notifier_manager.h (right):

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:12: #include
"base/containers/hash_tables.h"

Do we still need these two headers? It seems that you tried to use
base/containers/hash_tables.h but failed.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:52: // until C++11.

Nit: this comment needs more context. I don't know what it is referring to.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:60: // Owns the AckNotifiers in this list.

Nit: should we change all occurrences of "list" in this comment to "set"?

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:67: std::map<QuicPacketSequenceNumber,
AckNotifierSet> ack_notifier_map_;

This is an AckNotifierMap:
  AckNotifierMap ack_notifier_map_;

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_connection.cc
File net/quic/quic_connection.cc (right):

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_connection.cc#n...
net/quic/quic_connection.cc:918: if (consumed_data.bytes_consumed <= 0) {

Nit: consumed_data.bytes_consumed is a size_t, which is unsigned, so it looks
nicer to test == 0.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_sent_packet_man...
File net/quic/quic_sent_packet_manager.cc (right):

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_sent_packet_man...
net/quic/quic_sent_packet_manager.cc:100: 

Nit: delete one blank line.

[ramant (doing other things), 2013-10-01 22:29:32]

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
File net/quic/quic_ack_notifier_manager.h (right):

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:12: #include
"base/containers/hash_tables.h"
On 2013/10/01 14:06:59, wtc wrote:
> 
> Do we still need these two headers? It seems that you tried to use
> base/containers/hash_tables.h but failed.

Fixed the problem with hash_tables.h not compiling.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:52: // until C++11.
On 2013/10/01 14:06:59, wtc wrote:
> 
> Nit: this comment needs more context. I don't know what it is referring to.

Deleted the comment.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:60: // Owns the AckNotifiers in this list.
On 2013/10/01 14:06:59, wtc wrote:
> 
> Nit: should we change all occurrences of "list" in this comment to "set"?

Done.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_ack_notifier_ma...
net/quic/quic_ack_notifier_manager.h:67: std::map<QuicPacketSequenceNumber,
AckNotifierSet> ack_notifier_map_;
On 2013/10/01 14:06:59, wtc wrote:
> 
> This is an AckNotifierMap:
>   AckNotifierMap ack_notifier_map_;

Done.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_connection.cc
File net/quic/quic_connection.cc (right):

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_connection.cc#n...
net/quic/quic_connection.cc:918: if (consumed_data.bytes_consumed <= 0) {
On 2013/10/01 14:06:59, wtc wrote:
> 
> Nit: consumed_data.bytes_consumed is a size_t, which is unsigned, so it looks
> nicer to test == 0.

Done.

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_sent_packet_man...
File net/quic/quic_sent_packet_manager.cc (right):

https://codereview.chromium.org/25443002/diff/1/net/quic/quic_sent_packet_man...
net/quic/quic_sent_packet_manager.cc:100: 
On 2013/10/01 14:06:59, wtc wrote:
> 
> Nit: delete one blank line.

Done.

[commit-bot: I haz the power, 2013-10-01 22:32:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/25443002/12001

[commit-bot: I haz the power, 2013-10-01 22:32:16]

Failed to apply patch for net/quic/quic_connection.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file net/quic/quic_connection.h
  Hunk #1 succeeded at 722 (offset 17 lines).
  Hunk #2 succeeded at 750 (offset 17 lines).
  Hunk #3 FAILED at 768.
  1 out of 3 hunks FAILED -- saving rejects to file
net/quic/quic_connection.h.rej

Patch:       net/quic/quic_connection.h
Index: net/quic/quic_connection.h
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index
634736db786a19b4dfbf33850fbcb60575ccdaa4..522d15b0f087a0d52e6b578e273ee35d0e962056
100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -705,6 +705,9 @@ class NET_EXPORT_PRIVATE QuicConnection
   // An alarm that is scheduled when the sent scheduler requires a
   // a delay before sending packets and fires when the packet may be sent.
   scoped_ptr<QuicAlarm> send_alarm_;
+  // An alarm that is scheduled when the connection can still write and there
+  // may be more data to send.
+  scoped_ptr<QuicAlarm> resume_writes_alarm_;
   // An alarm that fires when the connection may have timed out.
   scoped_ptr<QuicAlarm> timeout_alarm_;
 
@@ -730,6 +733,11 @@ class NET_EXPORT_PRIVATE QuicConnection
   // The time that we last sent a packet for this connection.
   QuicTime time_of_last_sent_packet_;
 
+  // Sequence number of the last packet guaranteed to be sent in packet
sequence
+  // number order.  Not set when packets are queued, since that may cause
+  // re-ordering.
+  QuicPacketSequenceNumber sequence_number_of_last_inorder_packet_;
+
   // Congestion manager which controls the rate the connection sends packets
   // as well as collecting and generating congestion feedback.
   QuicCongestionManager congestion_manager_;
@@ -760,11 +768,6 @@ class NET_EXPORT_PRIVATE QuicConnection
   // This is checked later on validating a data or version negotiation packet.
   bool address_migrating_;
 
-  // An AckNotifier can register to be informed when ACKs have been received
for
-  // all packets that a given block of data was sent in. The AckNotifierManager
-  // maintains the currently active notifiers.
-  AckNotifierManager ack_notifier_manager_;
-
   DISALLOW_COPY_AND_ASSIGN(QuicConnection);
 };

[commit-bot: I haz the power, 2013-10-01 22:37:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/25443002/11001

[commit-bot: I haz the power, 2013-10-01 22:37:54]

Failed to apply patch for net/quic/quic_connection.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file net/quic/quic_connection.h
  Hunk #1 succeeded at 722 (offset 17 lines).
  Hunk #2 succeeded at 750 (offset 17 lines).
  Hunk #3 FAILED at 768.
  1 out of 3 hunks FAILED -- saving rejects to file
net/quic/quic_connection.h.rej

Patch:       net/quic/quic_connection.h
Index: net/quic/quic_connection.h
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index
634736db786a19b4dfbf33850fbcb60575ccdaa4..522d15b0f087a0d52e6b578e273ee35d0e962056
100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -705,6 +705,9 @@ class NET_EXPORT_PRIVATE QuicConnection
   // An alarm that is scheduled when the sent scheduler requires a
   // a delay before sending packets and fires when the packet may be sent.
   scoped_ptr<QuicAlarm> send_alarm_;
+  // An alarm that is scheduled when the connection can still write and there
+  // may be more data to send.
+  scoped_ptr<QuicAlarm> resume_writes_alarm_;
   // An alarm that fires when the connection may have timed out.
   scoped_ptr<QuicAlarm> timeout_alarm_;
 
@@ -730,6 +733,11 @@ class NET_EXPORT_PRIVATE QuicConnection
   // The time that we last sent a packet for this connection.
   QuicTime time_of_last_sent_packet_;
 
+  // Sequence number of the last packet guaranteed to be sent in packet
sequence
+  // number order.  Not set when packets are queued, since that may cause
+  // re-ordering.
+  QuicPacketSequenceNumber sequence_number_of_last_inorder_packet_;
+
   // Congestion manager which controls the rate the connection sends packets
   // as well as collecting and generating congestion feedback.
   QuicCongestionManager congestion_manager_;
@@ -760,11 +768,6 @@ class NET_EXPORT_PRIVATE QuicConnection
   // This is checked later on validating a data or version negotiation packet.
   bool address_migrating_;
 
-  // An AckNotifier can register to be informed when ACKs have been received
for
-  // all packets that a given block of data was sent in. The AckNotifierManager
-  // maintains the currently active notifiers.
-  AckNotifierManager ack_notifier_manager_;
-
   DISALLOW_COPY_AND_ASSIGN(QuicConnection);
 };

[commit-bot: I haz the power, 2013-10-01 23:03:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/25443002/20001

[commit-bot: I haz the power, 2013-10-01 23:56:38]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-02 00:56:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/25443002/20001

[commit-bot: I haz the power, 2013-10-02 01:49:53]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-02 01:51:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/25443002/20001

[commit-bot: I haz the power, 2013-10-02 03:33:13]

Change committed as 226390

[ramant (doing other things), 2013-10-07 20:00:43]

Hi Ryan,
  Tested Ian's this morning fix on windows (internal CL 53845630) and chrome
didn't crash.

  Updated the description to have his CL. Would like to re-land this CL. Could
you please take a look? (will merge with trunk or tip of the tree before trying
CQ).

thanks
raman

[Ryan Hamilton, 2013-10-07 21:27:20]

lgtm

[commit-bot: I haz the power, 2013-10-07 21:29:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rtenneti@chromium.org/25443002/47001

[commit-bot: I haz the power, 2013-10-08 00:10:20]

Change committed as 227396