From a4dcff92312b99670e9a19657f881b00a0404e69 Mon Sep 17 00:00:00 2001 From: rtenneti Date: Mon, 29 Sep 2014 11:16:08 -0700 Subject: Land Recent QUIC Changes. Remove loss detection from QuicConfig now that connection options has replaced it. Merge internal change: 75952172 Chromium specific changes: + Deleted enable_quic_time_based_loss_detection from NetworkSession params. + Deleted enable-quic-time-based-loss-detection and disable-quic-time-based-loss-detection command line switches. https://codereview.chromium.org/591323003/ Do not timeout QUIC connections when settings the timeouts from InitializeSession(). Protected by FLAG_quic_timeouts_only_from_alarms Removes FLAG_quic_timeouts_require_activity which was somewhat broken. Merge internal change: 75927669 https://codereview.chromium.org/605903002/ Factor out the QUIC timeout alarm setting logic from the CheckForTimeout method into a new SetTimeout method. - no behavior change, simply moving QUIC timeout alarm code. Merge internal change: 75915264 https://codereview.chromium.org/593193005/ Add a timestamp field to QUIC's CachedNetworkParams proto message. Context in b/17357338, follow-up CL will store CachedNetworkParams and copy into newly created STKs. Merge internal change: 75897792 https://codereview.chromium.org/604173002/ Call QuicSentPacketManager's OnPacketSent method and make OnRetransmittedPacket and OnSerializedPacket private. Merge internal change: 75830237 https://codereview.chromium.org/593193004/ Change the return type of QuicConnection::CheckForTimeout from bool to void since it is unused. Merge internal change: 75724127 https://codereview.chromium.org/604163002/ Test-only. Remove calls to OnSerializedPacket from QuicSentPacketManagerTest, in preparation for OnSerializedPacket to be removed. Merge internal change: 75716236 https://codereview.chromium.org/600823006/ R=rch@chromium.org, sky@chromium.org Added sky@ for OWNERS approval for chrome/browser and chrome/common changes Review URL: https://codereview.chromium.org/605733006 Cr-Commit-Position: refs/heads/master@{#297208} --- chrome/browser/io_thread.cc | 27 ----- chrome/browser/io_thread.h | 8 -- chrome/browser/io_thread_unittest.cc | 30 ------ chrome/common/chrome_switches.cc | 10 -- chrome/common/chrome_switches.h | 2 - net/http/http_network_session.cc | 4 - net/http/http_network_session.h | 1 - net/quic/crypto/crypto_handshake_message.cc | 1 - net/quic/crypto/crypto_protocol.h | 2 - net/quic/crypto/source_address_token.h | 5 + net/quic/quic_config.cc | 18 ---- net/quic/quic_config.h | 8 -- net/quic/quic_config_test.cc | 2 - net/quic/quic_connection.cc | 93 ++++++++-------- net/quic/quic_connection.h | 9 +- net/quic/quic_connection_test.cc | 10 -- net/quic/quic_flags.cc | 8 +- net/quic/quic_flags.h | 2 +- net/quic/quic_sent_packet_manager.cc | 20 ++-- net/quic/quic_sent_packet_manager.h | 23 ++-- net/quic/quic_sent_packet_manager_test.cc | 158 +++++----------------------- net/quic/quic_server_session.cc | 2 + net/quic/quic_stream_factory.cc | 9 +- net/quic/quic_stream_factory.h | 1 - net/quic/quic_stream_factory_test.cc | 1 - net/quic/test_tools/quic_config_peer.cc | 6 -- net/quic/test_tools/quic_config_peer.h | 3 - net/quic/test_tools/quic_test_utils.cc | 4 +- net/tools/quic/quic_server_session.cc | 2 + net/tools/quic/quic_server_session_test.cc | 2 + 30 files changed, 123 insertions(+), 348 deletions(-) diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc index 5ce6ca7..d42723e 100644 --- a/chrome/browser/io_thread.cc +++ b/chrome/browser/io_thread.cc @@ -122,8 +122,6 @@ const char kQuicFieldTrialEnabledGroupName[] = "Enabled"; const char kQuicFieldTrialHttpsEnabledGroupName[] = "HttpsEnabled"; const char kQuicFieldTrialPacketLengthSuffix[] = "BytePackets"; const char kQuicFieldTrialPacingSuffix[] = "WithPacing"; -const char kQuicFieldTrialTimeBasedLossDetectionSuffix[] = - "WithTimeBasedLossDetection"; // The SPDY trial composes two different trial plus control groups: // * A "holdback" group with SPDY disabled, and corresponding control @@ -1015,8 +1013,6 @@ void IOThread::InitializeNetworkSessionParamsFromGlobals( ¶ms->enable_websocket_over_spdy); globals.enable_quic.CopyToIfSet(¶ms->enable_quic); - globals.enable_quic_time_based_loss_detection.CopyToIfSet( - ¶ms->enable_quic_time_based_loss_detection); globals.quic_always_require_handshake_confirmation.CopyToIfSet( ¶ms->quic_always_require_handshake_confirmation); globals.quic_disable_connection_pooling.CopyToIfSet( @@ -1169,9 +1165,6 @@ void IOThread::ConfigureQuicGlobals( bool enable_quic = ShouldEnableQuic(command_line, quic_trial_group); globals->enable_quic.set(enable_quic); if (enable_quic) { - globals->enable_quic_time_based_loss_detection.set( - ShouldEnableQuicTimeBasedLossDetection(command_line, quic_trial_group, - quic_trial_params)); globals->quic_always_require_handshake_confirmation.set( ShouldQuicAlwaysRequireHandshakeConfirmation(quic_trial_params)); globals->quic_disable_connection_pooling.set( @@ -1334,26 +1327,6 @@ double IOThread::GetAlternateProtocolProbabilityThreshold( } // static -bool IOThread::ShouldEnableQuicTimeBasedLossDetection( - const CommandLine& command_line, - base::StringPiece quic_trial_group, - const VariationParameters& quic_trial_params) { - if (command_line.HasSwitch(switches::kEnableQuicTimeBasedLossDetection)) - return true; - - if (command_line.HasSwitch(switches::kDisableQuicTimeBasedLossDetection)) - return false; - - if (LowerCaseEqualsASCII( - GetVariationParam(quic_trial_params, "enable_time_based_loss_detection"), - "true")) - return true; - - return quic_trial_group.ends_with( - kQuicFieldTrialTimeBasedLossDetectionSuffix); -} - -// static bool IOThread::ShouldQuicAlwaysRequireHandshakeConfirmation( const VariationParameters& quic_trial_params) { return LowerCaseEqualsASCII( diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h index 801d48d..c4f9a8e 100644 --- a/chrome/browser/io_thread.h +++ b/chrome/browser/io_thread.h @@ -185,7 +185,6 @@ class IOThread : public content::BrowserThreadDelegate { Optional enable_websocket_over_spdy; Optional enable_quic; - Optional enable_quic_time_based_loss_detection; Optional enable_quic_port_selection; Optional quic_always_require_handshake_confirmation; Optional quic_disable_connection_pooling; @@ -353,13 +352,6 @@ class IOThread : public content::BrowserThreadDelegate { base::StringPiece quic_trial_group, const VariationParameters& quic_trial_params); - // Returns true if QUIC time-base loss detection should be negotiated during - // the QUIC handshake. - static bool ShouldEnableQuicTimeBasedLossDetection( - const base::CommandLine& command_line, - base::StringPiece quic_trial_group, - const VariationParameters& quic_trial_params); - // Returns true if QUIC should always require handshake confirmation during // the QUIC handshake. static bool ShouldQuicAlwaysRequireHandshakeConfirmation( diff --git a/chrome/browser/io_thread_unittest.cc b/chrome/browser/io_thread_unittest.cc index 1563e9e..6120e3d 100644 --- a/chrome/browser/io_thread_unittest.cc +++ b/chrome/browser/io_thread_unittest.cc @@ -127,7 +127,6 @@ TEST_F(IOThreadTest, EnableQuicFromFieldTrialGroup) { net::HttpNetworkSession::Params params; InitializeNetworkSessionParams(¶ms); EXPECT_TRUE(params.enable_quic); - EXPECT_FALSE(params.enable_quic_time_based_loss_detection); EXPECT_EQ(1350u, params.quic_max_packet_length); EXPECT_EQ(1.0, params.alternate_protocol_probability_threshold); EXPECT_EQ(default_params.quic_supported_versions, @@ -181,35 +180,6 @@ TEST_F(IOThreadTest, EnablePacingFromFieldTrialParams) { EXPECT_EQ(options, params.quic_connection_options); } -TEST_F(IOThreadTest, EnableTimeBasedLossDetectionFromCommandLine) { - command_line_.AppendSwitch("enable-quic"); - command_line_.AppendSwitch("enable-quic-time-based-loss-detection"); - - ConfigureQuicGlobals(); - net::HttpNetworkSession::Params params; - InitializeNetworkSessionParams(¶ms); - EXPECT_TRUE(params.enable_quic_time_based_loss_detection); -} - -TEST_F(IOThreadTest, EnableTimeBasedLossDetectionFromFieldTrialGroup) { - field_trial_group_ = "EnabledWithTimeBasedLossDetection"; - - ConfigureQuicGlobals(); - net::HttpNetworkSession::Params params; - InitializeNetworkSessionParams(¶ms); - EXPECT_TRUE(params.enable_quic_time_based_loss_detection); -} - -TEST_F(IOThreadTest, EnableTimeBasedLossDetectionFromFieldTrialParams) { - field_trial_group_ = "Enabled"; - field_trial_params_["enable_time_based_loss_detection"] = "true"; - - ConfigureQuicGlobals(); - net::HttpNetworkSession::Params params; - InitializeNetworkSessionParams(¶ms); - EXPECT_TRUE(params.enable_quic_time_based_loss_detection); -} - TEST_F(IOThreadTest, PacketLengthFromCommandLine) { command_line_.AppendSwitch("enable-quic"); command_line_.AppendSwitchASCII("quic-max-packet-length", "1350"); diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 86ceaa3..80ff7cb 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -334,11 +334,6 @@ const char kDisableQuicPacing[] = "disable-quic-pacing"; // This only has an effect if QUIC protocol is enabled. const char kDisableQuicPortSelection[] = "disable-quic-port-selection"; -// Disable use of time-base loss detection for QUIC connections. -// This only has an effect if QUIC protocol is enabled. -const char kDisableQuicTimeBasedLossDetection[] = - "disable-quic-time-based-loss-detection"; - // Prevents the save password bubble from being enabled. const char kDisableSavePasswordBubble[] = "disable-save-password-bubble"; @@ -544,11 +539,6 @@ const char kEnableQuicPacing[] = "enable-quic-pacing"; // This only has an effect if QUIC protocol is enabled. const char kEnableQuicPortSelection[] = "enable-quic-port-selection"; -// Enables use of time-base loss detection for QUIC connections. -// This only has an effect if QUIC protocol is enabled. -const char kEnableQuicTimeBasedLossDetection[] = - "enable-quic-time-based-loss-detection"; - // Enables context-sensitive reader mode button in the toolbar. const char kEnableReaderModeToolbarIcon[] = "enable-reader-mode-toolbar-icon"; diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index dcfc0cc..1769ffb 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -99,7 +99,6 @@ extern const char kDisablePromptOnRepost[]; extern const char kDisableQuic[]; extern const char kDisableQuicPacing[]; extern const char kDisableQuicPortSelection[]; -extern const char kDisableQuicTimeBasedLossDetection[]; extern const char kDisableSavePasswordBubble[]; extern const char kDisableSearchButtonInOmnibox[]; extern const char kDisableSessionCrashedBubble[]; @@ -157,7 +156,6 @@ extern const char kEnableQueryExtraction[]; extern const char kEnableQuic[]; extern const char kEnableQuicPacing[]; extern const char kEnableQuicPortSelection[]; -extern const char kEnableQuicTimeBasedLossDetection[]; extern const char kEnableReaderModeToolbarIcon[]; extern const char kEnableResourceContentSettings[]; extern const char kEnableSavePasswordBubble[]; diff --git a/net/http/http_network_session.cc b/net/http/http_network_session.cc index 6dd06ea..1571ecc 100644 --- a/net/http/http_network_session.cc +++ b/net/http/http_network_session.cc @@ -91,7 +91,6 @@ HttpNetworkSession::Params::Params() enable_websocket_over_spdy(false), enable_quic(false), enable_quic_port_selection(true), - enable_quic_time_based_loss_detection(false), quic_always_require_handshake_confirmation(false), quic_disable_connection_pooling(false), quic_clock(NULL), @@ -134,7 +133,6 @@ HttpNetworkSession::HttpNetworkSession(const Params& params) params.quic_user_agent_id, params.quic_supported_versions, params.enable_quic_port_selection, - params.enable_quic_time_based_loss_detection, params.quic_always_require_handshake_confirmation, params.quic_disable_connection_pooling, params.quic_connection_options), @@ -264,8 +262,6 @@ base::Value* HttpNetworkSession::QuicInfoToValue() const { connection_options->AppendString("'" + QuicUtils::TagToString(*it) + "'"); } dict->Set("connection_options", connection_options); - dict->SetBoolean("enable_quic_time_based_loss_detection", - params_.enable_quic_time_based_loss_detection); dict->SetString("origin_to_force_quic_on", params_.origin_to_force_quic_on.ToString()); dict->SetDouble("alternate_protocol_probability_threshold", diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h index 13d7258..51e2292 100644 --- a/net/http/http_network_session.h +++ b/net/http/http_network_session.h @@ -112,7 +112,6 @@ class NET_EXPORT HttpNetworkSession bool enable_quic; bool enable_quic_port_selection; - bool enable_quic_time_based_loss_detection; bool quic_always_require_handshake_confirmation; bool quic_disable_connection_pooling; HostPortPair origin_to_force_quic_on; diff --git a/net/quic/crypto/crypto_handshake_message.cc b/net/quic/crypto/crypto_handshake_message.cc index 77f7c52..7eafba3 100644 --- a/net/quic/crypto/crypto_handshake_message.cc +++ b/net/quic/crypto/crypto_handshake_message.cc @@ -259,7 +259,6 @@ string CryptoHandshakeMessage::DebugStringInternal(size_t indent) const { case kAEAD: case kCGST: case kCOPT: - case kLOSS: case kPDMD: case kVER: // tag lists diff --git a/net/quic/crypto/crypto_protocol.h b/net/quic/crypto/crypto_protocol.h index f791acb..9f04961 100644 --- a/net/quic/crypto/crypto_protocol.h +++ b/net/quic/crypto/crypto_protocol.h @@ -81,8 +81,6 @@ const QuicTag kAEAD = TAG('A', 'E', 'A', 'D'); // Authenticated const QuicTag kCGST = TAG('C', 'G', 'S', 'T'); // Congestion control // feedback types const QuicTag kCOPT = TAG('C', 'O', 'P', 'T'); // Connection options -// kLOSS was 'L', 'O', 'S', 'S', but was changed from a tag vector to a tag. -const QuicTag kLOSS = TAG('L', 'O', 'S', 'A'); // Loss detection algorithms const QuicTag kICSL = TAG('I', 'C', 'S', 'L'); // Idle connection state // lifetime const QuicTag kKATO = TAG('K', 'A', 'T', 'O'); // Keepalive timeout diff --git a/net/quic/crypto/source_address_token.h b/net/quic/crypto/source_address_token.h index 64a3a12..1101351 100644 --- a/net/quic/crypto/source_address_token.h +++ b/net/quic/crypto/source_address_token.h @@ -74,6 +74,9 @@ class NET_EXPORT_PRIVATE CachedNetworkParameters { previous_connection_state_ = previous_connection_state; } + int64 timestamp() const { return timestamp_; } + void set_timestamp(int64 timestamp) { timestamp_ = timestamp; } + private: // serving_region_ is used to decide whether or not the bandwidth estimate and // min RTT are reasonable and if they should be used. @@ -94,6 +97,8 @@ class NET_EXPORT_PRIVATE CachedNetworkParameters { int32 min_rtt_ms_; // Encodes the PreviousConnectionState enum. int32 previous_connection_state_; + // UNIX timestamp when this bandwidth estimate was created. + int64 timestamp_; }; // TODO(rtenneti): sync with server more rationally. diff --git a/net/quic/quic_config.cc b/net/quic/quic_config.cc index 8d0a02c..f38e28b 100644 --- a/net/quic/quic_config.cc +++ b/net/quic/quic_config.cc @@ -427,7 +427,6 @@ QuicErrorCode QuicFixedTagVector::ProcessPeerHello( QuicConfig::QuicConfig() : congestion_feedback_(kCGST, PRESENCE_REQUIRED), connection_options_(kCOPT, PRESENCE_OPTIONAL), - loss_detection_(kLOSS, PRESENCE_OPTIONAL), idle_connection_state_lifetime_seconds_(kICSL, PRESENCE_REQUIRED), keepalive_timeout_seconds_(kKATO, PRESENCE_OPTIONAL), max_streams_per_connection_(kMSPC, PRESENCE_REQUIRED), @@ -479,18 +478,6 @@ QuicTagVector QuicConfig::SendConnectionOptions() const { return connection_options_.GetSendValues(); } -void QuicConfig::SetLossDetectionToSend(QuicTag loss_detection) { - loss_detection_.SetSendValue(loss_detection); -} - -bool QuicConfig::HasReceivedLossDetection() const { - return loss_detection_.HasReceivedValue(); -} - -QuicTag QuicConfig::ReceivedLossDetection() const { - return loss_detection_.GetReceivedValue(); -} - void QuicConfig::set_idle_connection_state_lifetime( QuicTime::Delta max_idle_connection_state_lifetime, QuicTime::Delta default_idle_conection_state_lifetime) { @@ -676,7 +663,6 @@ void QuicConfig::ToHandshakeMessage(CryptoHandshakeMessage* out) const { max_streams_per_connection_.ToHandshakeMessage(out); initial_congestion_window_.ToHandshakeMessage(out); initial_round_trip_time_us_.ToHandshakeMessage(out); - loss_detection_.ToHandshakeMessage(out); initial_flow_control_window_bytes_.ToHandshakeMessage(out); initial_stream_flow_control_window_bytes_.ToHandshakeMessage(out); initial_session_flow_control_window_bytes_.ToHandshakeMessage(out); @@ -732,10 +718,6 @@ QuicErrorCode QuicConfig::ProcessPeerHello( peer_hello, hello_type, error_details); } if (error == QUIC_NO_ERROR) { - error = loss_detection_.ProcessPeerHello( - peer_hello, hello_type, error_details); - } - if (error == QUIC_NO_ERROR) { error = connection_options_.ProcessPeerHello( peer_hello, hello_type, error_details); } diff --git a/net/quic/quic_config.h b/net/quic/quic_config.h index 688d281..75e46f6 100644 --- a/net/quic/quic_config.h +++ b/net/quic/quic_config.h @@ -273,12 +273,6 @@ class NET_EXPORT_PRIVATE QuicConfig { QuicTagVector SendConnectionOptions() const; - void SetLossDetectionToSend(QuicTag loss_detection); - - bool HasReceivedLossDetection() const; - - QuicTag ReceivedLossDetection() const; - void set_idle_connection_state_lifetime( QuicTime::Delta max_idle_connection_state_lifetime, QuicTime::Delta default_idle_conection_state_lifetime); @@ -375,8 +369,6 @@ class NET_EXPORT_PRIVATE QuicConfig { QuicNegotiableTag congestion_feedback_; // Connection options. QuicFixedTagVector connection_options_; - // Loss detection feedback type. - QuicFixedTag loss_detection_; // Idle connection state lifetime QuicNegotiableUint32 idle_connection_state_lifetime_seconds_; // Keepalive timeout, or 0 to turn off keepalive probes diff --git a/net/quic/quic_config_test.cc b/net/quic/quic_config_test.cc index 3a43d36..66a47d6 100644 --- a/net/quic/quic_config_test.cc +++ b/net/quic/quic_config_test.cc @@ -114,7 +114,6 @@ TEST_F(QuicConfigTest, ProcessClientHello) { EXPECT_EQ(QuicTime::Delta::FromSeconds(0), config_.keepalive_timeout()); EXPECT_EQ(10 * base::Time::kMicrosecondsPerMillisecond, config_.ReceivedInitialRoundTripTimeUs()); - EXPECT_FALSE(config_.HasReceivedLossDetection()); EXPECT_TRUE(config_.HasReceivedConnectionOptions()); EXPECT_EQ(2u, config_.ReceivedConnectionOptions().size()); EXPECT_EQ(config_.ReceivedConnectionOptions()[0], kTBBR); @@ -167,7 +166,6 @@ TEST_F(QuicConfigTest, ProcessServerHello) { EXPECT_EQ(QuicTime::Delta::FromSeconds(0), config_.keepalive_timeout()); EXPECT_EQ(10 * base::Time::kMicrosecondsPerMillisecond, config_.ReceivedInitialRoundTripTimeUs()); - EXPECT_FALSE(config_.HasReceivedLossDetection()); EXPECT_EQ(config_.ReceivedInitialFlowControlWindowBytes(), 2 * kInitialSessionFlowControlWindowForTest); EXPECT_EQ(config_.ReceivedInitialStreamFlowControlWindowBytes(), diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 093f9bb..46e6ea1 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -219,12 +219,8 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, idle_network_timeout_( QuicTime::Delta::FromSeconds(kDefaultInitialTimeoutSecs)), overall_connection_timeout_(QuicTime::Delta::Infinite()), - time_of_last_received_packet_( - FLAGS_quic_timeouts_require_activity - ? QuicTime::Zero() : clock_->ApproximateNow()), - time_of_last_sent_new_packet_( - FLAGS_quic_timeouts_require_activity - ? QuicTime::Zero() : clock_->ApproximateNow()), + time_of_last_received_packet_(clock_->ApproximateNow()), + time_of_last_sent_new_packet_(clock_->ApproximateNow()), sequence_number_of_last_sent_packet_(0), sent_packet_manager_( is_server, clock_, &stats_, @@ -1442,24 +1438,17 @@ bool QuicConnection::WritePacketInner(QueuedPacket* packet) { sent_packet_manager_.least_packet_awaited_by_peer(), sent_packet_manager_.GetCongestionWindow()); - if (packet->original_sequence_number == 0) { - sent_packet_manager_.OnSerializedPacket(packet->serialized_packet); - } else { - if (debug_visitor_.get() != NULL) { - debug_visitor_->OnPacketRetransmitted( - packet->original_sequence_number, sequence_number); - } - sent_packet_manager_.OnRetransmittedPacket(packet->original_sequence_number, - sequence_number); + if (packet->original_sequence_number != 0 && debug_visitor_.get() != NULL) { + debug_visitor_->OnPacketRetransmitted( + packet->original_sequence_number, sequence_number); } bool reset_retransmission_alarm = sent_packet_manager_.OnPacketSent( - sequence_number, + &packet->serialized_packet, + packet->original_sequence_number, now, encrypted->length(), packet->transmission_type, IsRetransmittable(*packet)); - // The SentPacketManager now owns the retransmittable frames. - packet->serialized_packet.retransmittable_frames = NULL; if (reset_retransmission_alarm || !retransmission_alarm_->IsSet()) { retransmission_alarm_->Update(sent_packet_manager_.GetRetransmissionTime(), @@ -1887,7 +1876,11 @@ void QuicConnection::SetIdleNetworkTimeout(QuicTime::Delta timeout) { if (timeout < idle_network_timeout_) { idle_network_timeout_ = timeout; - CheckForTimeout(); + if (FLAGS_quic_timeouts_only_from_alarms) { + SetTimeoutAlarm(); + } else { + CheckForTimeout(); + } } else { idle_network_timeout_ = timeout; } @@ -1896,67 +1889,67 @@ void QuicConnection::SetIdleNetworkTimeout(QuicTime::Delta timeout) { void QuicConnection::SetOverallConnectionTimeout(QuicTime::Delta timeout) { if (timeout < overall_connection_timeout_) { overall_connection_timeout_ = timeout; - CheckForTimeout(); + if (FLAGS_quic_timeouts_only_from_alarms) { + SetTimeoutAlarm(); + } else { + CheckForTimeout(); + } } else { overall_connection_timeout_ = timeout; } } -bool QuicConnection::CheckForTimeout() { +void QuicConnection::CheckForTimeout() { QuicTime now = clock_->ApproximateNow(); QuicTime time_of_last_packet = max(time_of_last_received_packet_, time_of_last_sent_new_packet_); - // If no packets have been sent or received, then don't timeout. - if (FLAGS_quic_timeouts_require_activity && - !time_of_last_packet.IsInitialized()) { - timeout_alarm_->Cancel(); - timeout_alarm_->Set(now.Add(idle_network_timeout_)); - return false; - } - // |delta| can be < 0 as |now| is approximate time but |time_of_last_packet| // is accurate time. However, this should not change the behavior of // timeout handling. - QuicTime::Delta delta = now.Subtract(time_of_last_packet); + QuicTime::Delta idle_duration = now.Subtract(time_of_last_packet); DVLOG(1) << ENDPOINT << "last packet " << time_of_last_packet.ToDebuggingValue() << " now:" << now.ToDebuggingValue() - << " delta:" << delta.ToMicroseconds() - << " network_timeout: " << idle_network_timeout_.ToMicroseconds(); - if (delta >= idle_network_timeout_) { + << " idle_duration:" << idle_duration.ToMicroseconds() + << " idle_network_timeout: " + << idle_network_timeout_.ToMicroseconds(); + if (idle_duration >= idle_network_timeout_) { DVLOG(1) << ENDPOINT << "Connection timedout due to no network activity."; SendConnectionClose(QUIC_CONNECTION_TIMED_OUT); - return true; + return; } - // Next timeout delta. - QuicTime::Delta timeout = idle_network_timeout_.Subtract(delta); - if (!overall_connection_timeout_.IsInfinite()) { - QuicTime::Delta connected_time = + QuicTime::Delta connected_duration = now.Subtract(stats_.connection_creation_time); DVLOG(1) << ENDPOINT << "connection time: " - << connected_time.ToMilliseconds() << " overall timeout: " - << overall_connection_timeout_.ToMilliseconds(); - if (connected_time >= overall_connection_timeout_) { + << connected_duration.ToMicroseconds() << " overall timeout: " + << overall_connection_timeout_.ToMicroseconds(); + if (connected_duration >= overall_connection_timeout_) { DVLOG(1) << ENDPOINT << "Connection timedout due to overall connection timeout."; SendConnectionClose(QUIC_CONNECTION_OVERALL_TIMED_OUT); - return true; + return; } + } - // Take the min timeout. - QuicTime::Delta connection_timeout = - overall_connection_timeout_.Subtract(connected_time); - if (connection_timeout < timeout) { - timeout = connection_timeout; - } + SetTimeoutAlarm(); +} + +void QuicConnection::SetTimeoutAlarm() { + QuicTime time_of_last_packet = max(time_of_last_received_packet_, + time_of_last_sent_new_packet_); + + QuicTime deadline = time_of_last_packet.Add(idle_network_timeout_); + if (!overall_connection_timeout_.IsInfinite()) { + deadline = min(deadline, + stats_.connection_creation_time.Add( + overall_connection_timeout_)); } timeout_alarm_->Cancel(); - timeout_alarm_->Set(now.Add(timeout)); - return false; + timeout_alarm_->Set(deadline); } void QuicConnection::SetPingAlarm() { diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index 5cd5d37..084460f 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -439,9 +439,9 @@ class NET_EXPORT_PRIVATE QuicConnection // handshake finishes. void SetOverallConnectionTimeout(QuicTime::Delta timeout); - // If the connection has timed out, this will close the connection and return - // true. Otherwise, it will return false and will reset the timeout alarm. - bool CheckForTimeout(); + // If the connection has timed out, this will close the connection. + // Otherwise, it will reschedule the timeout alarm. + void CheckForTimeout(); // Sends a ping, and resets the ping alarm. void SendPing(); @@ -651,6 +651,9 @@ class NET_EXPORT_PRIVATE QuicConnection // Closes any FEC groups protecting packets before |sequence_number|. void CloseFecGroupsBefore(QuicPacketSequenceNumber sequence_number); + // Sets the timeout alarm to the appropriate value, if any. + void SetTimeoutAlarm(); + // Sets the ping alarm to the appropriate value, if any. void SetPingAlarm(); diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 22ad200..c939517 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -2658,16 +2658,6 @@ TEST_P(QuicConnectionTest, InitialTimeout) { QuicTime::Delta::FromSeconds(kDefaultInitialTimeoutSecs)); EXPECT_EQ(default_timeout, connection_.GetTimeoutAlarm()->deadline()); - if (FLAGS_quic_timeouts_require_activity) { - // Simulate the timeout alarm firing. - clock_.AdvanceTime( - QuicTime::Delta::FromSeconds(kDefaultInitialTimeoutSecs)); - connection_.GetTimeoutAlarm()->Fire(); - // We should not actually timeout until a packet is sent. - EXPECT_TRUE(connection_.connected()); - SendStreamDataToPeer(1, "GET /", 0, kFin, NULL); - } - // Simulate the timeout alarm firing. clock_.AdvanceTime( QuicTime::Delta::FromSeconds(kDefaultInitialTimeoutSecs)); diff --git a/net/quic/quic_flags.cc b/net/quic/quic_flags.cc index e0ca6e6..1402580 100644 --- a/net/quic/quic_flags.cc +++ b/net/quic/quic_flags.cc @@ -35,10 +35,10 @@ bool FLAGS_close_quic_connection_unfinished_streams_2 = false; // When true, defaults to BBR congestion control instead of Cubic. bool FLAGS_quic_use_bbr_congestion_control = false; -// If true, only times out QUIC connections when there has been network -// activity; packets sent or received. -bool FLAGS_quic_timeouts_require_activity = false; - // If true, the server will accept slightly more streams than the negotiated // limit. bool FLAGS_quic_allow_more_open_streams = false; + +// If true, then QUIC connections will only timeout when an alarm fires, never +// when setting a timeout. +bool FLAGS_quic_timeouts_only_from_alarms = true; diff --git a/net/quic/quic_flags.h b/net/quic/quic_flags.h index ccba029..188211a 100644 --- a/net/quic/quic_flags.h +++ b/net/quic/quic_flags.h @@ -15,7 +15,7 @@ NET_EXPORT_PRIVATE extern bool FLAGS_send_quic_crypto_reject_reason; NET_EXPORT_PRIVATE extern bool FLAGS_enable_quic_fec; NET_EXPORT_PRIVATE extern bool FLAGS_close_quic_connection_unfinished_streams_2; NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_bbr_congestion_control; -NET_EXPORT_PRIVATE extern bool FLAGS_quic_timeouts_require_activity; NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_more_open_streams; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_timeouts_only_from_alarms; #endif // NET_QUIC_QUIC_FLAGS_H_ diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index 12ddf29..0d5913b 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -128,10 +128,8 @@ void QuicSentPacketManager::SetFromConfig(const QuicConfig& config) { } // TODO(ianswett): Remove the "HasReceivedLossDetection" branch once // the ConnectionOptions code is live everywhere. - if ((config.HasReceivedLossDetection() && - config.ReceivedLossDetection() == kTIME) || - (config.HasReceivedConnectionOptions() && - ContainsQuicTag(config.ReceivedConnectionOptions(), kTIME))) { + if (config.HasReceivedConnectionOptions() && + ContainsQuicTag(config.ReceivedConnectionOptions(), kTIME)) { loss_algorithm_.reset(LossDetectionInterface::Create(kTime)); } send_algorithm_->SetFromConfig(config, is_server_); @@ -151,6 +149,7 @@ void QuicSentPacketManager::OnSerializedPacket( unacked_packets_.AddPacket(serialized_packet); if (debug_delegate_ != NULL) { + // TODO(ianswett): Merge calls in the debug delegate. debug_delegate_->OnSerializedPacket(serialized_packet); } } @@ -505,14 +504,23 @@ QuicSentPacketManager::GetLeastUnacked() const { } bool QuicSentPacketManager::OnPacketSent( - QuicPacketSequenceNumber sequence_number, + SerializedPacket* serialized_packet, + QuicPacketSequenceNumber original_sequence_number, QuicTime sent_time, QuicByteCount bytes, TransmissionType transmission_type, HasRetransmittableData has_retransmittable_data) { + QuicPacketSequenceNumber sequence_number = serialized_packet->sequence_number; DCHECK_LT(0u, sequence_number); - DCHECK(unacked_packets_.IsUnacked(sequence_number)); + DCHECK(!unacked_packets_.IsUnacked(sequence_number)); LOG_IF(DFATAL, bytes == 0) << "Cannot send empty packets."; + if (original_sequence_number == 0) { + OnSerializedPacket(*serialized_packet); + serialized_packet->retransmittable_frames = NULL; + } else { + OnRetransmittedPacket(original_sequence_number, sequence_number); + } + if (pending_timer_transmission_count_ > 0) { --pending_timer_transmission_count_; } diff --git a/net/quic/quic_sent_packet_manager.h b/net/quic/quic_sent_packet_manager.h index e6499ff..e6ce902 100644 --- a/net/quic/quic_sent_packet_manager.h +++ b/net/quic/quic_sent_packet_manager.h @@ -115,16 +115,6 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { void SetHandshakeConfirmed() { handshake_confirmed_ = true; } - // Called when a new packet is serialized. If the packet contains - // retransmittable data, it will be added to the unacked packet map. - void OnSerializedPacket(const SerializedPacket& serialized_packet); - - // Called when a packet is retransmitted with a new sequence number. - // Replaces the old entry in the unacked packet map with the new - // sequence number. - void OnRetransmittedPacket(QuicPacketSequenceNumber old_sequence_number, - QuicPacketSequenceNumber new_sequence_number); - // Processes the incoming ack. void OnIncomingAck(const QuicAckFrame& ack_frame, QuicTime ack_receive_time); @@ -169,7 +159,8 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { // Called when we have sent bytes to the peer. This informs the manager both // the number of bytes sent and if they were retransmitted. Returns true if // the sender should reset the retransmission timer. - virtual bool OnPacketSent(QuicPacketSequenceNumber sequence_number, + virtual bool OnPacketSent(SerializedPacket* serialized_packet, + QuicPacketSequenceNumber original_sequence_number, QuicTime sent_time, QuicByteCount bytes, TransmissionType transmission_type, @@ -266,6 +257,16 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { typedef linked_hash_map PendingRetransmissionMap; + // Called when a new packet is serialized. If the packet contains + // retransmittable data, it will be added to the unacked packet map. + void OnSerializedPacket(const SerializedPacket& serialized_packet); + + // Called when a packet is retransmitted with a new sequence number. + // Replaces the old entry in the unacked packet map with the new + // sequence number. + void OnRetransmittedPacket(QuicPacketSequenceNumber old_sequence_number, + QuicPacketSequenceNumber new_sequence_number); + // Updates the least_packet_awaited_by_peer. void UpdatePacketInformationReceivedByPeer(const QuicAckFrame& ack_frame); diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc index 8a386bf..bf5c5a6 100644 --- a/net/quic/quic_sent_packet_manager_test.cc +++ b/net/quic/quic_sent_packet_manager_test.cc @@ -148,11 +148,8 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { Times(AnyNumber()); } - // Retransmits a packet as though it was a TLP retransmission, because TLP - // leaves the |old_sequence_number| pending. - // TODO(ianswett): Test with transmission types besides TLP. - void RetransmitPacket(QuicPacketSequenceNumber old_sequence_number, - QuicPacketSequenceNumber new_sequence_number) { + void RetransmitAndSendPacket(QuicPacketSequenceNumber old_sequence_number, + QuicPacketSequenceNumber new_sequence_number) { QuicSentPacketManagerPeer::MarkForRetransmission( &manager_, old_sequence_number, TLP_RETRANSMISSION); EXPECT_TRUE(manager_.HasPendingRetransmissions()); @@ -161,25 +158,20 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { EXPECT_EQ(old_sequence_number, next_retransmission.sequence_number); EXPECT_EQ(TLP_RETRANSMISSION, next_retransmission.transmission_type); - manager_.OnRetransmittedPacket(old_sequence_number, - new_sequence_number); - EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission( - &manager_, new_sequence_number)); - } - - void RetransmitAndSendPacket(QuicPacketSequenceNumber old_sequence_number, - QuicPacketSequenceNumber new_sequence_number) { - RetransmitPacket(old_sequence_number, new_sequence_number); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, BytesInFlight(), new_sequence_number, kDefaultLength, HAS_RETRANSMITTABLE_DATA)) .WillOnce(Return(true)); - manager_.OnPacketSent(new_sequence_number, + SerializedPacket packet(CreatePacket(new_sequence_number, false)); + manager_.OnPacketSent(&packet, + old_sequence_number, clock_.Now(), kDefaultLength, LOSS_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission( + &manager_, new_sequence_number)); } SerializedPacket CreateDataPacket(QuicPacketSequenceNumber sequence_number) { @@ -210,8 +202,7 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { OnPacketSent(_, BytesInFlight(), sequence_number, _, _)) .Times(1).WillOnce(Return(true)); SerializedPacket packet(CreateDataPacket(sequence_number)); - manager_.OnSerializedPacket(packet); - manager_.OnPacketSent(sequence_number, clock_.Now(), + manager_.OnPacketSent(&packet, 0, clock_.Now(), packet.packet->length(), NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); } @@ -225,8 +216,7 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { packet.retransmittable_frames->AddStreamFrame( new QuicStreamFrame(1, false, 0, IOVector())); packet.retransmittable_frames->set_encryption_level(ENCRYPTION_NONE); - manager_.OnSerializedPacket(packet); - manager_.OnPacketSent(sequence_number, clock_.ApproximateNow(), + manager_.OnPacketSent(&packet, 0, clock_.Now(), packet.packet->length(), NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); } @@ -237,8 +227,7 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { kDefaultLength, NO_RETRANSMITTABLE_DATA)) .Times(1).WillOnce(Return(true)); SerializedPacket packet(CreateFecPacket(sequence_number)); - manager_.OnSerializedPacket(packet); - manager_.OnPacketSent(sequence_number, clock_.ApproximateNow(), + manager_.OnPacketSent(&packet, 0, clock_.Now(), packet.packet->length(), NOT_RETRANSMISSION, NO_RETRANSMITTABLE_DATA); } @@ -249,8 +238,7 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { kDefaultLength, NO_RETRANSMITTABLE_DATA)) .Times(1).WillOnce(Return(false)); SerializedPacket packet(CreatePacket(sequence_number, false)); - manager_.OnSerializedPacket(packet); - manager_.OnPacketSent(sequence_number, clock_.Now(), + manager_.OnPacketSent(&packet, 0, clock_.Now(), packet.packet->length(), NOT_RETRANSMISSION, NO_RETRANSMITTABLE_DATA); } @@ -265,9 +253,9 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { .Times(1).WillOnce(Return(true)); const QuicSentPacketManager::PendingRetransmission pending = manager_.NextPendingRetransmission(); - manager_.OnRetransmittedPacket(pending.sequence_number, - retransmission_sequence_number); - manager_.OnPacketSent(retransmission_sequence_number, clock_.Now(), + SerializedPacket packet( + CreatePacket(retransmission_sequence_number, false)); + manager_.OnPacketSent(&packet, pending.sequence_number, clock_.Now(), kDefaultLength, pending.transmission_type, HAS_RETRANSMITTABLE_DATA); } @@ -282,10 +270,7 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { TEST_F(QuicSentPacketManagerTest, IsUnacked) { VerifyUnackedPackets(NULL, 0); - - SerializedPacket serialized_packet(CreateDataPacket(1)); - - manager_.OnSerializedPacket(serialized_packet); + SendDataPacket(1); QuicPacketSequenceNumber unacked[] = { 1 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -295,7 +280,7 @@ TEST_F(QuicSentPacketManagerTest, IsUnacked) { TEST_F(QuicSentPacketManagerTest, IsUnAckedRetransmit) { SendDataPacket(1); - RetransmitPacket(1, 2); + RetransmitAndSendPacket(1, 2); EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission(&manager_, 2)); QuicPacketSequenceNumber unacked[] = { 1, 2 }; @@ -345,30 +330,6 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { TEST_F(QuicSentPacketManagerTest, RetransmitThenAckPrevious) { SendDataPacket(1); - RetransmitPacket(1, 2); - QuicTime::Delta rtt = QuicTime::Delta::FromMilliseconds(15); - clock_.AdvanceTime(rtt); - - // Ack 1 but not 2. - ExpectAck(1); - QuicAckFrame ack_frame; - ack_frame.largest_observed = 1; - manager_.OnIncomingAck(ack_frame, clock_.ApproximateNow()); - - // 2 should be unacked, since it may provide an RTT measurement. - QuicPacketSequenceNumber unacked[] = { 2 }; - VerifyUnackedPackets(unacked, arraysize(unacked)); - EXPECT_FALSE(QuicSentPacketManagerPeer::HasPendingPackets(&manager_)); - VerifyRetransmittablePackets(NULL, 0); - - // Verify that the retransmission alarm would not fire, - // since there is no retransmittable data outstanding. - EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime()); - EXPECT_EQ(1u, stats_.packets_spuriously_retransmitted); -} - -TEST_F(QuicSentPacketManagerTest, RetransmitAndSendThenAckPrevious) { - SendDataPacket(1); RetransmitAndSendPacket(1, 2); QuicTime::Delta rtt = QuicTime::Delta::FromMilliseconds(15); clock_.AdvanceTime(rtt); @@ -390,11 +351,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitAndSendThenAckPrevious) { TEST_F(QuicSentPacketManagerTest, RetransmitThenAckPreviousThenNackRetransmit) { SendDataPacket(1); - RetransmitPacket(1, 2); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, 2, _, _)) - .WillOnce(Return(true)); - manager_.OnPacketSent(2, clock_.ApproximateNow(), kDefaultLength, - LOSS_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + RetransmitAndSendPacket(1, 2); QuicTime::Delta rtt = QuicTime::Delta::FromMilliseconds(15); clock_.AdvanceTime(rtt); @@ -648,16 +605,12 @@ TEST_F(QuicSentPacketManagerTest, GetLeastUnacked) { } TEST_F(QuicSentPacketManagerTest, GetLeastUnackedUnacked) { - SerializedPacket serialized_packet(CreateDataPacket(1)); - - manager_.OnSerializedPacket(serialized_packet); + SendDataPacket(1); EXPECT_EQ(1u, manager_.GetLeastUnacked()); } TEST_F(QuicSentPacketManagerTest, GetLeastUnackedUnackedFec) { - SerializedPacket serialized_packet(CreateFecPacket(1)); - - manager_.OnSerializedPacket(serialized_packet); + SendFecPacket(1); EXPECT_EQ(1u, manager_.GetLeastUnacked()); } @@ -690,29 +643,17 @@ TEST_F(QuicSentPacketManagerTest, GetLeastUnackedAndDiscard) { TEST_F(QuicSentPacketManagerTest, GetSentTime) { VerifyUnackedPackets(NULL, 0); - SerializedPacket serialized_packet(CreateFecPacket(1)); - manager_.OnSerializedPacket(serialized_packet); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, 1, _, _)) - .Times(1).WillOnce(Return(true)); - manager_.OnPacketSent(1, QuicTime::Zero(), kDefaultLength, NOT_RETRANSMISSION, - NO_RETRANSMITTABLE_DATA); - - SerializedPacket serialized_packet2(CreateFecPacket(2)); - QuicTime sent_time = QuicTime::Zero().Add(QuicTime::Delta::FromSeconds(1)); - manager_.OnSerializedPacket(serialized_packet2); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, 2, _, _)) - .Times(1).WillOnce(Return(true)); - manager_.OnPacketSent(2, sent_time, kDefaultLength, NOT_RETRANSMISSION, - NO_RETRANSMITTABLE_DATA); - + QuicTime sent_time = clock_.Now(); + SendFecPacket(1); + QuicTime sent_time2 = clock_.Now(); + SendFecPacket(2); QuicPacketSequenceNumber unacked[] = { 1, 2 }; VerifyUnackedPackets(unacked, arraysize(unacked)); VerifyRetransmittablePackets(NULL, 0); EXPECT_TRUE(manager_.HasUnackedPackets()); - EXPECT_EQ(QuicTime::Zero(), - QuicSentPacketManagerPeer::GetSentTime(&manager_, 1)); - EXPECT_EQ(sent_time, QuicSentPacketManagerPeer::GetSentTime(&manager_, 2)); + EXPECT_EQ(sent_time, QuicSentPacketManagerPeer::GetSentTime(&manager_, 1)); + EXPECT_EQ(sent_time2, QuicSentPacketManagerPeer::GetSentTime(&manager_, 2)); } TEST_F(QuicSentPacketManagerTest, AckAckAndUpdateRtt) { @@ -1020,13 +961,12 @@ TEST_F(QuicSentPacketManagerTest, CryptoHandshakeSpuriousRetransmission) { } TEST_F(QuicSentPacketManagerTest, CryptoHandshakeTimeoutUnsentDataPacket) { - // Send 2 crypto packets and serialize 1 data packet. + // Send 2 crypto packets and 1 data packet. const size_t kNumSentCryptoPackets = 2; for (size_t i = 1; i <= kNumSentCryptoPackets; ++i) { SendCryptoPacket(i); } - SerializedPacket packet(CreateDataPacket(3)); - manager_.OnSerializedPacket(packet); + SendDataPacket(3); EXPECT_TRUE(QuicSentPacketManagerPeer::HasUnackedCryptoPackets(&manager_)); // Retransmit 2 crypto packets, but not the serialized packet. @@ -1092,32 +1032,6 @@ TEST_F(QuicSentPacketManagerTest, VerifyRetransmittablePackets(NULL, 0); } -TEST_F(QuicSentPacketManagerTest, TailLossProbeTimeoutUnsentDataPacket) { - QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); - // Serialize two data packets and send the latter. - SerializedPacket packet(CreateDataPacket(1)); - manager_.OnSerializedPacket(packet); - SendDataPacket(2); - EXPECT_FALSE(QuicSentPacketManagerPeer::HasUnackedCryptoPackets(&manager_)); - EXPECT_TRUE(QuicSentPacketManagerPeer::HasPendingPackets(&manager_)); - - // Retransmit 1 unacked packets, but not the first serialized packet. - manager_.OnRetransmissionTimeout(); - EXPECT_EQ(QuicTime::Delta::Zero(), - manager_.TimeUntilSend(clock_.Now(), HAS_RETRANSMITTABLE_DATA)); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - manager_.MaybeRetransmitTailLossProbe(); - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(3); - EXPECT_CALL(*send_algorithm_, TimeUntilSend(_, _, _)).WillOnce(Return( - QuicTime::Delta::Infinite())); - EXPECT_EQ(QuicTime::Delta::Infinite(), - manager_.TimeUntilSend(clock_.Now(), HAS_RETRANSMITTABLE_DATA)); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - EXPECT_FALSE(QuicSentPacketManagerPeer::HasUnackedCryptoPackets(&manager_)); - EXPECT_TRUE(QuicSentPacketManagerPeer::HasPendingPackets(&manager_)); -} - TEST_F(QuicSentPacketManagerTest, ResetRecentMinRTTWithEmptyWindow) { QuicTime::Delta min_rtt = QuicTime::Delta::FromMilliseconds(50); QuicSentPacketManagerPeer::GetRttStats(&manager_)->UpdateRtt( @@ -1372,24 +1286,6 @@ TEST_F(QuicSentPacketManagerTest, GetLossDelay) { manager_.OnRetransmissionTimeout(); } -TEST_F(QuicSentPacketManagerTest, NegotiateTimeLossDetection) { - EXPECT_EQ(kNack, - QuicSentPacketManagerPeer::GetLossAlgorithm( - &manager_)->GetLossDetectionType()); - - QuicConfig config; - QuicConfigPeer::SetReceivedLossDetection(&config, kTIME); - EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _)); - EXPECT_CALL(*network_change_visitor_, OnCongestionWindowChange(_)); - EXPECT_CALL(*send_algorithm_, GetCongestionWindow()) - .WillOnce(Return(100 * kDefaultTCPMSS)); - manager_.SetFromConfig(config); - - EXPECT_EQ(kTime, - QuicSentPacketManagerPeer::GetLossAlgorithm( - &manager_)->GetLossDetectionType()); -} - TEST_F(QuicSentPacketManagerTest, NegotiateTimeLossDetectionFromOptions) { EXPECT_EQ(kNack, QuicSentPacketManagerPeer::GetLossAlgorithm( diff --git a/net/quic/quic_server_session.cc b/net/quic/quic_server_session.cc index aefb021..76fae7b 100644 --- a/net/quic/quic_server_session.cc +++ b/net/quic/quic_server_session.cc @@ -128,6 +128,8 @@ void QuicServerSession::OnCongestionWindowChange(QuicTime now) { bandwidth_recorder.EstimateRecordedDuringSlowStart() ? CachedNetworkParameters::SLOW_START : CachedNetworkParameters::CONGESTION_AVOIDANCE); + cached_network_params.set_timestamp( + connection()->clock()->WallNow().ToUNIXSeconds()); if (!serving_region_.empty()) { cached_network_params.set_serving_region(serving_region_); } diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc index c7aeb4c..d1587e1 100644 --- a/net/quic/quic_stream_factory.cc +++ b/net/quic/quic_stream_factory.cc @@ -88,12 +88,9 @@ bool IsEcdsaSupported() { return true; } -QuicConfig InitializeQuicConfig(bool enable_time_based_loss_detection, - const QuicTagVector& connection_options) { +QuicConfig InitializeQuicConfig(const QuicTagVector& connection_options) { QuicConfig config; config.SetDefaults(); - if (enable_time_based_loss_detection) - config.SetLossDetectionToSend(kTIME); config.set_idle_connection_state_lifetime( QuicTime::Delta::FromSeconds(kIdleConnectionTimeoutSeconds), QuicTime::Delta::FromSeconds(kIdleConnectionTimeoutSeconds)); @@ -491,7 +488,6 @@ QuicStreamFactory::QuicStreamFactory( const std::string& user_agent_id, const QuicVersionVector& supported_versions, bool enable_port_selection, - bool enable_time_based_loss_detection, bool always_require_handshake_confirmation, bool disable_connection_pooling, const QuicTagVector& connection_options) @@ -505,8 +501,7 @@ QuicStreamFactory::QuicStreamFactory( random_generator_(random_generator), clock_(clock), max_packet_length_(max_packet_length), - config_(InitializeQuicConfig(enable_time_based_loss_detection, - connection_options)), + config_(InitializeQuicConfig(connection_options)), supported_versions_(supported_versions), enable_port_selection_(enable_port_selection), always_require_handshake_confirmation_( diff --git a/net/quic/quic_stream_factory.h b/net/quic/quic_stream_factory.h index 410a7af..f4aa77e7 100644 --- a/net/quic/quic_stream_factory.h +++ b/net/quic/quic_stream_factory.h @@ -102,7 +102,6 @@ class NET_EXPORT_PRIVATE QuicStreamFactory const std::string& user_agent_id, const QuicVersionVector& supported_versions, bool enable_port_selection, - bool enable_time_based_loss_detection, bool always_require_handshake_confirmation, bool disable_connection_pooling, const QuicTagVector& connection_options); diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc index 374eda4..341ecb0 100644 --- a/net/quic/quic_stream_factory_test.cc +++ b/net/quic/quic_stream_factory_test.cc @@ -113,7 +113,6 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam { std::string(), SupportedVersions(GetParam()), /*enable_port_selection=*/true, - /*enable_time_based_loss_detection=*/true, /*always_require_handshake_confirmation=*/false, /*disable_connection_pooling=*/false, QuicTagVector()), diff --git a/net/quic/test_tools/quic_config_peer.cc b/net/quic/test_tools/quic_config_peer.cc index 1d1d6d1..f700171 100644 --- a/net/quic/test_tools/quic_config_peer.cc +++ b/net/quic/test_tools/quic_config_peer.cc @@ -16,12 +16,6 @@ void QuicConfigPeer::SetReceivedInitialWindow(QuicConfig* config, } // static -void QuicConfigPeer::SetReceivedLossDetection(QuicConfig* config, - QuicTag loss_detection) { - config->loss_detection_.SetReceivedValue(loss_detection); -} - -// static void QuicConfigPeer::SetReceivedInitialFlowControlWindow(QuicConfig* config, uint32 window_bytes) { config->initial_flow_control_window_bytes_.SetReceivedValue(window_bytes); diff --git a/net/quic/test_tools/quic_config_peer.h b/net/quic/test_tools/quic_config_peer.h index 26f45d9..8e9a5ea 100644 --- a/net/quic/test_tools/quic_config_peer.h +++ b/net/quic/test_tools/quic_config_peer.h @@ -18,9 +18,6 @@ class QuicConfigPeer { static void SetReceivedInitialWindow(QuicConfig* config, size_t initial_window); - static void SetReceivedLossDetection(QuicConfig* config, - QuicTag loss_detection); - // TODO(rjshade): Remove when removing QUIC_VERSION_19. static void SetReceivedInitialFlowControlWindow(QuicConfig* config, uint32 window_bytes); diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc index 98bde10..8926727 100644 --- a/net/quic/test_tools/quic_test_utils.cc +++ b/net/quic/test_tools/quic_test_utils.cc @@ -311,7 +311,9 @@ void PacketSavingConnection::SendOrQueuePacket(QueuedPacket packet) { encrypted_packets_.push_back(encrypted); // Transfer ownership of the packet to the SentPacketManager and the // ack notifier to the AckNotifierManager. - sent_packet_manager_.OnSerializedPacket(packet.serialized_packet); + sent_packet_manager_.OnPacketSent( + &packet.serialized_packet, 0, QuicTime::Zero(), 1000, + NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); } MockSession::MockSession(QuicConnection* connection) diff --git a/net/tools/quic/quic_server_session.cc b/net/tools/quic/quic_server_session.cc index 64ac110..e2245ae 100644 --- a/net/tools/quic/quic_server_session.cc +++ b/net/tools/quic/quic_server_session.cc @@ -128,6 +128,8 @@ void QuicServerSession::OnCongestionWindowChange(QuicTime now) { bandwidth_recorder.EstimateRecordedDuringSlowStart() ? CachedNetworkParameters::SLOW_START : CachedNetworkParameters::CONGESTION_AVOIDANCE); + cached_network_params.set_timestamp( + connection()->clock()->WallNow().ToUNIXSeconds()); if (!serving_region_.empty()) { cached_network_params.set_serving_region(serving_region_); } diff --git a/net/tools/quic/quic_server_session_test.cc b/net/tools/quic/quic_server_session_test.cc index 887c0b1..a2e55cb 100644 --- a/net/tools/quic/quic_server_session_test.cc +++ b/net/tools/quic/quic_server_session_test.cc @@ -352,6 +352,8 @@ TEST_P(QuicServerSessionTest, BandwidthEstimates) { .ToMilliseconds()); expected_network_params.set_previous_connection_state( CachedNetworkParameters::CONGESTION_AVOIDANCE); + expected_network_params.set_timestamp( + session_->connection()->clock()->WallNow().ToUNIXSeconds()); expected_network_params.set_serving_region(serving_region); EXPECT_CALL(*crypto_stream, -- cgit v1.1