Display brokenness expiration times for broken alternative services in...

Display brokenness expiration times for broken alternative services in chrome://net-internals/#alt-svc

Bug: 792155
Change-Id: I64e9286e157d8d475a90222da910b5c1171bd56b
Reviewed-on: https://chromium-review.googlesource.com/882545
Commit-Queue: Yixin Wang <wangyix@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531974}

net/http/broken_alternative_services.cc

@@ -95,6 +95,21 @@ bool BrokenAlternativeServices::IsAlternativeServiceBroken(
          broken_alternative_service_map_.end();
 }
 
+bool BrokenAlternativeServices::IsAlternativeServiceBroken(
+    const AlternativeService& alternative_service,
+    base::TimeTicks* brokenness_expiration) const {
+  DCHECK(brokenness_expiration != nullptr);
+  // Empty host means use host of origin, callers are supposed to substitute.
+  DCHECK(!alternative_service.host.empty());
+  auto map_it = broken_alternative_service_map_.find(alternative_service);
+  if (map_it == broken_alternative_service_map_.end()) {
+    return false;
+  }
+  auto list_it = map_it->second;
+  *brokenness_expiration = list_it->second;
+  return true;
+}
+
 bool BrokenAlternativeServices::WasAlternativeServiceRecentlyBroken(
     const AlternativeService& alternative_service) {
   DCHECK(!alternative_service.host.empty());

net/http/broken_alternative_services.h

@@ -73,6 +73,12 @@ class NET_EXPORT_PRIVATE BrokenAlternativeServices {
   bool IsAlternativeServiceBroken(
       const AlternativeService& alternative_service) const;
 
+  // Same as IsAlternativeServiceBroken() defined above, but will also set
+  // |brokenness_expiration| to when |alternative_service|'s brokenness will
+  // expire if this function returns true.
+  bool IsAlternativeServiceBroken(const AlternativeService& alternative_service,
+                                  base::TimeTicks* brokenness_expiration) const;
+
   // Returns true if MarkAlternativeServiceRecentlyBroken(alternative_service)
   // or MarkAlternativeServiceBroken(alternative_service) has been called and
   // ConfirmAlternativeService(alternative_service) has not been called

net/http/broken_alternative_services_unittest.cc

@@ -137,6 +137,46 @@ TEST_F(BrokenAlternativeServicesTest, ExpireBrokenAlternateProtocolMappings) {
       alternative_service));
 }
 
