Add API in HttpServerProperties to persist alternative service broken…

… until the default network changes.

Bug: 790547
Change-Id: I2d784037a0ab316232ad51f2f5cfceaaf6dd8c44
Reviewed-on: https://chromium-review.googlesource.com/1192109
Commit-Queue: Zhongyi Shi <zhongyi@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586863}

net/http/broken_alternative_services.cc

@@ -157,10 +157,12 @@ void BrokenAlternativeServices::Confirm(
   broken_alternative_services_on_default_network_.erase(alternative_service);
 }
 
-void BrokenAlternativeServices::OnDefaultNetworkChanged() {
+bool BrokenAlternativeServices::OnDefaultNetworkChanged() {
+  bool changed = !broken_alternative_services_on_default_network_.empty();
   while (!broken_alternative_services_on_default_network_.empty()) {
     Confirm(*broken_alternative_services_on_default_network_.begin());
   }
+  return changed;
 }
 
 void BrokenAlternativeServices::SetBrokenAndRecentlyBrokenAlternativeServices(

net/http/broken_alternative_services.h

@@ -89,9 +89,11 @@ class NET_EXPORT_PRIVATE BrokenAlternativeServices {
   // Changes the alternative service to be considered as working.
   void Confirm(const AlternativeService& alternative_service);
 
-  // All alternative services which were marked as broken until the default
-  // network changed will now be considered working.
-  void OnDefaultNetworkChanged();
+  // Clears all alternative services which were marked as broken until the
+  // default network changed, those services will now be considered working.
+  // Returns true if there was any broken alternative service affected by this
+  // network change.
+  bool OnDefaultNetworkChanged();
 
   // Sets broken and recently broken alternative services.
   // |broken_alternative_service_list|, |recently_broken_alternative_services|

net/http/http_server_properties.h

@@ -389,6 +389,11 @@ class NET_EXPORT HttpServerProperties {
   virtual void MarkAlternativeServiceBroken(
       const AlternativeService& alternative_service) = 0;
 
+  // Marks |alternative_service| as broken until the default network changes.
+  // |alternative_service.host| must not be empty.
+  virtual void MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      const AlternativeService& alternative_service) = 0;
+
   // Marks |alternative_service| as recently broken.
   // |alternative_service.host| must not be empty.
   virtual void MarkAlternativeServiceRecentlyBroken(
@@ -409,6 +414,13 @@ class NET_EXPORT HttpServerProperties {
   virtual void ConfirmAlternativeService(
       const AlternativeService& alternative_service) = 0;
 
+  // Called when the default network changes.
+  // Clears all the alternative services that were marked broken until the
+  // default network changed.
+  // Returns true if there is any broken alternative service affected by the
+  // default network change.
+  virtual bool OnDefaultNetworkChanged() = 0;
+
   // Returns all alternative service mappings.
   // Returned alternative services may have empty hostnames.
   virtual const AlternativeServiceMap& alternative_service_map() const = 0;

net/http/http_server_properties_impl.cc

@@ -481,6 +481,13 @@ void HttpServerPropertiesImpl::MarkAlternativeServiceBroken(
   broken_alternative_services_.MarkBroken(alternative_service);
 }
 
+void HttpServerPropertiesImpl::
+    MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+        const AlternativeService& alternative_service) {
+  broken_alternative_services_.MarkBrokenUntilDefaultNetworkChanges(
+      alternative_service);
+}
+
 void HttpServerPropertiesImpl::MarkAlternativeServiceRecentlyBroken(
     const AlternativeService& alternative_service) {
   broken_alternative_services_.MarkRecentlyBroken(alternative_service);
@@ -501,6 +508,10 @@ void HttpServerPropertiesImpl::ConfirmAlternativeService(
   broken_alternative_services_.Confirm(alternative_service);
 }
 
+bool HttpServerPropertiesImpl::OnDefaultNetworkChanged() {
+  return broken_alternative_services_.OnDefaultNetworkChanged();
+}
+
 const AlternativeServiceMap& HttpServerPropertiesImpl::alternative_service_map()
     const {
   return alternative_service_map_;

net/http/http_server_properties_impl.h

@@ -115,6 +115,8 @@ class NET_EXPORT HttpServerPropertiesImpl
                                   alternative_service_info_vector) override;
   void MarkAlternativeServiceBroken(
       const AlternativeService& alternative_service) override;
+  void MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      const AlternativeService& alternative_service) override;
   void MarkAlternativeServiceRecentlyBroken(
       const AlternativeService& alternative_service) override;
   bool IsAlternativeServiceBroken(
@@ -123,6 +125,7 @@ class NET_EXPORT HttpServerPropertiesImpl
       const AlternativeService& alternative_service) override;
   void ConfirmAlternativeService(
       const AlternativeService& alternative_service) override;
+  bool OnDefaultNetworkChanged() override;
   const AlternativeServiceMap& alternative_service_map() const override;
   std::unique_ptr<base::Value> GetAlternativeServiceInfoAsValue()
       const override;

net/http/http_server_properties_impl_unittest.cc

@@ -671,6 +671,60 @@ TEST_F(AlternateProtocolServerPropertiesTest, SetBroken) {
   EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
 }
 
+TEST_F(AlternateProtocolServerPropertiesTest,
+       SetBrokenUntilDefaultNetworkChanges) {
+  url::SchemeHostPort test_server("http", "foo", 80);
+  const AlternativeService alternative_service1(kProtoHTTP2, "foo", 443);
+  SetAlternativeService(test_server, alternative_service1);
+  AlternativeServiceInfoVector alternative_service_info_vector =
+      impl_.GetAlternativeServiceInfos(test_server);
+  ASSERT_EQ(1u, alternative_service_info_vector.size());
+  EXPECT_EQ(alternative_service1,
+            alternative_service_info_vector[0].alternative_service());
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service1));
+
+  // Mark the alternative service as broken until the default network changes.
+  impl_.MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      alternative_service1);
+  // The alternative service should be persisted and marked as broken.
+  alternative_service_info_vector =
+      impl_.GetAlternativeServiceInfos(test_server);
+  ASSERT_EQ(1u, alternative_service_info_vector.size());
+  EXPECT_EQ(alternative_service1,
+            alternative_service_info_vector[0].alternative_service());
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
+
+  // SetAlternativeServices should add a broken alternative service to the map.
+  AlternativeServiceInfoVector alternative_service_info_vector2;
+  base::Time expiration = test_clock_.Now() + base::TimeDelta::FromDays(1);
+  alternative_service_info_vector2.push_back(
+      AlternativeServiceInfo::CreateHttp2AlternativeServiceInfo(
+          alternative_service1, expiration));
+  const AlternativeService alternative_service2(kProtoHTTP2, "foo", 1234);
+  alternative_service_info_vector2.push_back(
+      AlternativeServiceInfo::CreateHttp2AlternativeServiceInfo(
+          alternative_service2, expiration));
+  impl_.SetAlternativeServices(test_server, alternative_service_info_vector2);
+  alternative_service_info_vector =
+      impl_.GetAlternativeServiceInfos(test_server);
+  ASSERT_EQ(2u, alternative_service_info_vector.size());
+  EXPECT_EQ(alternative_service1,
+            alternative_service_info_vector[0].alternative_service());
+  EXPECT_EQ(alternative_service2,
+            alternative_service_info_vector[1].alternative_service());
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service2));
+
+  // SetAlternativeService should add a broken alternative service to the map.
+  SetAlternativeService(test_server, alternative_service1);
+  alternative_service_info_vector =
+      impl_.GetAlternativeServiceInfos(test_server);
+  ASSERT_EQ(1u, alternative_service_info_vector.size());
+  EXPECT_EQ(alternative_service1,
+            alternative_service_info_vector[0].alternative_service());
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
+}
+
 TEST_F(AlternateProtocolServerPropertiesTest, MaxAge) {
   AlternativeServiceInfoVector alternative_service_info_vector;
   base::Time now = test_clock_.Now();
@@ -853,6 +907,71 @@ TEST_F(AlternateProtocolServerPropertiesTest, MarkRecentlyBroken) {
   EXPECT_FALSE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
 }
 
+TEST_F(AlternateProtocolServerPropertiesTest,
+       MarkBrokenUntilDefaultNetworkChanges) {
+  url::SchemeHostPort server("http", "foo", 80);
+  const AlternativeService alternative_service(kProtoHTTP2, "foo", 443);
+  SetAlternativeService(server, alternative_service);
+
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  impl_.MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      alternative_service);
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  impl_.ConfirmAlternativeService(alternative_service);
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+}
+
+TEST_F(AlternateProtocolServerPropertiesTest, OnDefaultNetworkChanged) {
+  url::SchemeHostPort server("http", "foo", 80);
+  const AlternativeService alternative_service(kProtoHTTP2, "foo", 443);
+
+  SetAlternativeService(server, alternative_service);
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  impl_.MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      alternative_service);
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  // Default network change clears alt svc broken until default network changes.
+  EXPECT_TRUE(impl_.OnDefaultNetworkChanged());
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  impl_.MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      alternative_service);
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  impl_.MarkAlternativeServiceBroken(alternative_service);
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  // Default network change doesn't affect alt svc that was simply marked broken
+  // most recently.
+  EXPECT_FALSE(impl_.OnDefaultNetworkChanged());
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  impl_.MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      alternative_service);
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+
+  // Default network change clears alt svc that was marked broken until default
+  // network change most recently even if the alt svc was initially marked
+  // broken.
+  EXPECT_TRUE(impl_.OnDefaultNetworkChanged());
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(impl_.WasAlternativeServiceRecentlyBroken(alternative_service));
+}
+
 TEST_F(AlternateProtocolServerPropertiesTest, Canonical) {
   url::SchemeHostPort test_server("https", "foo.c.youtube.com", 443);
   EXPECT_FALSE(HasAlternativeService(test_server));
@@ -918,10 +1037,25 @@ TEST_F(AlternateProtocolServerPropertiesTest, CanonicalBroken) {
                                                    "bar.c.youtube.com", 1234);
 
   SetAlternativeService(canonical_server, canonical_alternative_service);
+  EXPECT_TRUE(HasAlternativeService(test_server));
   impl_.MarkAlternativeServiceBroken(canonical_alternative_service);
   EXPECT_FALSE(HasAlternativeService(test_server));
 }
 
