Rename, refactor, and document "HttpServerProperties::SupportsQuic()"
This method name is confusing, since
HttpServerProperties::SupportsSpdy() is used to store whether a server
supports SPDY/H2, but SupportsQuic() actually means a network where
QUIC works (i.e., the ISP isn't breaking QUIC, or something similar).
This difference in meaning was also not documented anywhere.
This CL renames "SupportsQuic" to "LastLocalAddressWhenQuicWorked",
adds documentation, and reworks the API slightly - its behavior
around empty IPAddresses was a little weird, as was passing in an
IPAddress() and a bool when clearing information for all IP addresses.
This CL also renames QuicFactory::requires_confirmation_ to
is_quic_known_to_work_on_current_network_ (which also requires
inverting its value) and adds some documentation for it, as it was
similarly confusing.
Bug: 969890
Change-Id: Id51b082343ff31ecd36f1bedfab8342307da26ee
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1790325
Commit-Queue: Matt Menke <[email protected]>
Reviewed-by: Ryan Hamilton <[email protected]>
Reviewed-by: Zhongyi Shi <[email protected]>
Cr-Commit-Position: refs/heads/master@{#695833}
diff --git a/net/http/http_server_properties.cc b/net/http/http_server_properties.cc
index 9dff090..63ee667f 100644
--- a/net/http/http_server_properties.cc
+++ b/net/http/http_server_properties.cc
@@ -155,7 +155,7 @@
server_info_map_.Clear();
broken_alternative_services_.Clear();
canonical_alt_svc_map_.clear();
- last_quic_address_ = IPAddress();
+ last_local_address_when_quic_worked_ = IPAddress();
quic_server_info_map_.Clear();
canonical_server_info_map_.clear();
@@ -606,24 +606,33 @@
return std::move(dict_list);
}
-bool HttpServerProperties::GetSupportsQuic(IPAddress* last_address) const {
- if (last_quic_address_.empty())
- return false;
-
- *last_address = last_quic_address_;
- return true;
+bool HttpServerProperties::WasLastLocalAddressWhenQuicWorked(
+ const IPAddress& local_address) const {
+ return !last_local_address_when_quic_worked_.empty() &&
+ last_local_address_when_quic_worked_ == local_address;
}
-void HttpServerProperties::SetSupportsQuic(bool used_quic,
- const IPAddress& address) {
- IPAddress new_quic_address;
- if (used_quic)
- new_quic_address = address;
+bool HttpServerProperties::HasLastLocalAddressWhenQuicWorked() const {
+ return !last_local_address_when_quic_worked_.empty();
+}
- if (new_quic_address == last_quic_address_)
+void HttpServerProperties::SetLastLocalAddressWhenQuicWorked(
+ IPAddress last_local_address_when_quic_worked) {
+ DCHECK(!last_local_address_when_quic_worked.empty());
+ if (last_local_address_when_quic_worked_ ==
+ last_local_address_when_quic_worked) {
+ return;
+ }
+
+ last_local_address_when_quic_worked_ = last_local_address_when_quic_worked;
+ MaybeQueueWriteProperties();
+}
+
+void HttpServerProperties::ClearLastLocalAddressWhenQuicWorked() {
+ if (last_local_address_when_quic_worked_.empty())
return;
- last_quic_address_ = new_quic_address;
+ last_local_address_when_quic_worked_ = IPAddress();
MaybeQueueWriteProperties();
}
@@ -906,7 +915,7 @@
void HttpServerProperties::OnPrefsLoaded(
std::unique_ptr<ServerInfoMap> server_info_map,
- const IPAddress& last_quic_address,
+ const IPAddress& last_local_address_when_quic_worked,
std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
std::unique_ptr<BrokenAlternativeServiceList>
broken_alternative_service_list,
@@ -920,7 +929,7 @@
// service fields).
if (server_info_map) {
OnServerInfoLoaded(std::move(server_info_map));
- OnSupportsQuicLoaded(last_quic_address);
+ OnLastLocalAddressWhenQuicWorkedLoaded(last_local_address_when_quic_worked);
OnQuicServerInfoMapLoaded(std::move(quic_server_info_map));
if (recently_broken_alternative_services) {
DCHECK(broken_alternative_service_list);
@@ -1011,8 +1020,9 @@
}
}
-void HttpServerProperties::OnSupportsQuicLoaded(const IPAddress& last_address) {
- last_quic_address_ = last_address;
+void HttpServerProperties::OnLastLocalAddressWhenQuicWorkedLoaded(
+ const IPAddress& last_local_address_when_quic_worked) {
+ last_local_address_when_quic_worked_ = last_local_address_when_quic_worked;
}
void HttpServerProperties::OnQuicServerInfoMapLoaded(
@@ -1082,7 +1092,7 @@
server_info_map_,
base::BindRepeating(&HttpServerProperties::GetCanonicalSuffix,
base::Unretained(this)),
- last_quic_address_, quic_server_info_map_,
+ last_local_address_when_quic_worked_, quic_server_info_map_,
broken_alternative_services_.broken_alternative_service_list(),
broken_alternative_services_.recently_broken_alternative_services(),
std::move(callback));
diff --git a/net/http/http_server_properties.h b/net/http/http_server_properties.h
index 78aab4cf..8331cd3 100644
--- a/net/http/http_server_properties.h
+++ b/net/http/http_server_properties.h
@@ -343,9 +343,14 @@
// Empty alternative service hostnames will be printed as such.
std::unique_ptr<base::Value> GetAlternativeServiceInfoAsValue() const;
- bool GetSupportsQuic(IPAddress* last_address) const;
-
- void SetSupportsQuic(bool used_quic, const IPAddress& last_address);
+ // Tracks the last local address when QUIC was known to work. The address
+ // cannot be set to an empty address - use
+ // ClearLastLocalAddressWhenQuicWorked() if it needs to be cleared.
+ bool WasLastLocalAddressWhenQuicWorked(const IPAddress& local_address) const;
+ bool HasLastLocalAddressWhenQuicWorked() const;
+ void SetLastLocalAddressWhenQuicWorked(
+ IPAddress last_local_address_when_quic_worked);
+ void ClearLastLocalAddressWhenQuicWorked();
// Sets |stats| for |server|.
void SetServerNetworkStats(const url::SchemeHostPort& server,
@@ -396,8 +401,9 @@
std::unique_ptr<ServerInfoMap> server_info_map) {
OnServerInfoLoaded(std::move(server_info_map));
}
- void OnSupportsQuicLoadedForTesting(const IPAddress& last_address) {
- OnSupportsQuicLoaded(last_address);
+ void OnLastLocalAddressWhenQuicWorkedForTesting(
+ const IPAddress& last_local_address_when_quic_worked) {
+ OnLastLocalAddressWhenQuicWorkedLoaded(last_local_address_when_quic_worked);
}
void OnQuicServerInfoMapLoadedForTesting(
std::unique_ptr<QuicServerInfoMap> quic_server_info_map) {
@@ -479,20 +485,20 @@
// exists.
const std::string* GetCanonicalSuffix(const std::string& host) const;
- void OnPrefsLoaded(
- std::unique_ptr<ServerInfoMap> server_info_map,
- const IPAddress& last_quic_address,
- std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
- std::unique_ptr<BrokenAlternativeServiceList>
- broken_alternative_service_list,
- std::unique_ptr<RecentlyBrokenAlternativeServices>
- recently_broken_alternative_services);
+ void OnPrefsLoaded(std::unique_ptr<ServerInfoMap> server_info_map,
+ const IPAddress& last_local_address_when_quic_worked,
+ std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
+ std::unique_ptr<BrokenAlternativeServiceList>
+ broken_alternative_service_list,
+ std::unique_ptr<RecentlyBrokenAlternativeServices>
+ recently_broken_alternative_services);
// These methods are called by OnPrefsLoaded to handle merging properties
// loaded from prefs with what has been learned while waiting for prefs to
// load.
void OnServerInfoLoaded(std::unique_ptr<ServerInfoMap> server_info_map);
- void OnSupportsQuicLoaded(const IPAddress& last_address);
+ void OnLastLocalAddressWhenQuicWorkedLoaded(
+ const IPAddress& last_local_address_when_quic_worked);
void OnQuicServerInfoMapLoaded(
std::unique_ptr<QuicServerInfoMap> quic_server_info_map);
void OnBrokenAndRecentlyBrokenAlternativeServicesLoaded(
@@ -533,7 +539,7 @@
BrokenAlternativeServices broken_alternative_services_;
- IPAddress last_quic_address_;
+ IPAddress last_local_address_when_quic_worked_;
// Contains a map of servers which could share the same alternate protocol.
// Map from a Canonical scheme/host/port (host is some postfix of host names)
// to an actual origin, which has a plausible alternate protocol mapping.
diff --git a/net/http/http_server_properties_manager.cc b/net/http/http_server_properties_manager.cc
index 12a28ac2..9a9deeab 100644
--- a/net/http/http_server_properties_manager.cc
+++ b/net/http/http_server_properties_manager.cc
@@ -184,7 +184,7 @@
void HttpServerPropertiesManager::ReadPrefs(
std::unique_ptr<HttpServerProperties::ServerInfoMap>* server_info_map,
- IPAddress* last_quic_address,
+ IPAddress* last_local_address_when_quic_worked,
std::unique_ptr<QuicServerInfoMap>* quic_server_info_map,
std::unique_ptr<BrokenAlternativeServiceList>*
broken_alternative_service_list,
@@ -231,7 +231,8 @@
return;
}
- ReadSupportsQuic(*http_server_properties_dict, last_quic_address);
+ ReadLastLocalAddressWhenQuicWorked(*http_server_properties_dict,
+ last_local_address_when_quic_worked);
*server_info_map = std::make_unique<HttpServerProperties::ServerInfoMap>();
*quic_server_info_map = std::make_unique<QuicServerInfoMap>(
@@ -574,9 +575,9 @@
return true;
}
-void HttpServerPropertiesManager::ReadSupportsQuic(
+void HttpServerPropertiesManager::ReadLastLocalAddressWhenQuicWorked(
const base::DictionaryValue& http_server_properties_dict,
- IPAddress* last_quic_address) {
+ IPAddress* last_local_address_when_quic_worked) {
const base::DictionaryValue* supports_quic_dict = nullptr;
if (!http_server_properties_dict.GetDictionaryWithoutPathExpansion(
kSupportsQuicKey, &supports_quic_dict)) {
@@ -594,7 +595,7 @@
std::string address;
if (!supports_quic_dict->GetStringWithoutPathExpansion(kAddressKey,
&address) ||
- !last_quic_address->AssignFromIPLiteral(address)) {
+ !last_local_address_when_quic_worked->AssignFromIPLiteral(address)) {
DVLOG(1) << "Malformed SupportsQuic";
}
}
@@ -667,7 +668,7 @@
void HttpServerPropertiesManager::WriteToPrefs(
const HttpServerProperties::ServerInfoMap& server_info_map,
const GetCannonicalSuffix& get_canonical_suffix,
- const IPAddress& last_quic_address,
+ const IPAddress& last_local_address_when_quic_worked,
const QuicServerInfoMap& quic_server_info_map,
const BrokenAlternativeServiceList& broken_alternative_service_list,
const RecentlyBrokenAlternativeServices&
@@ -730,8 +731,8 @@
http_server_properties_dict.SetInteger(kVersionKey, kVersionNumber);
- if (last_quic_address.IsValid())
- SaveSupportsQuicToPrefs(last_quic_address, &http_server_properties_dict);
+ SaveLastLocalAddressWhenQuicWorkedToPrefs(last_local_address_when_quic_worked,
+ &http_server_properties_dict);
SaveQuicServerInfoMapToServerPrefs(quic_server_info_map,
&http_server_properties_dict);
@@ -784,15 +785,16 @@
kAlternativeServiceKey, std::move(alternative_service_list));
}
-void HttpServerPropertiesManager::SaveSupportsQuicToPrefs(
- const IPAddress& last_quic_address,
+void HttpServerPropertiesManager::SaveLastLocalAddressWhenQuicWorkedToPrefs(
+ const IPAddress& last_local_address_when_quic_worked,
base::DictionaryValue* http_server_properties_dict) {
- if (!last_quic_address.IsValid())
+ if (!last_local_address_when_quic_worked.IsValid())
return;
auto supports_quic_dict = std::make_unique<base::DictionaryValue>();
supports_quic_dict->SetBoolean(kUsedQuicKey, true);
- supports_quic_dict->SetString(kAddressKey, last_quic_address.ToString());
+ supports_quic_dict->SetString(kAddressKey,
+ last_local_address_when_quic_worked.ToString());
http_server_properties_dict->SetWithoutPathExpansion(
kSupportsQuicKey, std::move(supports_quic_dict));
}
@@ -920,18 +922,18 @@
return;
std::unique_ptr<HttpServerProperties::ServerInfoMap> server_info_map;
- IPAddress last_quic_address;
+ IPAddress last_local_address_when_quic_worked;
std::unique_ptr<QuicServerInfoMap> quic_server_info_map;
std::unique_ptr<BrokenAlternativeServiceList> broken_alternative_service_list;
std::unique_ptr<RecentlyBrokenAlternativeServices>
recently_broken_alternative_services;
- ReadPrefs(&server_info_map, &last_quic_address, &quic_server_info_map,
- &broken_alternative_service_list,
+ ReadPrefs(&server_info_map, &last_local_address_when_quic_worked,
+ &quic_server_info_map, &broken_alternative_service_list,
&recently_broken_alternative_services);
std::move(on_prefs_loaded_callback_)
- .Run(std::move(server_info_map), last_quic_address,
+ .Run(std::move(server_info_map), last_local_address_when_quic_worked,
std::move(quic_server_info_map),
std::move(broken_alternative_service_list),
std::move(recently_broken_alternative_services));
diff --git a/net/http/http_server_properties_manager.h b/net/http/http_server_properties_manager.h
index 4b656fb6..a7530448 100644
--- a/net/http/http_server_properties_manager.h
+++ b/net/http/http_server_properties_manager.h
@@ -42,7 +42,7 @@
// |recently_broken_alternative_services|, which may be null.
using OnPrefsLoadedCallback = base::OnceCallback<void(
std::unique_ptr<HttpServerProperties::ServerInfoMap> server_info_map,
- const IPAddress& last_quic_address,
+ const IPAddress& last_local_address_when_quic_worked,
std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
std::unique_ptr<BrokenAlternativeServiceList>
broken_alternative_service_list,
@@ -81,7 +81,7 @@
// simpler API.
void ReadPrefs(
std::unique_ptr<HttpServerProperties::ServerInfoMap>* server_info_map,
- IPAddress* last_quic_address,
+ IPAddress* last_local_address_when_quic_worked,
std::unique_ptr<QuicServerInfoMap>* quic_server_info_map,
std::unique_ptr<BrokenAlternativeServiceList>*
broken_alternative_service_list,
@@ -106,7 +106,7 @@
void WriteToPrefs(
const HttpServerProperties::ServerInfoMap& server_info_map,
const GetCannonicalSuffix& get_canonical_suffix,
- const IPAddress& last_quic_address,
+ const IPAddress& last_local_address_when_quic_worked,
const QuicServerInfoMap& quic_server_info_map,
const BrokenAlternativeServiceList& broken_alternative_service_list,
const RecentlyBrokenAlternativeServices&
@@ -157,8 +157,9 @@
const base::DictionaryValue& server_dict,
HttpServerProperties::ServerInfo* server_info);
- void ReadSupportsQuic(const base::DictionaryValue& server_dict,
- IPAddress* last_quic_address);
+ void ReadLastLocalAddressWhenQuicWorked(
+ const base::DictionaryValue& server_dict,
+ IPAddress* last_local_address_when_quic_worked);
void ParseNetworkStats(const url::SchemeHostPort& server,
const base::DictionaryValue& server_dict,
HttpServerProperties::ServerInfo* server_info);
@@ -173,8 +174,8 @@
void SaveAlternativeServiceToServerPrefs(
const AlternativeServiceInfoVector& alternative_service_info_vector,
base::DictionaryValue* server_pref_dict);
- void SaveSupportsQuicToPrefs(
- const IPAddress& last_quic_address,
+ void SaveLastLocalAddressWhenQuicWorkedToPrefs(
+ const IPAddress& last_local_address_when_quic_worked,
base::DictionaryValue* http_server_properties_dict);
void SaveNetworkStatsToServerPrefs(
const ServerNetworkStats& server_network_stats,
diff --git a/net/http/http_server_properties_manager_unittest.cc b/net/http/http_server_properties_manager_unittest.cc
index 083faee..b09091d 100644
--- a/net/http/http_server_properties_manager_unittest.cc
+++ b/net/http/http_server_properties_manager_unittest.cc
@@ -853,16 +853,16 @@
alternative_service, NetworkIsolationKey()));
}
-TEST_F(HttpServerPropertiesManagerTest, SupportsQuic) {
+TEST_F(HttpServerPropertiesManagerTest, LastLocalAddressWhenQuicWorked) {
InitializePrefs();
- IPAddress address;
- EXPECT_FALSE(http_server_props_->GetSupportsQuic(&address));
-
IPAddress actual_address(127, 0, 0, 1);
- http_server_props_->SetSupportsQuic(true, actual_address);
+ EXPECT_FALSE(http_server_props_->HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(
+ http_server_props_->WasLastLocalAddressWhenQuicWorked(actual_address));
+ http_server_props_->SetLastLocalAddressWhenQuicWorked(actual_address);
// Another task should not be scheduled.
- http_server_props_->SetSupportsQuic(true, actual_address);
+ http_server_props_->SetLastLocalAddressWhenQuicWorked(actual_address);
// Run the task.
EXPECT_EQ(0, pref_delegate_->GetAndClearNumPrefUpdates());
@@ -870,11 +870,11 @@
FastForwardUntilNoTasksRemain();
EXPECT_EQ(1, pref_delegate_->GetAndClearNumPrefUpdates());
- EXPECT_TRUE(http_server_props_->GetSupportsQuic(&address));
- EXPECT_EQ(actual_address, address);
+ EXPECT_TRUE(
+ http_server_props_->WasLastLocalAddressWhenQuicWorked(actual_address));
// Another task should not be scheduled.
- http_server_props_->SetSupportsQuic(true, actual_address);
+ http_server_props_->SetLastLocalAddressWhenQuicWorked(actual_address);
EXPECT_EQ(0, pref_delegate_->GetAndClearNumPrefUpdates());
EXPECT_EQ(0u, GetPendingMainThreadTaskCount());
}
@@ -974,7 +974,7 @@
http_server_props_->MarkAlternativeServiceBroken(broken_alternative_service,
NetworkIsolationKey());
http_server_props_->SetSupportsSpdy(spdy_server, NetworkIsolationKey(), true);
- http_server_props_->SetSupportsQuic(true, actual_address);
+ http_server_props_->SetLastLocalAddressWhenQuicWorked(actual_address);
ServerNetworkStats stats;
stats.srtt = base::TimeDelta::FromMicroseconds(10);
http_server_props_->SetServerNetworkStats(spdy_server, NetworkIsolationKey(),
@@ -993,9 +993,8 @@
EXPECT_TRUE(http_server_props_->SupportsRequestPriority(
spdy_server, NetworkIsolationKey()));
EXPECT_TRUE(HasAlternativeService(spdy_server, NetworkIsolationKey()));
- IPAddress address;
- EXPECT_TRUE(http_server_props_->GetSupportsQuic(&address));
- EXPECT_EQ(actual_address, address);
+ EXPECT_TRUE(
+ http_server_props_->WasLastLocalAddressWhenQuicWorked(actual_address));
const ServerNetworkStats* stats1 = http_server_props_->GetServerNetworkStats(
spdy_server, NetworkIsolationKey());
EXPECT_EQ(10, stats1->srtt.ToInternalValue());
@@ -1021,7 +1020,7 @@
EXPECT_FALSE(http_server_props_->SupportsRequestPriority(
spdy_server, NetworkIsolationKey()));
EXPECT_FALSE(HasAlternativeService(spdy_server, NetworkIsolationKey()));
- EXPECT_FALSE(http_server_props_->GetSupportsQuic(&address));
+ EXPECT_FALSE(http_server_props_->HasLastLocalAddressWhenQuicWorked());
const ServerNetworkStats* stats2 = http_server_props_->GetServerNetworkStats(
spdy_server, NetworkIsolationKey());
EXPECT_EQ(nullptr, stats2);
@@ -1031,7 +1030,7 @@
// https://crbug.com/444956: Add 200 alternative_service servers followed by
// supports_quic and verify we have read supports_quic from prefs.
-TEST_F(HttpServerPropertiesManagerTest, BadSupportsQuic) {
+TEST_F(HttpServerPropertiesManagerTest, BadLastLocalAddressWhenQuicWorked) {
std::unique_ptr<base::ListValue> servers_list =
std::make_unique<base::ListValue>();
@@ -1086,10 +1085,9 @@
EXPECT_EQ(i, alternative_service_info_vector[0].alternative_service().port);
}
- // Verify SupportsQuic.
- IPAddress address;
- ASSERT_TRUE(http_server_props_->GetSupportsQuic(&address));
- EXPECT_EQ("127.0.0.1", address.ToString());
+ // Verify WasLastLocalAddressWhenQuicWorked.
+ ASSERT_TRUE(http_server_props_->WasLastLocalAddressWhenQuicWorked(
+ IPAddress::IPv4Localhost()));
}
TEST_F(HttpServerPropertiesManagerTest, UpdatePrefsWithCache) {
@@ -1149,7 +1147,7 @@
// #6: Set SupportsQuic.
IPAddress actual_address(127, 0, 0, 1);
- http_server_props_->SetSupportsQuic(true, actual_address);
+ http_server_props_->SetLastLocalAddressWhenQuicWorked(actual_address);
base::Time time_before_prefs_update = base::Time::Now();
@@ -1512,7 +1510,7 @@
// #5: Set SupportsQuic.
IPAddress actual_address(127, 0, 0, 1);
- http_server_props_->SetSupportsQuic(true, actual_address);
+ http_server_props_->SetLastLocalAddressWhenQuicWorked(actual_address);
// Update Prefs.
EXPECT_EQ(0, pref_delegate_->GetAndClearNumPrefUpdates());
@@ -1631,7 +1629,7 @@
// Set SupportsQuic.
IPAddress actual_address(127, 0, 0, 1);
- http_server_props_->SetSupportsQuic(true, actual_address);
+ http_server_props_->SetLastLocalAddressWhenQuicWorked(actual_address);
// Update Prefs.
EXPECT_EQ(0, pref_delegate_->GetAndClearNumPrefUpdates());
@@ -1981,7 +1979,8 @@
// Verify supports QUIC.
//
IPAddress actual_address(127, 0, 0, 1);
- EXPECT_TRUE(http_server_props_->GetSupportsQuic(&actual_address));
+ EXPECT_TRUE(
+ http_server_props_->WasLastLocalAddressWhenQuicWorked(actual_address));
EXPECT_EQ(4, pref_delegate_->GetAndClearNumPrefUpdates());
}
diff --git a/net/http/http_server_properties_unittest.cc b/net/http/http_server_properties_unittest.cc
index 8b58814..da69834e 100644
--- a/net/http/http_server_properties_unittest.cc
+++ b/net/http/http_server_properties_unittest.cc
@@ -2277,41 +2277,84 @@
EXPECT_EQ(expected_json, alternative_service_info_json);
}
-typedef HttpServerPropertiesTest SupportsQuicServerPropertiesTest;
-
-TEST_F(SupportsQuicServerPropertiesTest, Set) {
- HostPortPair quic_server_google("www.google.com", 443);
+TEST_F(HttpServerPropertiesTest, LoadLastLocalAddressWhenQuicWorked) {
+ const IPAddress kEmptyAddress;
+ const IPAddress kValidAddress1 = IPAddress::IPv4Localhost();
+ const IPAddress kValidAddress2 = IPAddress::IPv6Localhost();
// Check by initializing empty address.
- IPAddress initial_address;
- impl_.OnSupportsQuicLoadedForTesting(initial_address);
-
- IPAddress address;
- EXPECT_FALSE(impl_.GetSupportsQuic(&address));
- EXPECT_TRUE(address.empty());
+ impl_.OnLastLocalAddressWhenQuicWorkedForTesting(kEmptyAddress);
+ EXPECT_FALSE(impl_.HasLastLocalAddressWhenQuicWorked());
+ // Empty address should not be considered an address that was used when QUIC
+ // worked.
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
// Check by initializing with a valid address.
- initial_address = IPAddress::IPv4Localhost();
- impl_.OnSupportsQuicLoadedForTesting(initial_address);
+ impl_.OnLastLocalAddressWhenQuicWorkedForTesting(kValidAddress1);
+ EXPECT_TRUE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_TRUE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
- EXPECT_TRUE(impl_.GetSupportsQuic(&address));
- EXPECT_EQ(initial_address, address);
+ // Try another valid address.
+ impl_.OnLastLocalAddressWhenQuicWorkedForTesting(kValidAddress2);
+ EXPECT_TRUE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_TRUE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
+
+ // And loading an empty address clears the current one.
+ // TODO(mmenke): This seems like a bug, since if we've learned the current
+ // network supports QUIC, surely we want to save that to disk? Seems like a
+ // pre-existing value should take precedence, if non-empty, since if the
+ // current network is already known to support QUIC, the loaded value is no
+ // longer relevant.
+ impl_.OnLastLocalAddressWhenQuicWorkedForTesting(kEmptyAddress);
+ EXPECT_FALSE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
}
-TEST_F(SupportsQuicServerPropertiesTest, SetSupportsQuic) {
- IPAddress address;
- EXPECT_FALSE(impl_.GetSupportsQuic(&address));
- EXPECT_TRUE(address.empty());
+TEST_F(HttpServerPropertiesTest, SetLastLocalAddressWhenQuicWorked) {
+ const IPAddress kEmptyAddress;
+ const IPAddress kValidAddress1 = IPAddress::IPv4Localhost();
+ const IPAddress kValidAddress2 = IPAddress::IPv6Localhost();
- IPAddress actual_address(127, 0, 0, 1);
- impl_.SetSupportsQuic(true, actual_address);
+ EXPECT_FALSE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
- EXPECT_TRUE(impl_.GetSupportsQuic(&address));
- EXPECT_EQ(actual_address, address);
+ // Set to a valid address.
+ impl_.SetLastLocalAddressWhenQuicWorked(kValidAddress1);
+ EXPECT_TRUE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_TRUE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
+ // Clear only this value.
+ impl_.ClearLastLocalAddressWhenQuicWorked();
+ EXPECT_FALSE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
+
+ // Try another valid address.
+ impl_.SetLastLocalAddressWhenQuicWorked(kValidAddress2);
+ EXPECT_TRUE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_TRUE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
+
+ // Clear all values.
impl_.Clear(base::OnceClosure());
-
- EXPECT_FALSE(impl_.GetSupportsQuic(&address));
+ EXPECT_FALSE(impl_.HasLastLocalAddressWhenQuicWorked());
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kEmptyAddress));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress1));
+ EXPECT_FALSE(impl_.WasLastLocalAddressWhenQuicWorked(kValidAddress2));
}
TEST_F(HttpServerPropertiesTest, LoadServerNetworkStats) {
diff --git a/net/http/http_stream_factory_job_controller_unittest.cc b/net/http/http_stream_factory_job_controller_unittest.cc
index e03077f..7c87cf8 100644
--- a/net/http/http_stream_factory_job_controller_unittest.cc
+++ b/net/http/http_stream_factory_job_controller_unittest.cc
@@ -714,7 +714,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromSeconds(100);
session_->http_server_properties()->SetServerNetworkStats(
@@ -1716,7 +1716,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromMicroseconds(10);
session_->http_server_properties()->SetServerNetworkStats(
@@ -1790,7 +1790,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromMicroseconds(10);
session_->http_server_properties()->SetServerNetworkStats(
@@ -1854,7 +1854,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromMicroseconds(10);
session_->http_server_properties()->SetServerNetworkStats(
@@ -1933,7 +1933,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromSeconds(100);
session_->http_server_properties()->SetServerNetworkStats(
@@ -1994,7 +1994,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromMicroseconds(10);
session_->http_server_properties()->SetServerNetworkStats(
@@ -2117,7 +2117,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromMicroseconds(10);
session_->http_server_properties()->SetServerNetworkStats(
@@ -2188,7 +2188,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromMicroseconds(300 * 1000);
session_->http_server_properties()->SetServerNetworkStats(
@@ -2241,7 +2241,7 @@
// Enable delayed TCP and set time delay for waiting job.
QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
- quic_stream_factory->set_require_confirmation(false);
+ quic_stream_factory->set_is_quic_known_to_work_on_current_network(true);
ServerNetworkStats stats1;
stats1.srtt = base::TimeDelta::FromMicroseconds(300 * 1000);
session_->http_server_properties()->SetServerNetworkStats(
diff --git a/net/http/http_stream_factory_unittest.cc b/net/http/http_stream_factory_unittest.cc
index e4226c3..bc6158b 100644
--- a/net/http/http_stream_factory_unittest.cc
+++ b/net/http/http_stream_factory_unittest.cc
@@ -809,7 +809,8 @@
auto session =
std::make_unique<HttpNetworkSession>(session_params, session_context);
- session->quic_stream_factory()->set_require_confirmation(false);
+ session->quic_stream_factory()
+ ->set_is_quic_known_to_work_on_current_network(true);
StaticSocketDataProvider socket_data1;
socket_data1.set_connect_data(
@@ -2370,7 +2371,8 @@
session_context.ssl_config_service = ssl_config_service_.get();
session_context.client_socket_factory = &socket_factory_;
session_.reset(new HttpNetworkSession(params_, session_context));
- session_->quic_stream_factory()->set_require_confirmation(false);
+ session_->quic_stream_factory()
+ ->set_is_quic_known_to_work_on_current_network(true);
}
void AddQuicAlternativeService() {
diff --git a/net/quic/quic_chromium_client_session.cc b/net/quic/quic_chromium_client_session.cc
index 892aab1..3a66bcf 100644
--- a/net/quic/quic_chromium_client_session.cc
+++ b/net/quic/quic_chromium_client_session.cc
@@ -1527,7 +1527,7 @@
}
if (event == HANDSHAKE_CONFIRMED) {
if (stream_factory_)
- stream_factory_->set_require_confirmation(false);
+ stream_factory_->set_is_quic_known_to_work_on_current_network(true);
// Update |connect_end| only when handshake is confirmed. This should also
// take care of any failed 0-RTT request.
diff --git a/net/quic/quic_chromium_client_session.h b/net/quic/quic_chromium_client_session.h
index 4854c7f8..0ac279b 100644
--- a/net/quic/quic_chromium_client_session.h
+++ b/net/quic/quic_chromium_client_session.h
@@ -368,6 +368,12 @@
// Constructs a new session which will own |connection|, but not
// |stream_factory|, which must outlive this session.
// TODO(rch): decouple the factory from the session via a Delegate interface.
+ //
+ // If |require_confirmation| is true, the returned session will wait for a
+ // successful QUIC handshake before vending any streams, to ensure that both
+ // the server and the current network support QUIC, as HTTP fallback can't
+ // trigger (or at least will take longer) after a QUIC stream has successfully
+ // been created.
QuicChromiumClientSession(
quic::QuicConnection* connection,
std::unique_ptr<DatagramClientSocket> socket,
diff --git a/net/quic/quic_network_transaction_unittest.cc b/net/quic/quic_network_transaction_unittest.cc
index c435912..3de0d1c 100644
--- a/net/quic/quic_network_transaction_unittest.cc
+++ b/net/quic/quic_network_transaction_unittest.cc
@@ -671,7 +671,8 @@
session_context_.net_log = net_log_.bound().net_log();
session_.reset(new HttpNetworkSession(session_params_, session_context_));
- session_->quic_stream_factory()->set_require_confirmation(false);
+ session_->quic_stream_factory()
+ ->set_is_quic_known_to_work_on_current_network(true);
SpdySessionPoolPeer spdy_pool_peer(session_->spdy_session_pool());
spdy_pool_peer.SetEnableSendingInitialData(false);
}
@@ -2518,7 +2519,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());
@@ -2601,7 +2603,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
// Use a TestTaskRunner to avoid waiting in real time for timeouts.
QuicStreamFactoryPeer::SetAlarmFactory(
session_->quic_stream_factory(),
@@ -2715,7 +2718,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
// Use a TestTaskRunner to avoid waiting in real time for timeouts.
QuicStreamFactoryPeer::SetAlarmFactory(
session_->quic_stream_factory(),
@@ -2852,7 +2856,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
// Use a TestTaskRunner to avoid waiting in real time for timeouts.
QuicStreamFactoryPeer::SetAlarmFactory(
session_->quic_stream_factory(),
@@ -2989,7 +2994,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
// Use a TestTaskRunner to avoid waiting in real time for timeouts.
QuicStreamFactoryPeer::SetAlarmFactory(
session_->quic_stream_factory(),
@@ -4961,7 +4967,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());
@@ -5172,7 +5179,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());
@@ -5235,7 +5243,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());
@@ -5293,7 +5302,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());
@@ -5354,7 +5364,8 @@
"");
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());
@@ -5510,7 +5521,8 @@
// Set QUIC support on the last IP address, which is same with the local IP
// address. Require confirmation mode will be turned off immediately when
// local IP address is sorted out after we configure the UDP socket.
- http_server_properties_->SetSupportsQuic(true, IPAddress(192, 0, 2, 33));
+ http_server_properties_->SetLastLocalAddressWhenQuicWorked(
+ IPAddress(192, 0, 2, 33));
MockQuicData mock_quic_data(version_);
client_maker_.SetEncryptionLevel(quic::ENCRYPTION_ZERO_RTT);
@@ -5536,7 +5548,8 @@
CreateSession();
// QuicStreamFactory by default requires confirmation on construction.
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
@@ -5567,7 +5580,8 @@
// Set QUIC support on the last IP address, which is different with the local
// IP address. Require confirmation mode will remain when local IP address is
// sorted out after we configure the UDP socket.
- http_server_properties_->SetSupportsQuic(true, IPAddress(1, 2, 3, 4));
+ http_server_properties_->SetLastLocalAddressWhenQuicWorked(
+ IPAddress(1, 2, 3, 4));
MockQuicData mock_quic_data(version_);
int packet_num = 1;
@@ -5596,7 +5610,8 @@
// No HTTP data is mocked as TCP job will be delayed and never starts.
CreateSession();
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
// Stall host resolution so that QUIC job could not proceed and unblocks TCP.
@@ -5647,7 +5662,8 @@
// Require handshake confirmation to ensure that no QUIC streams are
// created, and to ensure that the TCP job does not wait for the QUIC
// job to fail before it starts.
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()->set_is_quic_known_to_work_on_current_network(
+ false);
AddQuicAlternateProtocolMapping(MockCryptoClientStream::COLD_START);
HttpNetworkTransaction trans(DEFAULT_PRIORITY, session_.get());
@@ -6892,7 +6908,8 @@
session_context.http_server_properties = &http_server_properties_;
session_.reset(new HttpNetworkSession(session_params, session_context));
- session_->quic_stream_factory()->set_require_confirmation(true);
+ session_->quic_stream_factory()
+ ->set_is_quic_known_to_work_on_current_network(false);
}
void TearDown() override {
diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc
index 41e2b3f..bef5d34 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -1079,7 +1079,7 @@
quic::QuicRandom* random_generator,
quic::QuicClock* clock,
const QuicParams& params)
- : require_confirmation_(true),
+ : is_quic_known_to_work_on_current_network_(false),
net_log_(net_log),
host_resolver_(host_resolver),
client_socket_factory_(client_socket_factory),
@@ -1232,21 +1232,37 @@
}
}
-void QuicStreamFactory::set_require_confirmation(bool require_confirmation) {
- require_confirmation_ = require_confirmation;
+void QuicStreamFactory::set_is_quic_known_to_work_on_current_network(
+ bool is_quic_known_to_work_on_current_network) {
+ is_quic_known_to_work_on_current_network_ =
+ is_quic_known_to_work_on_current_network;
if (!(local_address_ == IPEndPoint())) {
- http_server_properties_->SetSupportsQuic(!require_confirmation,
- local_address_.address());
+ if (is_quic_known_to_work_on_current_network_) {
+ http_server_properties_->SetLastLocalAddressWhenQuicWorked(
+ local_address_.address());
+ } else {
+ http_server_properties_->ClearLastLocalAddressWhenQuicWorked();
+ }
}
}
base::TimeDelta QuicStreamFactory::GetTimeDelayForWaitingJob(
const quic::QuicServerId& server_id,
const NetworkIsolationKey& network_isolation_key) {
- if (require_confirmation_) {
- IPAddress last_address;
+ // If |is_quic_known_to_work_on_current_network_| is false, then one of the
+ // following is true:
+ // 1) This is startup and QuicStreamFactory::CreateSession() and
+ // ConfigureSocket() have yet to be called, and it is not yet known
+ // if the current network is the last one where QUIC worked.
+ // 2) Startup has been completed, and QUIC has not been used
+ // successfully since startup, or on this network before.
+ if (!is_quic_known_to_work_on_current_network_) {
+ // If |need_to_check_persisted_supports_quic_| is false, this is case 1)
+ // above. If HasLastLocalAddressWhenQuicWorked() is also true, then there's
+ // a chance the current network is the last one on which QUIC worked. So
+ // only delay the request if there's no chance that is the case.
if (!need_to_check_persisted_supports_quic_ ||
- !http_server_properties_->GetSupportsQuic(&last_address)) {
+ !http_server_properties_->HasLastLocalAddressWhenQuicWorked()) {
return base::TimeDelta();
}
}
@@ -1486,7 +1502,7 @@
auto iter = active_jobs_.find(job->key().session_key());
DCHECK(iter != active_jobs_.end());
if (rv == OK) {
- set_require_confirmation(false);
+ set_is_quic_known_to_work_on_current_network(true);
auto session_it = active_sessions_.find(job->key().session_key());
CHECK(session_it != active_sessions_.end());
@@ -1627,7 +1643,7 @@
if (params_.migrate_sessions_on_network_change_v2)
return;
- set_require_confirmation(true);
+ set_is_quic_known_to_work_on_current_network(false);
if (params_.close_sessions_on_ip_change) {
CloseAllSessions(ERR_NETWORK_CHANGED, quic::QUIC_IP_ADDRESS_CHANGED);
} else {
@@ -1677,7 +1693,7 @@
++it;
session->OnNetworkMadeDefault(network, scoped_event_log.net_log());
}
- set_require_confirmation(true);
+ set_is_quic_known_to_work_on_current_network(false);
}
void QuicStreamFactory::OnNetworkDisconnected(NetworkHandle network) {
@@ -1804,14 +1820,13 @@
socket->GetLocalAddress(&local_address_);
if (need_to_check_persisted_supports_quic_) {
need_to_check_persisted_supports_quic_ = false;
- IPAddress last_address;
- if (http_server_properties_->GetSupportsQuic(&last_address) &&
- last_address == local_address_.address()) {
- require_confirmation_ = false;
+ if (http_server_properties_->WasLastLocalAddressWhenQuicWorked(
+ local_address_.address())) {
+ is_quic_known_to_work_on_current_network_ = true;
// Clear the persisted IP address, in case the network no longer supports
// QUIC so the next restart will require confirmation. It will be
// re-persisted when the first job completes successfully.
- http_server_properties_->SetSupportsQuic(false, last_address);
+ http_server_properties_->ClearLastLocalAddressWhenQuicWorked();
}
}
@@ -1910,7 +1925,7 @@
// Wait for handshake confirmation before allowing streams to be created if
// either this session or the factory require confirmation.
- if (require_confirmation_)
+ if (!is_quic_known_to_work_on_current_network_)
require_confirmation = true;
*session = new QuicChromiumClientSession(
diff --git a/net/quic/quic_stream_factory.h b/net/quic/quic_stream_factory.h
index f46d1673..08ec094 100644
--- a/net/quic/quic_stream_factory.h
+++ b/net/quic/quic_stream_factory.h
@@ -458,11 +458,14 @@
// We close all sessions when certificate database is changed.
void OnCertDBChanged() override;
- bool require_confirmation() const { return require_confirmation_; }
+ bool is_quic_known_to_work_on_current_network() const {
+ return is_quic_known_to_work_on_current_network_;
+ }
bool allow_server_migration() const { return params_.allow_server_migration; }
- void set_require_confirmation(bool require_confirmation);
+ void set_is_quic_known_to_work_on_current_network(
+ bool is_quic_known_to_work_on_current_network);
// It returns the amount of time waiting job should be delayed.
base::TimeDelta GetTimeDelayForWaitingJob(
@@ -577,7 +580,12 @@
const quic::QuicServerId& server_id,
bool was_session_active);
- bool require_confirmation_;
+ // Whether QUIC is known to work on current network. This is true when QUIC is
+ // expected to work in general, rather than whether QUIC was broken / recently
+ // broken when used with a particular server. That information is stored in
+ // the broken alternative service map in HttpServerProperties.
+ bool is_quic_known_to_work_on_current_network_;
+
NetLog* net_log_;
HostResolver* host_resolver_;
ClientSocketFactory* client_socket_factory_;
diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc
index 70aa92e..fc32c2f 100644
--- a/net/quic/quic_stream_factory_test.cc
+++ b/net/quic/quic_stream_factory_test.cc
@@ -561,7 +561,7 @@
test_params_.quic_params.idle_connection_timeout =
base::TimeDelta::FromSeconds(500);
Initialize();
- factory_->set_require_confirmation(false);
+ factory_->set_is_quic_known_to_work_on_current_network(true);
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
@@ -942,7 +942,7 @@
TEST_P(QuicStreamFactoryTest, CreateZeroRtt) {
Initialize();
- factory_->set_require_confirmation(false);
+ factory_->set_is_quic_known_to_work_on_current_network(true);
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
@@ -1031,7 +1031,7 @@
host_resolver_->rules()->AddIPLiteralRule(host_port_pair_.host(),
"192.168.0.1", "");
Initialize();
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
@@ -1049,13 +1049,12 @@
/*cert_verify_flags=*/0, url_, net_log_, &net_error_details_,
failed_on_default_network_callback_, callback_.callback()));
- IPAddress last_address;
- EXPECT_FALSE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_FALSE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
crypto_client_stream_factory_.last_stream()->SendOnCryptoHandshakeEvent(
quic::QuicSession::HANDSHAKE_CONFIRMED);
- EXPECT_TRUE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_TRUE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
EXPECT_THAT(callback_.WaitForResult(), IsOk());
std::unique_ptr<HttpStream> stream = CreateStream(&request);
@@ -1072,8 +1071,9 @@
host_resolver_->rules()->AddIPLiteralRule(host_port_pair_.host(),
"192.168.0.1", "");
Initialize();
- factory_->set_require_confirmation(true);
- http_server_properties_->SetSupportsQuic(true, IPAddress(192, 0, 2, 33));
+ factory_->set_is_quic_known_to_work_on_current_network(false);
+ http_server_properties_->SetLastLocalAddressWhenQuicWorked(
+ IPAddress(192, 0, 2, 33));
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
@@ -1092,8 +1092,7 @@
failed_on_default_network_callback_, callback_.callback()),
IsOk());
- IPAddress last_address;
- EXPECT_FALSE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_FALSE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
std::unique_ptr<HttpStream> stream = CreateStream(&request);
EXPECT_TRUE(stream.get());
@@ -1104,7 +1103,7 @@
crypto_client_stream_factory_.last_stream()->SendOnCryptoHandshakeEvent(
quic::QuicSession::HANDSHAKE_CONFIRMED);
- EXPECT_TRUE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_TRUE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
}
TEST_P(QuicStreamFactoryTest, CachedInitialRtt) {
@@ -2302,15 +2301,14 @@
QuicChromiumClientSession* session = GetActiveSession(host_port_pair_);
EXPECT_TRUE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
- IPAddress last_address;
- EXPECT_TRUE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_TRUE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
// Change the IP address and verify that stream saw the error and the active
// session is closed.
NotifyIPAddressChanged();
EXPECT_EQ(ERR_NETWORK_CHANGED,
stream->ReadResponseHeaders(callback_.callback()));
- EXPECT_TRUE(factory_->require_confirmation());
- EXPECT_FALSE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_FALSE(factory_->is_quic_known_to_work_on_current_network());
+ EXPECT_FALSE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
// Check no active session exists for the destination.
EXPECT_FALSE(HasActiveSession(host_port_pair_));
@@ -2483,13 +2481,12 @@
EXPECT_EQ(OK, stream->InitializeStream(&request_info, false, DEFAULT_PRIORITY,
net_log_, CompletionOnceCallback()));
- IPAddress last_address;
- EXPECT_TRUE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_TRUE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
// Change the IP address and verify that the connection is unaffected.
NotifyIPAddressChanged();
- EXPECT_FALSE(factory_->require_confirmation());
- EXPECT_TRUE(http_server_properties_->GetSupportsQuic(&last_address));
+ EXPECT_TRUE(factory_->is_quic_known_to_work_on_current_network());
+ EXPECT_TRUE(http_server_properties_->HasLastLocalAddressWhenQuicWorked());
// Attempting a new request to the same origin uses the same connection.
QuicStreamRequest request2(factory_.get());
@@ -9420,7 +9417,7 @@
// Change the CA cert and verify that stream saw the event.
factory_->OnCertDBChanged();
- EXPECT_FALSE(factory_->require_confirmation());
+ EXPECT_TRUE(factory_->is_quic_known_to_work_on_current_network());
EXPECT_TRUE(QuicStreamFactoryPeer::IsLiveSession(factory_.get(), session));
EXPECT_FALSE(HasActiveSession(host_port_pair_));
@@ -9531,7 +9528,7 @@
TEST_P(QuicStreamFactoryTest, EnableNotLoadFromDiskCache) {
Initialize();
- factory_->set_require_confirmation(false);
+ factory_->set_is_quic_known_to_work_on_current_network(true);
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
@@ -9739,7 +9736,7 @@
TEST_P(QuicStreamFactoryTest, YieldAfterPackets) {
Initialize();
- factory_->set_require_confirmation(false);
+ factory_->set_is_quic_known_to_work_on_current_network(true);
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
QuicStreamFactoryPeer::SetYieldAfterPackets(factory_.get(), 0);
@@ -9787,7 +9784,7 @@
TEST_P(QuicStreamFactoryTest, YieldAfterDuration) {
Initialize();
- factory_->set_require_confirmation(false);
+ factory_->set_is_quic_known_to_work_on_current_network(true);
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
QuicStreamFactoryPeer::SetYieldAfterDuration(
@@ -10561,7 +10558,7 @@
host_resolver_->set_ondemand_mode(true);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
MockQuicData socket_data(version_);
socket_data.AddRead(ASYNC, ERR_IO_PENDING); // Pause
@@ -10650,7 +10647,7 @@
host_resolver_->set_synchronous_mode(true);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
MockQuicData socket_data(version_);
socket_data.AddRead(ASYNC, ERR_IO_PENDING); // Pause
@@ -10907,7 +10904,7 @@
// Set an address in resolver for asynchronous return.
host_resolver_->set_ondemand_mode(true);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
host_resolver_->rules()->AddIPLiteralRule(host_port_pair_.host(),
@@ -10975,7 +10972,7 @@
// Set an address in resolver for asynchronous return.
host_resolver_->set_ondemand_mode(true);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
host_resolver_->rules()->AddIPLiteralRule(host_port_pair_.host(),
@@ -11115,7 +11112,7 @@
// Set an address in resolver for asynchronous return.
host_resolver_->set_ondemand_mode(true);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
host_resolver_->rules()->AddIPLiteralRule(host_port_pair_.host(),
@@ -11192,7 +11189,7 @@
// Set an address in resolver for asynchronous return.
host_resolver_->set_ondemand_mode(true);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
host_resolver_->rules()->AddIPLiteralRule(host_port_pair_.host(),
@@ -11543,7 +11540,7 @@
// Add asynchronous failure in host resolver.
host_resolver_->set_ondemand_mode(true);
host_resolver_->rules()->AddSimulatedFailure(host_port_pair_.host());
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
@@ -11596,7 +11593,7 @@
// Add asynchronous failure to host resolver.
host_resolver_->set_ondemand_mode(true);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
host_resolver_->rules()->AddSimulatedFailure(host_port_pair_.host());
@@ -11768,7 +11765,7 @@
// Set an address in resolver for asynchronous return.
host_resolver_->set_ondemand_mode(true);
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::ZERO_RTT);
host_resolver_->rules()->AddIPLiteralRule(host_port_pair_.host(),
@@ -11838,7 +11835,7 @@
crypto_client_stream_factory_.set_handshake_mode(
MockCryptoClientStream::COLD_START_WITH_CHLO_SENT);
Initialize();
- factory_->set_require_confirmation(true);
+ factory_->set_is_quic_known_to_work_on_current_network(false);
ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
