Fix and refactor HttpServerPropertiesImpl's alternative services brokenness expiration behavior

net/http/http_server_properties_impl_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_server_properties_impl.h"
 
 #include <memory>
 #include <string>
 #include <vector>
 
 #include "base/logging.h"
+#include "base/test/test_mock_time_task_runner.h"
 #include "base/values.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_address.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 namespace base {
 class ListValue;
 }
 
 namespace net {
 
 class HttpServerPropertiesImplPeer {
  public:
   static void AddBrokenAlternativeServiceWithExpirationTime(
       HttpServerPropertiesImpl& impl,
-      AlternativeService alternative_service,
+      const AlternativeService& alternative_service,
       base::TimeTicks when) {
-    impl.broken_alternative_services_.insert(
-        std::make_pair(alternative_service, when));
+    HttpBrokenAlternativeServicesManager::BrokenAlternativeServiceList::iterator
+        unused_it;
+    impl.broken_alternative_services_.AddToBrokenAlternativeServiceListAndMap(
+        alternative_service, when, &unused_it);
     auto it =
-        impl.recently_broken_alternative_services_.Get(alternative_service);
-    if (it == impl.recently_broken_alternative_services_.end()) {
-      impl.recently_broken_alternative_services_.Put(alternative_service, 1);
+        impl.broken_alternative_services_.recently_broken_alternative_services_
+            .Get(alternative_service);
+    if (it == impl.broken_alternative_services_
+                  .recently_broken_alternative_services_.end()) {
+      impl.broken_alternative_services_.recently_broken_alternative_services_
+          .Put(alternative_service, 1);
     } else {
       it->second++;
     }
   }
 
   static void ExpireBrokenAlternateProtocolMappings(
       HttpServerPropertiesImpl& impl) {
-    impl.ExpireBrokenAlternateProtocolMappings();
+    impl.broken_alternative_services_.ExpireBrokenAlternateProtocolMappings();
   }
 };
 
 namespace {
 
 const int kMaxSupportsSpdyServerHosts = 500;
 
+class TestClock : public HttpBrokenAlternativeServicesManager::Clock {
+ public:
+  TestClock() : now_(base::TimeTicks::Now()) {}
+  ~TestClock() override {}
+
+  base::TimeTicks Now() const override { return now_; }
+
+  void SetNow(base::TimeTicks now) { now_ = now; }
+  void AdvanceNow(base::TimeDelta delta) { now_ += delta; }
+
+ private:
+  base::TimeTicks now_;
+};
+
 class HttpServerPropertiesImplTest : public testing::Test {
  protected:
+  HttpServerPropertiesImplTest() : impl_(&test_clock_) {}
+
   bool HasAlternativeService(const url::SchemeHostPort& origin) {
     const AlternativeServiceVector alternative_service_vector =
         impl_.GetAlternativeServices(origin);
     return !alternative_service_vector.empty();
   }
 
   bool SetAlternativeService(const url::SchemeHostPort& origin,
                              const AlternativeService& alternative_service) {
     const base::Time expiration =
         base::Time::Now() + base::TimeDelta::FromDays(1);
     return impl_.SetAlternativeService(origin, alternative_service, expiration);
   }
 
+  void MarkBrokenAndLetExpireAlternativeServiceNTimes(
+      const AlternativeService& alternative_service,
+      int num_times) {}
+
+  TestClock test_clock_;
   HttpServerPropertiesImpl impl_;
 };
 
 typedef HttpServerPropertiesImplTest SpdyServerPropertiesTest;
 
 TEST_F(SpdyServerPropertiesTest, SetWithSchemeHostPort) {
   // Check spdy servers are correctly set with SchemeHostPort key.
   url::SchemeHostPort https_www_server("https", "www.google.com", 443);
   url::SchemeHostPort http_photo_server("http", "photos.google.com", 80);
   // Servers with port equal to default port in scheme will drop port components
@@ skipping 900 matching lines @@
   EXPECT_FALSE(HasAlternativeService(bar_server1));
   // The alternative service of "bar:443" should be unaffected.
   EXPECT_TRUE(HasAlternativeService(bar_server2));
 
   EXPECT_TRUE(
       impl_.WasAlternativeServiceRecentlyBroken(bar_alternative_service));
   EXPECT_FALSE(
       impl_.WasAlternativeServiceRecentlyBroken(baz_alternative_service));
 }
 
+// Regression test for https://crbug.com/724302
+TEST_F(AlternateProtocolServerPropertiesTest, RemoveExpiredBrokenAltSvc2) {
+  scoped_refptr<base::TestMockTimeTaskRunner> test_task_runner =
+      new base::TestMockTimeTaskRunner;
+
+  // This test will mark an alternative service A that has already been marked
+  // broken many times, then immediately mark another alternative service B as
+  // broken for the first time. Because A's been marked broken many times
+  // already, its brokenness will be scheduled to expire much further in the
+  // future than B, even though it was marked broken before B. This test makes
+  // sure that even though A was marked broken before B, B's brokenness should
+  // expire before A.
+
+  // This test only makes sense if HttpServerPropertiesImpl's expiration delay
+  // for a broken AlternativeService is an increasing function of how many times
+  // that AlternativeService has been marked broken already.
+  DCHECK(HttpBrokenAlternativeServicesManager::
+             ComputeBrokenAltSvcExpirationDelayForTest(3) >
+         HttpBrokenAlternativeServicesManager::
+             ComputeBrokenAltSvcExpirationDelayForTest(0));
+
+  url::SchemeHostPort server1("https", "foo", 443);
+  AlternativeService alternative_service1(kProtoQUIC, "foo", 443);
+  SetAlternativeService(server1, alternative_service1);
+
+  url::SchemeHostPort server2("https", "bar", 443);
+  AlternativeService alternative_service2(kProtoQUIC, "bar", 443);
+  SetAlternativeService(server2, alternative_service2);
+
+  // Repeatedly mark alt svc 1 broken and wait for its brokenness to expire.
+  // This will increase its time until expiration.
+  for (int i = 0; i < 3; ++i) {
+    {
+      base::TestMockTimeTaskRunner::ScopedContext scoped_context(
+          test_task_runner);
+      impl_.MarkAlternativeServiceBroken(alternative_service1);
+    }
+    // |impl_| should have posted task to expire the brokenness of
+    // |alternative_service1|
+    EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+    EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
+
+    // Advance time by just enough so that |alternative_service1|'s brokenness
+    // expires.
+    base::TimeDelta delta = HttpBrokenAlternativeServicesManager::
+                                ComputeBrokenAltSvcExpirationDelayForTest(i) +
+                            base::TimeDelta::FromInternalValue(1);
+    test_clock_.AdvanceNow(delta);
+    test_task_runner->FastForwardBy(delta);
+
+    // Ensure brokenness of |alternative_service1| has expired.
+    EXPECT_FALSE(test_task_runner->HasPendingTask());
+    EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service1));
+  }
+
+  {
+    base::TestMockTimeTaskRunner::ScopedContext scoped_context(
+        test_task_runner);
+    impl_.MarkAlternativeServiceBroken(alternative_service1);
+  }
+
+  // Advance time by one quantum
+  base::TimeDelta delta = base::TimeDelta::FromInternalValue(1);
+  test_clock_.AdvanceNow(delta);
+  test_task_runner->FastForwardBy(delta);
+
+  {
+    base::TestMockTimeTaskRunner::ScopedContext scoped_context(
+        test_task_runner);
+    impl_.MarkAlternativeServiceBroken(alternative_service2);
+  }
+
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service2));
+
+  // Advance time by just enough so that |alternative_service2|'s brokennness
+  // expires.
+  delta = HttpBrokenAlternativeServicesManager::
+              ComputeBrokenAltSvcExpirationDelayForTest(0) +
+          base::TimeDelta::FromInternalValue(1);
+  test_clock_.AdvanceNow(delta);
+  test_task_runner->FastForwardBy(delta);
+
+  EXPECT_TRUE(impl_.IsAlternativeServiceBroken(alternative_service1));
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service2));
+
+  // Advance time by enough so that |alternative_service1|'s brokenness expires.
+  delta = HttpBrokenAlternativeServicesManager ::
+              ComputeBrokenAltSvcExpirationDelayForTest(3) +
+          base::TimeDelta::FromInternalValue(1);
+  test_clock_.AdvanceNow(delta);
+  test_task_runner->FastForwardBy(delta);
+
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service1));
+  EXPECT_FALSE(impl_.IsAlternativeServiceBroken(alternative_service2));
+}
+
 typedef HttpServerPropertiesImplTest SupportsQuicServerPropertiesTest;
 
 TEST_F(SupportsQuicServerPropertiesTest, Set) {
   HostPortPair quic_server_google("www.google.com", 443);
 
   // Check by initializing empty address.
   IPAddress initial_address;
   impl_.SetSupportsQuic(&initial_address);
 
   IPAddress address;
@@ skipping 228 matching lines @@
   EXPECT_EQ(quic_server_info1, *(impl_.GetQuicServerInfo(quic_server_id)));
 
   impl_.Clear();
   EXPECT_EQ(0u, impl_.quic_server_info_map().size());
   EXPECT_EQ(nullptr, impl_.GetQuicServerInfo(quic_server_id));
 }
 
 }  // namespace
 
 }  // namespace net