+TEST_F(AlternateProtocolServerPropertiesTest,
+       CanonicalBrokenUntilDefaultNetworkChanges) {
+  url::SchemeHostPort test_server("https", "foo.c.youtube.com", 443);
+  url::SchemeHostPort canonical_server("https", "bar.c.youtube.com", 443);
+  AlternativeService canonical_alternative_service(kProtoQUIC,
+                                                   "bar.c.youtube.com", 1234);
+
+  SetAlternativeService(canonical_server, canonical_alternative_service);
+  EXPECT_TRUE(HasAlternativeService(test_server));
+  impl_.MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      canonical_alternative_service);
+  EXPECT_FALSE(HasAlternativeService(test_server));
+}
+
 // Adding an alternative service for a new host overrides canonical host.
 TEST_F(AlternateProtocolServerPropertiesTest, CanonicalOverride) {
   url::SchemeHostPort foo_server("https", "foo.c.youtube.com", 443);
@@ -1100,12 +1234,21 @@ TEST_F(AlternateProtocolServerPropertiesTest,
           AlternativeService(kProtoQUIC, "bar", 443),
           now + base::TimeDelta::FromHours(1),
           HttpNetworkSession::Params().quic_supported_versions));
+  alternative_service_info_vector.push_back(
+      AlternativeServiceInfo::CreateQuicAlternativeServiceInfo(
+          AlternativeService(kProtoQUIC, "baz", 443),
+          now + base::TimeDelta::FromHours(1),
+          HttpNetworkSession::Params().quic_supported_versions));
+
   impl_.SetAlternativeServices(url::SchemeHostPort("https", "youtube.com", 443),
                                alternative_service_info_vector);
 
   impl_.MarkAlternativeServiceBroken(
       AlternativeService(kProtoQUIC, "bar", 443));
 
