Decouple update_client::NetworkFetcher interface from Chromium network.

This change is mechanical.

It enables NetworkFetcherFactory, which is injected by the embedder, and
which creates instances of NetworkFetcher.

Bug: 929167
Change-Id: I0246974ab18697dbfda49031666f04811e7b1bcf
Reviewed-on: https://chromium-review.googlesource.com/c/1476323Reviewed-by: Julian Pastarmov <pastarmovj@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Sorin Jianu <sorin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#633755}

chrome/browser/component_updater/chrome_component_updater_configurator.cc

@@ -28,7 +28,7 @@
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/update_client/activity_data_service.h"
-#include "components/update_client/network.h"
+#include "components/update_client/net/network_chromium.h"
 #include "components/update_client/protocol_handler.h"
 #include "components/update_client/update_query_params.h"
 #include "content/public/browser/browser_thread.h"
@@ -176,7 +176,7 @@ scoped_refptr<update_client::NetworkFetcherFactory>
 ChromeConfigurator::GetNetworkFetcherFactory() {
   if (!network_fetcher_factory_) {
     network_fetcher_factory_ =
-        base::MakeRefCounted<update_client::NetworkFetcherFactory>(
+        base::MakeRefCounted<update_client::NetworkFetcherChromiumFactory>(
             g_browser_process->system_network_context_manager()
                 ->GetSharedURLLoaderFactory());
   }

chrome/browser/extensions/updater/chrome_update_client_config.cc

@@ -17,7 +17,7 @@
 #include "chrome/common/channel_info.h"
 #include "components/prefs/pref_service.h"
 #include "components/update_client/activity_data_service.h"
-#include "components/update_client/network.h"
+#include "components/update_client/net/network_chromium.h"
 #include "components/update_client/protocol_handler.h"
 #include "components/update_client/update_query_params.h"
 #include "content/public/browser/browser_context.h"
@@ -170,7 +170,7 @@ scoped_refptr<update_client::NetworkFetcherFactory>
 ChromeUpdateClientConfig::GetNetworkFetcherFactory() {
   if (!network_fetcher_factory_) {
     network_fetcher_factory_ =
-        base::MakeRefCounted<update_client::NetworkFetcherFactory>(
+        base::MakeRefCounted<update_client::NetworkFetcherChromiumFactory>(
             content::BrowserContext::GetDefaultStoragePartition(context_)
                 ->GetURLLoaderFactoryForBrowserProcess());
   }

chrome/browser/policy/policy_browsertest.cc

@@ -164,9 +164,9 @@
 #include "components/translate/core/browser/language_state.h"
 #include "components/translate/core/browser/translate_infobar_delegate.h"
 #include "components/unified_consent/pref_names.h"
+#include "components/update_client/net/url_loader_post_interceptor.h"
 #include "components/update_client/update_client.h"
 #include "components/update_client/update_client_errors.h"
-#include "components/update_client/url_loader_post_interceptor.h"
 #include "components/user_prefs/user_prefs.h"
 #include "components/variations/service/variations_service.h"
 #include "components/variations/variations_params_manager.h"

components/update_client/BUILD.gn

@@ -4,14 +4,26 @@
 
 import("//net/features.gni")
 
-source_set("network") {
+source_set("network_public") {
   sources = [
     "net/network_chromium.h",
+  ]
+
+  deps = [
+    "//base",
+  ]
+}
+
+source_set("network") {
+  sources = [
     "net/network_impl.cc",
     "net/network_impl.h",
   ]
 
+  visibility = [ ":*" ]
+
   deps = [
+    ":network_public",
     "//base",
     "//net",
     "//services/network/public/cpp:cpp",
@@ -92,8 +104,14 @@ static_library("update_client") {
     "utils.h",
   ]
 
+  # Allows callers to include the network factory through "update_client" deps.
+  public_deps = [
+    ":network_public",
+  ]
+
   deps = [
     ":network",
+    ":network_public",
     "//base",
     "//components/client_update_protocol",
     "//components/crx_file",
@@ -104,13 +122,14 @@ static_library("update_client") {
     "//components/version_info:version_info",
     "//courgette:courgette_lib",
     "//crypto",
-    "//net",
-    "//services/network/public/cpp:cpp",
     "//third_party/libxml",
     "//url",
   ]
 
-  allow_circular_includes_from = [ ":network" ]
+  allow_circular_includes_from = [
+    ":network",
+    ":network_public",
+  ]
 }
 
 static_library("test_support") {
@@ -201,6 +220,7 @@ source_set("unit_tests") {
   }
 
   deps = [
+    ":network",
     ":test_support",
     ":unit_tests_bundle_data",
     ":update_client",

components/update_client/DEPS

@@ -10,11 +10,17 @@ include_rules = [
   "+crypto",
   "+libxml",
   "+mojo",
-  "+net",
-  "+services/network/public/cpp",
-  "+services/network/test",
   "+services/service_manager/public",
   "+third_party/libxml",
   "+third_party/re2",
   "+third_party/zlib",
 ]
+
+# Allow the unit tests to depend on the network for testing purposes.
+specific_include_rules = {
+  "(test_configurator|.*_unittest\.(cc|h))" : [
+    "+net",
+    "+services/network/public/cpp",
+    "+services/network/test",
+  ]
+}

components/update_client/crx_downloader_unittest.cc

@@ -17,7 +17,7 @@
 #include "base/test/scoped_task_environment.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
-#include "components/update_client/network.h"
+#include "components/update_client/net/network_chromium.h"
 #include "components/update_client/update_client_errors.h"
 #include "components/update_client/utils.h"
 #include "net/base/net_errors.h"
@@ -123,9 +123,9 @@ void CrxDownloaderTest::SetUp() {
   num_progress_calls_ = 0;
 
   // Do not use the background downloader in these tests.
-  crx_downloader_ =
-      CrxDownloader::Create(false, base::MakeRefCounted<NetworkFetcherFactory>(
-                                       test_shared_url_loader_factory_));
+  crx_downloader_ = CrxDownloader::Create(
+      false, base::MakeRefCounted<NetworkFetcherChromiumFactory>(
+                 test_shared_url_loader_factory_));
   crx_downloader_->set_progress_callback(progress_callback_);
 
   test_url_loader_factory_.SetInterceptor(base::BindLambdaForTesting(

components/update_client/net/DEPS

+include_rules = [
+  "+net",
+  "+services/network/public/cpp",
+  "+services/network/test",
+]

components/update_client/net/network_chromium.h

@@ -17,7 +17,6 @@ class SharedURLLoaderFactory;
 
 namespace update_client {
 
-// TODO(sorin): enable the Chromium network factory.
 class NetworkFetcherChromiumFactory : public NetworkFetcherFactory {
  public:
   explicit NetworkFetcherChromiumFactory(

components/update_client/net/network_impl.cc

@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "components/update_client/net/network_chromium.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
@@ -156,13 +157,13 @@ void NetworkFetcherImpl::OnResponseStartedCallback(
            response_head.content_length);
 }
 
-NetworkFetcherFactory::NetworkFetcherFactory(
+NetworkFetcherChromiumFactory::NetworkFetcherChromiumFactory(
     scoped_refptr<network::SharedURLLoaderFactory> shared_url_network_factory)
     : shared_url_network_factory_(shared_url_network_factory) {}
 
-NetworkFetcherFactory::~NetworkFetcherFactory() = default;
+NetworkFetcherChromiumFactory::~NetworkFetcherChromiumFactory() = default;
 
-std::unique_ptr<NetworkFetcher> NetworkFetcherFactory::Create() const {
+std::unique_ptr<NetworkFetcher> NetworkFetcherChromiumFactory::Create() const {
   return std::make_unique<NetworkFetcherImpl>(shared_url_network_factory_);
 }
 

components/update_client/network.h

@@ -21,10 +21,6 @@ namespace base {
 class FilePath;
 }  // namespace base
 
-namespace network {
-class SharedURLLoaderFactory;
-}
-
 namespace update_client {
 
 class NetworkFetcher {
@@ -70,19 +66,14 @@ class NetworkFetcher {
 
 class NetworkFetcherFactory : public base::RefCounted<NetworkFetcherFactory> {
  public:
-  explicit NetworkFetcherFactory(scoped_refptr<network::SharedURLLoaderFactory>
-                                     shared_url_network_factory);
-
-  // TODO(sorin): make this class pure virtual. Instead of this
-  // class, instantiate NetworkFetcherChromiumFactory from update_client/net.
-  virtual std::unique_ptr<NetworkFetcher> Create() const;
+  virtual std::unique_ptr<NetworkFetcher> Create() const = 0;
 
  protected:
   friend class base::RefCounted<NetworkFetcherFactory>;
-  virtual ~NetworkFetcherFactory();
+  NetworkFetcherFactory() = default;
+  virtual ~NetworkFetcherFactory() = default;
 
  private:
-  scoped_refptr<network::SharedURLLoaderFactory> shared_url_network_factory_;
   DISALLOW_COPY_AND_ASSIGN(NetworkFetcherFactory);
 };
 

components/update_client/ping_manager.cc

@@ -23,7 +23,6 @@
 #include "components/update_client/protocol_serializer.h"
 #include "components/update_client/request_sender.h"
 #include "components/update_client/utils.h"
-#include "net/url_request/url_fetcher.h"
 #include "url/gurl.h"
 
 namespace update_client {

components/update_client/test_configurator.cc

@@ -12,7 +12,7 @@
 #include "components/services/patch/public/interfaces/constants.mojom.h"
 #include "components/services/unzip/public/interfaces/constants.mojom.h"
 #include "components/update_client/activity_data_service.h"
-#include "components/update_client/network.h"
+#include "components/update_client/net/network_chromium.h"
 #include "components/update_client/protocol_handler.h"
 #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/service_manager/public/cpp/connector.h"
@@ -46,8 +46,9 @@ TestConfigurator::TestConfigurator()
       test_shared_loader_factory_(
           base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
               &test_url_loader_factory_)),
-      network_fetcher_factory_(base::MakeRefCounted<NetworkFetcherFactory>(
-          test_shared_loader_factory_)) {
+      network_fetcher_factory_(
+          base::MakeRefCounted<NetworkFetcherChromiumFactory>(
+              test_shared_loader_factory_)) {
   connector_factory_.set_ignore_quit_requests(true);
 }
 

components/update_client/url_loader_post_interceptor.h

-// Copyright 2018 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.
-
-#ifndef COMPONENTS_UPDATE_CLIENT_URL_LOADER_POST_INTERCEPTOR_H_
-#define COMPONENTS_UPDATE_CLIENT_URL_LOADER_POST_INTERCEPTOR_H_
-
-// TODO(sorin): remove this shim.
-#include "components/update_client/net/url_loader_post_interceptor.h"
-
-#endif  // COMPONENTS_UPDATE_CLIENT_URL_LOADER_POST_INTERCEPTOR_H_

ios/chrome/browser/component_updater/ios_component_updater_configurator.cc

@@ -15,7 +15,7 @@
 #include "components/component_updater/component_updater_command_line_config_policy.h"
 #include "components/component_updater/configurator_impl.h"
 #include "components/update_client/activity_data_service.h"
-#include "components/update_client/network.h"
+#include "components/update_client/net/network_chromium.h"
 #include "components/update_client/protocol_handler.h"
 #include "components/update_client/update_query_params.h"
 #include "ios/chrome/browser/application_context.h"
@@ -144,7 +144,7 @@ scoped_refptr<update_client::NetworkFetcherFactory>
 IOSConfigurator::GetNetworkFetcherFactory() {
   if (!network_fetcher_factory_) {
     network_fetcher_factory_ =
-        base::MakeRefCounted<update_client::NetworkFetcherFactory>(
+        base::MakeRefCounted<update_client::NetworkFetcherChromiumFactory>(
             GetApplicationContext()->GetSharedURLLoaderFactory());
   }
   return network_fetcher_factory_;