Cache PreviewsProber results, sharded by network id

Step 1 of 3 for caching prober results. This CL adds caching by network
id (more or less just the connection type), and the URL host:port being
probed.

Future CLs on this thread:
* Step 2 - async revalidate cache entries after some expiry
* Step 3 - persist to disk with PrefService

Bug: 977603
Change-Id: If84f74e4cdf29eba16f864c34e08170d65ee2d20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1691548
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#675794}

chrome/browser/BUILD.gn

@@ -1852,6 +1852,7 @@ jumbo_split_static_library("browser") {
   deps = [
     ":active_use_util",
     ":ntp_background_proto",
+    ":previews_protos",
     ":resource_prefetch_predictor_proto",
     "//base:i18n",
     "//base/allocator:buildflags",
@@ -5124,6 +5125,12 @@ source_set("theme_properties") {
   ]
 }
 
+proto_library("previews_protos") {
+  sources = [
+    "previews/proto/previews_prober_cache_entry.proto",
+  ]
+}
+
 proto_library("resource_prefetch_predictor_proto") {
   sources = [
     "predictors/resource_prefetch_predictor.proto",

chrome/browser/previews/previews_prober.cc

@@ -6,10 +6,14 @@
 
 #include <math.h>
 
+#include "base/base64.h"
 #include "base/bind.h"
 #include "base/guid.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/stringprintf.h"
 #include "base/time/default_tick_clock.h"
 #include "build/build_config.h"
+#include "chrome/browser/previews/proto/previews_prober_cache_entry.pb.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/network_service_instance.h"
 #include "net/base/load_flags.h"
@@ -20,6 +24,11 @@
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/simple_url_loader.h"
 
+#if defined(OS_ANDROID)
+#include "net/android/network_library.h"
+#include "net/base/network_interfaces.h"
+#endif
+
 namespace {
 
 std::string NameForClient(PreviewsProber::ClientName name) {
@@ -51,6 +60,99 @@ base::TimeDelta ComputeNextTimeDeltaForBackoff(PreviewsProber::Backoff backoff,
   }
 }
 
+std::string GenerateNetworkID(
+    network::NetworkConnectionTracker* network_connection_tracker) {
+  network::mojom::ConnectionType connection_type =
+      network::mojom::ConnectionType::CONNECTION_UNKNOWN;
+  if (network_connection_tracker) {
+    network_connection_tracker->GetConnectionType(&connection_type,
+                                                  base::DoNothing());
+  }
+
+  std::string id = base::NumberToString(static_cast<int>(connection_type));
+  bool is_cellular =
+      network::NetworkConnectionTracker::IsConnectionCellular(connection_type);
+  if (is_cellular) {
+    // Don't care about cell connection type.
+    id = "cell";
+  }
+
+// Further identify WiFi and cell connections. These calls are only supported
+// for Android devices.
+#if defined(OS_ANDROID)
+  if (connection_type == network::mojom::ConnectionType::CONNECTION_WIFI) {
+    return base::StringPrintf("%s,%s", id.c_str(), net::GetWifiSSID().c_str());
+  }
+
+  if (is_cellular) {
+    return base::StringPrintf(
+        "%s,%s", id.c_str(),
+        net::android::GetTelephonyNetworkOperator().c_str());
+  }
+#endif
+
+  return id;
+}
+
+base::Optional<base::Value> EncodeCacheEntryValue(
+    const PreviewsProberCacheEntry& entry) {
+  std::string serialized_entry;
+  bool serialize_to_string_ok = entry.SerializeToString(&serialized_entry);
+  if (!serialize_to_string_ok)
+    return base::nullopt;
+
+  std::string base64_encoded;
+  base::Base64Encode(serialized_entry, &base64_encoded);
+  return base::Value(base64_encoded);
+}
+
+base::Optional<PreviewsProberCacheEntry> DecodeCacheEntryValue(
+    const base::Value& value) {
+  if (!value.is_string())
+    return base::nullopt;
+
+  std::string base64_decoded;
+  if (!base::Base64Decode(value.GetString(), &base64_decoded))
+    return base::nullopt;
+
+  PreviewsProberCacheEntry entry;
+  if (!entry.ParseFromString(base64_decoded))
+    return base::nullopt;
+
+  return entry;
+}
+
+void RemoveOldestDictionaryEntry(base::DictionaryValue* dict) {
+  std::vector<std::string> keys_to_remove;
+
+  std::string oldest_key;
+  base::Time oldest_mod_time = base::Time::Max();
+  for (const auto& iter : dict->DictItems()) {
+    base::Optional<PreviewsProberCacheEntry> entry =
+        DecodeCacheEntryValue(iter.second);
+    if (!entry.has_value()) {
+      // Also remove anything that can't be decoded.
+      keys_to_remove.push_back(iter.first);
+      continue;
+    }
+
+    base::Time mod_time = base::Time::FromDeltaSinceWindowsEpoch(
+        base::TimeDelta::FromMicroseconds(entry.value().last_modified()));
+    if (mod_time < oldest_mod_time) {
+      oldest_key = iter.first;
+      oldest_mod_time = mod_time;
+    }
+  }
+
+  if (!oldest_key.empty()) {
+    keys_to_remove.push_back(oldest_key);
+  }
+
+  for (const std::string& key : keys_to_remove) {
+    dict->RemoveKey(key);
+  }
+}
+
 #if defined(OS_ANDROID)
 bool IsInForeground(base::android::ApplicationState state) {
   switch (state) {
@@ -84,7 +186,8 @@ PreviewsProber::PreviewsProber(
     const HttpMethod http_method,
     const net::HttpRequestHeaders headers,
     const RetryPolicy& retry_policy,
-    const TimeoutPolicy& timeout_policy)
+    const TimeoutPolicy& timeout_policy,
+    const size_t max_cache_entries)
     : PreviewsProber(delegate,
                      url_loader_factory,
                      name,
@@ -93,6 +196,7 @@ PreviewsProber::PreviewsProber(
                      headers,
                      retry_policy,
                      timeout_policy,
+                     max_cache_entries,
                      base::DefaultTickClock::GetInstance()) {}
 
 PreviewsProber::PreviewsProber(
@@ -104,6 +208,7 @@ PreviewsProber::PreviewsProber(
     const net::HttpRequestHeaders headers,
     const RetryPolicy& retry_policy,
     const TimeoutPolicy& timeout_policy,
+    const size_t max_cache_entries,
     const base::TickClock* tick_clock)
     : delegate_(delegate),
       name_(NameForClient(name)),
@@ -112,11 +217,12 @@ PreviewsProber::PreviewsProber(
       headers_(headers),
       retry_policy_(retry_policy),
       timeout_policy_(timeout_policy),
+      max_cache_entries_(max_cache_entries),
       successive_retry_count_(0),
       successive_timeout_count_(0),
+      cached_probe_results_(std::make_unique<base::DictionaryValue>()),
       tick_clock_(tick_clock),
       is_active_(false),
-      last_probe_status_(base::nullopt),
       network_connection_tracker_(nullptr),
       url_loader_factory_(url_loader_factory),
       weak_factory_(this) {
@@ -314,7 +420,7 @@ void PreviewsProber::ProcessProbeFailure() {
   DCHECK(!url_loader_);
   DCHECK(is_active_);
 
-  last_probe_status_ = false;
+  RecordProbeResult(false);
 
   if (retry_policy_.max_retries > successive_retry_count_) {
     base::TimeDelta interval = ComputeNextTimeDeltaForBackoff(
@@ -341,11 +447,48 @@ void PreviewsProber::ProcessProbeSuccess() {
   DCHECK(!url_loader_);
   DCHECK(is_active_);
 
-  last_probe_status_ = true;
+  RecordProbeResult(true);
   ResetState();
 }
 
 base::Optional<bool> PreviewsProber::LastProbeWasSuccessful() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  return last_probe_status_;
+
+  base::Value* cache_entry =
+      cached_probe_results_->FindKey(GetCacheKeyForCurrentNetwork());
+  if (!cache_entry)
+    return base::nullopt;
+
+  base::Optional<PreviewsProberCacheEntry> entry =
+      DecodeCacheEntryValue(*cache_entry);
+  if (!entry.has_value())
+    return base::nullopt;
+
+  return entry.value().is_success();
+}
+
+void PreviewsProber::RecordProbeResult(bool success) {
+  PreviewsProberCacheEntry entry;
+  entry.set_is_success(success);
+  entry.set_last_modified(
+      base::Time::Now().ToDeltaSinceWindowsEpoch().InMicroseconds());
+
+  base::Optional<base::Value> encoded = EncodeCacheEntryValue(entry);
+  if (!encoded.has_value()) {
+    NOTREACHED();
+    return;
+  }
+
+  cached_probe_results_->SetKey(GetCacheKeyForCurrentNetwork(),
+                                std::move(encoded.value()));
+
+  if (cached_probe_results_->DictSize() > max_cache_entries_)
+    RemoveOldestDictionaryEntry(cached_probe_results_.get());
+}
+
+std::string PreviewsProber::GetCacheKeyForCurrentNetwork() const {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  return base::StringPrintf(
+      "%s;%s:%d", GenerateNetworkID(network_connection_tracker_).c_str(),
+      url_.host().c_str(), url_.EffectiveIntPort());
 }

chrome/browser/previews/previews_prober.h

@@ -15,6 +15,7 @@
 #include "base/sequence_checker.h"
 #include "base/time/tick_clock.h"
 #include "base/timer/timer.h"
+#include "base/values.h"
 #include "build/build_config.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_request_headers.h"
@@ -126,7 +127,8 @@ class PreviewsProber
       HttpMethod http_method,
       const net::HttpRequestHeaders headers,
       const RetryPolicy& retry_policy,
-      const TimeoutPolicy& timeout_policy);
+      const TimeoutPolicy& timeout_policy,
+      const size_t max_cache_entries);
   ~PreviewsProber() override;
 
   // Sends a probe now if the prober is currently inactive. If the probe is
@@ -155,6 +157,7 @@ class PreviewsProber
       const net::HttpRequestHeaders headers,
       const RetryPolicy& retry_policy,
       const TimeoutPolicy& timeout_policy,
+      const size_t max_cache_entries,
       const base::TickClock* tick_clock);
 
  private:
@@ -166,6 +169,8 @@ class PreviewsProber
   void ProcessProbeSuccess();
   void AddSelfAsNetworkConnectionObserver(
       network::NetworkConnectionTracker* network_connection_tracker);
+  void RecordProbeResult(bool success);
+  std::string GetCacheKeyForCurrentNetwork() const;
 #if defined(OS_ANDROID)
   void OnApplicationStateChange(base::android::ApplicationState new_state);
 #endif
@@ -193,6 +198,9 @@ class PreviewsProber
   // The timeout policy to use in this prober.
   const TimeoutPolicy timeout_policy_;
 
+  // The maximum allowable size of |cached_probe_results_|.
+  const size_t max_cache_entries_;
+
   // The number of retries that have been attempted. This count does not include
   // the original probe.
   size_t successive_retry_count_;
@@ -206,15 +214,17 @@ class PreviewsProber
   // If a probe is being attempted, this will be running until the TTL.
   std::unique_ptr<base::OneShotTimer> timeout_timer_;
 
+  // Caches past probe results in a mapping of one tuple to another:
+  //   (network_id, url_) -> (last_probe_status, last_modification_time).
+  // No more than |max_cache_entries_| will be kept in this dictionary.
+  std::unique_ptr<base::DictionaryValue> cached_probe_results_;
+
   // The tick clock used within this class.
   const base::TickClock* tick_clock_;
 
   // Whether the prober is currently sending probes.
   bool is_active_;
 
-  // The status of the last completed probe, if any.
-  base::Optional<bool> last_probe_status_;
-
   // This reference is kept around for unregistering |this| as an observer on
   // any thread.
   network::NetworkConnectionTracker* network_connection_tracker_;

chrome/browser/previews/previews_prober_browsertest.cc

@@ -131,7 +131,7 @@ IN_PROC_BROWSER_TEST_F(PreviewsProberBrowserTest, OK) {
   PreviewsProber prober(&delegate, browser()->profile()->GetURLLoaderFactory(),
                         PreviewsProber::ClientName::kLitepages, url,
                         PreviewsProber::HttpMethod::kGet, headers, retry_policy,
-                        timeout_policy);
+                        timeout_policy, 1);
   prober.SendNowIfInactive(false);
   WaitForCompletedProbe(&prober);
 
@@ -152,7 +152,7 @@ IN_PROC_BROWSER_TEST_F(PreviewsProberBrowserTest, Timeout) {
   PreviewsProber prober(&delegate, browser()->profile()->GetURLLoaderFactory(),
                         PreviewsProber::ClientName::kLitepages, url,
                         PreviewsProber::HttpMethod::kGet, headers, retry_policy,
-                        timeout_policy);
+                        timeout_policy, 1);
   prober.SendNowIfInactive(false);
   WaitForCompletedProbe(&prober);
 
@@ -169,7 +169,7 @@ IN_PROC_BROWSER_TEST_F(PreviewsProberBrowserTest, NetworkChange) {
   PreviewsProber prober(&delegate, browser()->profile()->GetURLLoaderFactory(),
                         PreviewsProber::ClientName::kLitepages, url,
                         PreviewsProber::HttpMethod::kGet, headers, retry_policy,
-                        timeout_policy);
+                        timeout_policy, 1);
   SimulateNetworkChange(network::mojom::ConnectionType::CONNECTION_4G);
   WaitForCompletedProbe(&prober);
 

chrome/browser/previews/previews_prober_unittest.cc

@@ -54,6 +54,7 @@ class TestPreviewsProber : public PreviewsProber {
       const net::HttpRequestHeaders headers,
       const RetryPolicy& retry_policy,
       const TimeoutPolicy& timeout_policy,
+      const size_t max_cache_entries,
       const base::TickClock* tick_clock)
       : PreviewsProber(delegate,
                        url_loader_factory,
@@ -63,6 +64,7 @@ class TestPreviewsProber : public PreviewsProber {
                        headers,
                        retry_policy,
                        timeout_policy,
+                       max_cache_entries,
                        tick_clock) {}
 };
 
@@ -103,7 +105,7 @@ class PreviewsProberTest : public testing::Test {
         delegate, test_shared_loader_factory_,
         PreviewsProber::ClientName::kLitepages, kTestUrl,
         PreviewsProber::HttpMethod::kGet, headers, retry_policy, timeout_policy,
-        thread_bundle_.GetMockTickClock());
+        1, thread_bundle_.GetMockTickClock());
   }
 
   void RunUntilIdle() { thread_bundle_.RunUntilIdle(); }
@@ -205,6 +207,74 @@ TEST_F(PreviewsProberTest, NetworkChangeStartsProber) {
   EXPECT_TRUE(prober->is_active());
 }
 
+TEST_F(PreviewsProberTest, NetworkConnectionShardsCache) {
+  network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_3G);
+  RunUntilIdle();
+
+  std::unique_ptr<PreviewsProber> prober = NewProber();
+  EXPECT_EQ(prober->LastProbeWasSuccessful(), base::nullopt);
+
+  prober->SendNowIfInactive(false);
+  VerifyRequest();
+
+  MakeResponseAndWait(net::HTTP_OK, net::OK);
+  EXPECT_TRUE(prober->LastProbeWasSuccessful().value());
+  EXPECT_FALSE(prober->is_active());
+
+  // The different type of cellular networks shouldn't make a difference.
+  network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_4G);
+  RunUntilIdle();
+  EXPECT_TRUE(prober->LastProbeWasSuccessful().value());
+  network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_2G);
+  RunUntilIdle();
+  EXPECT_TRUE(prober->LastProbeWasSuccessful().value());
+
+  // Switching to WIFI does make a difference.
+  network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_WIFI);
+  RunUntilIdle();
+  EXPECT_EQ(prober->LastProbeWasSuccessful(), base::nullopt);
+}
+
+TEST_F(PreviewsProberTest, CacheMaxSize) {
+  network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_3G);
+  RunUntilIdle();
+
+  std::unique_ptr<PreviewsProber> prober = NewProber();
+  EXPECT_EQ(prober->LastProbeWasSuccessful(), base::nullopt);
+
+  prober->SendNowIfInactive(false);
+  VerifyRequest();
+
+  MakeResponseAndWait(net::HTTP_OK, net::OK);
+  EXPECT_TRUE(prober->LastProbeWasSuccessful().value());
+  EXPECT_FALSE(prober->is_active());
+
+  // Change the connection type and report a new probe result.
+  network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_WIFI);
+  RunUntilIdle();
+
+  EXPECT_EQ(prober->LastProbeWasSuccessful(), base::nullopt);
+
+  prober->SendNowIfInactive(false);
+  VerifyRequest();
+  MakeResponseAndWait(net::HTTP_OK, net::OK);
+  EXPECT_TRUE(prober->LastProbeWasSuccessful().value());
+
+  // Then, flip back to the original connection type. The old probe status
+  // should not be persisted since the max cache size for testing is 1.
+  network::TestNetworkConnectionTracker::GetInstance()->SetConnectionType(
+      network::mojom::ConnectionType::CONNECTION_3G);
+  RunUntilIdle();
+
+  EXPECT_EQ(prober->LastProbeWasSuccessful(), base::nullopt);
+}
+
 #if defined(OS_ANDROID)
 TEST_F(PreviewsProberTest, StartInForeground) {
   std::unique_ptr<PreviewsProber> prober = NewProber();

chrome/browser/previews/proto/previews_prober_cache_entry.proto

+// Copyright 2019 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.
+
+syntax = "proto2";
+
+option optimize_for = LITE_RUNTIME;
+
+message PreviewsProberCacheEntry {
+  // Whether the probe was successful.
+  optional bool is_success = 1;
+
+  // Time when this cache entry was last modified.
+  optional int64 last_modified = 2;
+}