+  impl_.MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      AlternativeService(kProtoQUIC, "baz", 443));
+
   alternative_service_info_vector.clear();
   alternative_service_info_vector.push_back(
       AlternativeServiceInfo::CreateHttp2AlternativeServiceInfo(
@@ -1125,6 +1268,8 @@ TEST_F(AlternateProtocolServerPropertiesTest,
       "\"alternative_service\":"
       "[\"h2 foo:443, expires 2018-01-24 15:13:53\","
       "\"quic bar:443, expires 2018-01-24 16:12:53"
+      " (broken until 2018-01-24 15:17:53)\","
+      "\"quic baz:443, expires 2018-01-24 16:12:53"
       " (broken until 2018-01-24 15:17:53)\"],"
       "\"server\":\"https://youtube.com\""
       "}"

net/http/http_server_properties_manager.cc

@@ -256,6 +256,17 @@ void HttpServerPropertiesManager::MarkAlternativeServiceBroken(
   ScheduleUpdatePrefs(MARK_ALTERNATIVE_SERVICE_BROKEN);
 }
 
+void HttpServerPropertiesManager::
+    MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+        const AlternativeService& alternative_service) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  http_server_properties_impl_
+      ->MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+          alternative_service);
+  ScheduleUpdatePrefs(
+      MARK_ALTERNATIVE_SERVICE_BROKEN_UNTIL_DEFAULT_NETWORK_CHANGES);
+}
+
 void HttpServerPropertiesManager::MarkAlternativeServiceRecentlyBroken(
     const AlternativeService& alternative_service) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -292,6 +303,14 @@ void HttpServerPropertiesManager::ConfirmAlternativeService(
     ScheduleUpdatePrefs(CONFIRM_ALTERNATIVE_SERVICE);
 }
 