+TEST_F(BrokenAlternativeServicesTest, IsAlternativeServiceBroken) {
+  // Tests the IsAlternativeServiceBroken() methods.
+  AlternativeService alternative_service(kProtoQUIC, "foo", 443);
+  base::TimeTicks brokenness_expiration;
+
+  EXPECT_FALSE(
+      broken_services_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(broken_services_.IsAlternativeServiceBroken(
+      alternative_service, &brokenness_expiration));
+
+  broken_services_.MarkAlternativeServiceBroken(alternative_service);
+  EXPECT_TRUE(broken_services_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(broken_services_.IsAlternativeServiceBroken(
+      alternative_service, &brokenness_expiration));
+  EXPECT_EQ(
+      broken_services_clock_->NowTicks() + base::TimeDelta::FromMinutes(5),
+      brokenness_expiration);
+
+  // Fast forward time until |alternative_service|'s brokenness expires.
+  test_task_runner_->FastForwardBy(base::TimeDelta::FromMinutes(5));
+  EXPECT_FALSE(
+      broken_services_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(broken_services_.IsAlternativeServiceBroken(
+      alternative_service, &brokenness_expiration));
+
+  broken_services_.MarkAlternativeServiceBroken(alternative_service);
+  EXPECT_TRUE(broken_services_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_TRUE(broken_services_.IsAlternativeServiceBroken(
+      alternative_service, &brokenness_expiration));
+  EXPECT_EQ(
+      broken_services_clock_->NowTicks() + base::TimeDelta::FromMinutes(10),
+      brokenness_expiration);
+
+  broken_services_.ConfirmAlternativeService(alternative_service);
+  EXPECT_FALSE(
+      broken_services_.IsAlternativeServiceBroken(alternative_service));
+  EXPECT_FALSE(broken_services_.IsAlternativeServiceBroken(
+      alternative_service, &brokenness_expiration));
+}
+
 TEST_F(BrokenAlternativeServicesTest, ExponentialBackoff) {
   // Tests the exponential backoff of the computed expiration delay when an
   // alt svc is marked broken. After being marked broken 10 times, the max

net/http/http_server_properties_impl.cc

@@ -15,15 +15,19 @@
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "base/time/default_clock.h"
 #include "base/values.h"
 
 namespace net {
 
-HttpServerPropertiesImpl::HttpServerPropertiesImpl(base::TickClock* clock)
-    : broken_alternative_services_(
-          this,
-          clock ? clock : base::DefaultTickClock::GetInstance()),
+HttpServerPropertiesImpl::HttpServerPropertiesImpl(base::TickClock* tick_clock,
+                                                   base::Clock* clock)
+    : tick_clock_(tick_clock ? tick_clock
+                             : base::DefaultTickClock::GetInstance()),
+      clock_(clock ? clock : base::DefaultClock::GetInstance()),
+      broken_alternative_services_(this, tick_clock_),
       quic_server_info_map_(kDefaultMaxQuicServerEntries),
       max_server_configs_stored_in_properties_(kDefaultMaxQuicServerEntries) {
   canonical_suffixes_.push_back(".ggpht.com");
@@ -33,7 +37,7 @@ HttpServerPropertiesImpl::HttpServerPropertiesImpl(base::TickClock* clock)
 }
 
 HttpServerPropertiesImpl::HttpServerPropertiesImpl()
-    : HttpServerPropertiesImpl(nullptr) {}
+    : HttpServerPropertiesImpl(nullptr, nullptr) {}
 
 HttpServerPropertiesImpl::~HttpServerPropertiesImpl() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
@@ -285,7 +289,7 @@ HttpServerPropertiesImpl::GetAlternativeServiceInfos(
   // Copy valid alternative service infos into
   // |valid_alternative_service_infos|.
   AlternativeServiceInfoVector valid_alternative_service_infos;
-  const base::Time now = base::Time::Now();
+  const base::Time now = clock_->Now();
   AlternativeServiceMap::iterator map_it = alternative_service_map_.Get(origin);
   if (map_it != alternative_service_map_.end()) {
     HostPortPair host_port_pair(origin.host(), origin.port());
@@ -413,7 +417,7 @@ bool HttpServerPropertiesImpl::SetAlternativeServices(
   if (it != alternative_service_map_.end()) {
     DCHECK(!it->second.empty());
     if (it->second.size() == alternative_service_info_vector.size()) {
-      const base::Time now = base::Time::Now();
+      const base::Time now = clock_->Now();
       changed = false;
       auto new_it = alternative_service_info_vector.begin();
       for (const auto& old : it->second) {
@@ -507,6 +511,8 @@ const AlternativeServiceMap& HttpServerPropertiesImpl::alternative_service_map()
 
 std::unique_ptr<base::Value>
 HttpServerPropertiesImpl::GetAlternativeServiceInfoAsValue() const {
+  const base::Time now = clock_->Now();
+  const base::TimeTicks now_ticks = tick_clock_->NowTicks();
   std::unique_ptr<base::ListValue> dict_list(new base::ListValue);
   for (const auto& alternative_service_map_item : alternative_service_map_) {
     std::unique_ptr<base::ListValue> alternative_service_list(
@@ -521,8 +527,22 @@ HttpServerPropertiesImpl::GetAlternativeServiceInfoAsValue() const {
       if (alternative_service.host.empty()) {
         alternative_service.host = server.host();
       }
-      if (IsAlternativeServiceBroken(alternative_service)) {
-        alternative_service_string.append(" (broken)");
+      base::TimeTicks brokenness_expiration_ticks;
+      if (broken_alternative_services_.IsAlternativeServiceBroken(
+              alternative_service, &brokenness_expiration_ticks)) {
+        // Convert |brokenness_expiration| from TimeTicks to Time
+        base::Time brokenness_expiration =
+            now + (brokenness_expiration_ticks - now_ticks);
+        base::Time::Exploded exploded;
+        brokenness_expiration.LocalExplode(&exploded);
+        std::string broken_info_string =
+            " (broken until " +
+            base::StringPrintf("%04d-%02d-%02d %0d:%0d:%0d", exploded.year,
+                               exploded.month, exploded.day_of_month,
+                               exploded.hour, exploded.minute,
+                               exploded.second) +
+            ")";
+        alternative_service_string.append(broken_info_string);
       }
       alternative_service_list->AppendString(alternative_service_string);
     }

net/http/http_server_properties_impl.h

@@ -27,6 +27,7 @@
 #include "net/http/http_server_properties.h"
 
 namespace base {
+class Clock;
 class TickClock;
 }
 
@@ -37,10 +38,12 @@ class NET_EXPORT HttpServerPropertiesImpl
     : public HttpServerProperties,
       public BrokenAlternativeServices::Delegate {
  public:
-  // |clock| is used for setting expiration times and scheduling the
+  // |tick_clock| is used for setting expiration times and scheduling the
   // expiration of broken alternative services. If null, default clock will be
   // used.
-  explicit HttpServerPropertiesImpl(base::TickClock* clock);
+  // |clock| is used for converting base::TimeTicks to base::Time for
+  // wherever base::Time is preferable.
+  HttpServerPropertiesImpl(base::TickClock* tick_clock, base::Clock* clock);
 
   // Default clock will be used.
   HttpServerPropertiesImpl();
@@ -177,6 +180,9 @@ class NET_EXPORT HttpServerPropertiesImpl
   // have an entry associated with |server|, the method will add one.
   void UpdateCanonicalServerInfoMap(const QuicServerId& server);
 
+  base::TickClock* tick_clock_;  // Unowned
+  base::Clock* clock_;           // Unowned
+
   SpdyServersMap spdy_servers_map_;
   Http11ServerHostPortSet http11_servers_;
 

net/http/http_server_properties_manager.cc

@@ -121,7 +121,8 @@ HttpServerPropertiesManager::HttpServerPropertiesManager(
       base::Unretained(this)));
   net_log_.BeginEvent(NetLogEventType::HTTP_SERVER_PROPERTIES_INITIALIZATION);
 
-  http_server_properties_impl_.reset(new HttpServerPropertiesImpl(clock_));
+  http_server_properties_impl_.reset(
+      new HttpServerPropertiesImpl(clock_, nullptr));
 }
 
 HttpServerPropertiesManager::~HttpServerPropertiesManager() {