Reland "[Previews] Add offline check before HintsFetch."

This is a reland of b58c0181

The original CL used an unset class variable rather than the callback
being passed into the function.

The reland solves this by using the correct function variable and adds
better unittesting to confirm the correct behavior.

Original change's description:
> [Previews] Add offline check before HintsFetch.
>
> Only attempt to fetch hints if the network is available.
>
> Note: The Optimization Guide does not support iOS but is in components
> due to supporting Previews. A future refactor of Previews may allow for
>  moving Optimization Guide to the browser layer. An assertion was added
> to the BUILD.gn file to make the iOS decision more clear.
>
> Bug: 986817
> Change-Id: Iafa4101b42147530e0b38f2d6a4eb36a7ddac37c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715505
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Robert Ogden <robertogden@chromium.org>
> Commit-Queue: Michael Crouse <mcrouse@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#681180}

TBR=webrtc-chromium-sheriffs-robots@google.com,kinuko@google.com

Bug: 986817
Change-Id: I4a2e51c8eb7b498b3d9cb542da265e5ebc15431f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729053
Commit-Queue: Michael Crouse <mcrouse@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683272}

chrome/browser/previews/hints_fetcher_browsertest.cc

@@ -40,6 +40,7 @@
 #include "components/previews/core/previews_switches.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/network_connection_change_simulator.h"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
 #include "services/network/public/cpp/network_quality_tracker.h"
@@ -168,6 +169,11 @@ class HintsFetcherDisabledBrowserTest : public InProcessBrowserTest {
     run_loop.Run();
   }
 
+  void SetNetworkConnectionOffline() {
+    content::NetworkConnectionChangeSimulator().SetConnectionType(
+        network::mojom::ConnectionType::CONNECTION_NONE);
+  }
+
   // Seeds the Site Engagement Service with two HTTP and two HTTPS sites for the
   // current profile.
   void SeedSiteEngagementService() {
@@ -652,3 +658,34 @@ IN_PROC_BROWSER_TEST_F(
           optimization_guide::HintCacheStore::StoreEntryType::kComponentHint),
       0);
 }
+
+IN_PROC_BROWSER_TEST_F(
+    HintsFetcherBrowserTest,
+    DISABLE_ON_WIN_MAC_CHROMESOS(HintsFetcherNetworkOffline)) {
+  const base::HistogramTester* histogram_tester = GetHistogramTester();
+  GURL url = https_url();
+  base::CommandLine::ForCurrentProcess()->RemoveSwitch(
+      optimization_guide::switches::kFetchHintsOverride);
+  base::CommandLine::ForCurrentProcess()->AppendSwitch(
+      optimization_guide::switches::kFetchHintsOverrideTimer);
+
+  // Set the network to be offline.
+  SetNetworkConnectionOffline();
+
+  // Set the blacklist state to initialized so the sites in the engagement
+  // service will be used and not blacklisted on the first GetTopHosts
+  // request.
+  SeedSiteEngagementService();
+
+  // Set the blacklist state to initialized so the sites in the engagement
+  // service will be used and not blacklisted on the first GetTopHosts request.
+  SetTopHostBlacklistState(optimization_guide::prefs::
+                               HintsFetcherTopHostBlacklistState::kInitialized);
+
+  // Whitelist NoScript for https_url()'s' host.
+  SetUpComponentUpdateHints(https_url());
+
+  // No HintsFetch should occur because the connection is offline.
+  histogram_tester->ExpectTotalCount(
+      "OptimizationGuide.HintsFetcher.GetHintsRequest.HostCount", 0);
+}

components/optimization_guide/BUILD.gn

@@ -2,6 +2,8 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+assert(!is_ios, "Optimization Guide is not available on iOS.")
+
 static_library("optimization_guide") {
   sources = [
     "bloom_filter.cc",
@@ -42,6 +44,7 @@ static_library("optimization_guide") {
     "//components/leveldb_proto",
     "//components/optimization_guide/proto:optimization_guide_proto",
     "//components/prefs",
+    "//content/public/browser",
     "//google_apis",
     "//net:net",
     "//services/network/public/cpp",

components/optimization_guide/DEPS

 include_rules = [
   "+components/leveldb_proto",
   "+components/prefs",
+  "+content/public/browser",
   "+google_apis",
   "+net",
   "+services/network",

components/optimization_guide/hints_fetcher.cc

@@ -12,6 +12,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "components/optimization_guide/optimization_guide_features.h"
 #include "components/optimization_guide/proto/hints.pb.h"
+#include "content/public/browser/network_service_instance.h"
 #include "net/base/load_flags.h"
 #include "net/base/url_util.h"
 #include "net/http/http_request_headers.h"
@@ -42,6 +43,11 @@ bool HintsFetcher::FetchOptimizationGuideServiceHints(
     HintsFetchedCallback hints_fetched_callback) {
   SEQUENCE_CHECKER(sequence_checker_);
 
+  if (content::GetNetworkConnectionTracker()->IsOffline()) {
+    std::move(hints_fetched_callback).Run(base::nullopt);
+    return false;
+  }
+
   if (url_loader_)
     return false;
 

components/optimization_guide/hints_fetcher_unittest.cc

@@ -18,6 +18,7 @@
 #include "net/base/url_util.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
+#include "services/network/test/test_network_connection_tracker.h"
 #include "services/network/test/test_url_loader_factory.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -49,6 +50,18 @@ class HintsFetcherTest : public testing::Test {
 
   bool hints_fetched() { return hints_fetched_; }
 
+  void SetConnectionOffline() {
+    network_tracker_ = network::TestNetworkConnectionTracker::GetInstance();
+    network_tracker_->SetConnectionType(
+        network::mojom::ConnectionType::CONNECTION_NONE);
+  }
+
+  void SetConnectionOnline() {
+    network_tracker_ = network::TestNetworkConnectionTracker::GetInstance();
+    network_tracker_->SetConnectionType(
+        network::mojom::ConnectionType::CONNECTION_4G);
+  }
+
  protected:
   bool FetchHints(const std::vector<std::string>& hosts) {
     bool status = hints_fetcher_->FetchOptimizationGuideServiceHints(
@@ -90,6 +103,7 @@ class HintsFetcherTest : public testing::Test {
 
   scoped_refptr<network::SharedURLLoaderFactory> shared_url_loader_factory_;
   network::TestURLLoaderFactory test_url_loader_factory_;
+  network::TestNetworkConnectionTracker* network_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(HintsFetcherTest);
 };
@@ -135,4 +149,17 @@ TEST_F(HintsFetcherTest, FetchReturnBadResponse) {
   EXPECT_FALSE(hints_fetched());
 }
 
+TEST_F(HintsFetcherTest, FetchAttemptWhenNetworkOffline) {
+  SetConnectionOffline();
+  std::string response_content;
+  EXPECT_FALSE(FetchHints(std::vector<std::string>()));
+  EXPECT_FALSE(hints_fetched());
+
+  SetConnectionOnline();
+  EXPECT_TRUE(FetchHints(std::vector<std::string>()));
+  VerifyHasPendingFetchRequests();
+  EXPECT_TRUE(SimulateResponse(response_content, net::HTTP_OK));
+  EXPECT_TRUE(hints_fetched());
+}
+
 }  // namespace optimization_guide