+bool HttpServerPropertiesManager::OnDefaultNetworkChanged() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  bool changed = http_server_properties_impl_->OnDefaultNetworkChanged();
+  if (changed)
+    ScheduleUpdatePrefs(ON_DEFAULT_NETWORK_CHANGED);
+  return changed;
+}
+
 const AlternativeServiceMap&
 HttpServerPropertiesManager::alternative_service_map() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

net/http/http_server_properties_manager.h

@@ -112,6 +112,8 @@ class NET_EXPORT HttpServerPropertiesManager : public HttpServerProperties {
                                   alternative_service_info_vector) override;
   void MarkAlternativeServiceBroken(
       const AlternativeService& alternative_service) override;
+  void MarkAlternativeServiceBrokenUntilDefaultNetworkChanges(
+      const AlternativeService& alternative_service) override;
   void MarkAlternativeServiceRecentlyBroken(
       const AlternativeService& alternative_service) override;
   bool IsAlternativeServiceBroken(
@@ -120,6 +122,7 @@ class NET_EXPORT HttpServerPropertiesManager : public HttpServerProperties {
       const AlternativeService& alternative_service) override;
   void ConfirmAlternativeService(
       const AlternativeService& alternative_service) override;
+  bool OnDefaultNetworkChanged() override;
   const AlternativeServiceMap& alternative_service_map() const override;
   std::unique_ptr<base::Value> GetAlternativeServiceInfoAsValue()
       const override;
@@ -166,7 +169,9 @@ class NET_EXPORT HttpServerPropertiesManager : public HttpServerProperties {
     DETECTED_CORRUPTED_PREFS = 12,
     SET_QUIC_SERVER_INFO = 13,
     CLEAR_SERVER_NETWORK_STATS = 14,
-    NUM_LOCATIONS = 15,
+    MARK_ALTERNATIVE_SERVICE_BROKEN_UNTIL_DEFAULT_NETWORK_CHANGES = 15,
+    ON_DEFAULT_NETWORK_CHANGED = 16,
+    NUM_LOCATIONS = 17,
   };
 
   // --------------------