NetworkService + iOS: Use a NetworkContext to make URLLoaderFactories.

This will let use remove the use of URLRequestContextGetters in the
NetworkService, and also let us adjust URLLoaderFactory lifetime to
so that it lives as long as it has a live URLLoader, so it can own
shared objects.

Bug: 803149
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7028886f9df671530ee9863f80dc437adc20cdfb
Reviewed-on: https://chromium-review.googlesource.com/952342Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542188}

ios/chrome/browser/browser_state/BUILD.gn

@@ -10,13 +10,17 @@ source_set("browser_state") {
     "chrome_browser_state.mm",
     "chrome_browser_state_manager.h",
   ]
+
+  public_deps = [
+    "//ios/web",
+    "//net",
+  ]
+
   deps = [
     "//base",
     "//components/prefs",
     "//components/sync_preferences",
     "//ios/chrome/browser/net:net_types",
-    "//ios/web",
-    "//net",
   ]
 
   configs += [ "//build/config/compiler:enable_arc" ]
@@ -138,6 +142,7 @@ source_set("unit_tests") {
     "test_chrome_browser_state_manager_unittest.cc",
   ]
   deps = [
+    ":browser_state",
     ":test_support",
     "//base",
     "//ios/web/public/test",
@@ -156,8 +161,12 @@ source_set("test_support") {
     "test_chrome_browser_state_manager.cc",
     "test_chrome_browser_state_manager.h",
   ]
-  deps = [
+
+  public_deps = [
     ":browser_state",
+  ]
+
+  deps = [
     ":browser_state_impl",
     "//base",
     "//components/bookmarks/browser",

ios/web/DEPS

@@ -5,8 +5,8 @@ include_rules = [
   "+ios/web",
   "+mojo/public",
   "+net",
-  "+services/network/public/mojom/url_loader_factory.mojom.h",
-  "+services/network/url_loader.h",
+  "+services/network/network_context.h",
+  "+services/network/public/mojom",
   "+ui",
 
   # Needed to embed the ServiceManager in //ios/web.

ios/web/browser_state.mm

@@ -19,9 +19,10 @@
 #include "ios/web/public/web_client.h"
 #include "ios/web/public/web_thread.h"
 #include "ios/web/webui/url_data_manager_ios_backend.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
 #include "net/url_request/url_request_context_getter.h"
-#include "services/network/public/mojom/url_loader_factory.mojom.h"
-#include "services/network/url_loader.h"
+#include "net/url_request/url_request_context_getter_observer.h"
+#include "services/network/network_context.h"
 #include "services/service_manager/public/cpp/connector.h"
 #include "services/service_manager/public/mojom/service.mojom.h"
 
@@ -106,50 +107,55 @@ class BrowserStateServiceManagerConnectionHolder
 
 }  // namespace
 
-class BrowserState::URLLoaderFactory : public network::mojom::URLLoaderFactory {
+// Class that owns a NetworkContext wrapping the BrowserState's
+// URLRequestContext.  This allows using the URLLoaderFactory and
+// NetworkContext APIs while still issuing requests with a URLRequestContext
+// created by a BrowserState subclass.
+//
+// Created on the UI thread by the BrowserState on first use, so the
+// BrowserState can own the NetworkContextOwner.  A task is then posted to the
+// IO thread to create the NetworkContext itself, which has to live on the IO
+// thread, since that's where the URLRequestContext lives.  Destroyed on the IO
+// thread during shutdown, to ensure the NetworkContext is destroyed on the
+// right thread.
+class BrowserState::NetworkContextOwner
+    : public net::URLRequestContextGetterObserver {
  public:
-  explicit URLLoaderFactory(net::URLRequestContextGetter* request_context)
-      : request_context_(request_context) {}
-
-  void CreateLoaderAndStart(network::mojom::URLLoaderRequest request,
-                            int32_t routing_id,
-                            int32_t request_id,
-                            uint32_t options,
-                            const network::ResourceRequest& resource_request,
-                            network::mojom::URLLoaderClientPtr client,
-                            const net::MutableNetworkTrafficAnnotationTag&
-                                traffic_annotation) override {
-    WebThread::PostTask(
-        web::WebThread::IO, FROM_HERE,
-        base::BindOnce(URLLoaderFactory::CreateLoaderAndStartOnIO,
-                       request_context_, std::move(request), routing_id,
-                       request_id, options, resource_request,
-                       client.PassInterface(), traffic_annotation));
+  explicit NetworkContextOwner(net::URLRequestContextGetter* request_context)
+      : request_context_(request_context) {
+    DCHECK_CURRENTLY_ON(WebThread::UI);
   }
 
-  void Clone(network::mojom::URLLoaderFactoryRequest request) override {
-    NOTREACHED() << "Clone shouldn't be called on iOS";
+  ~NetworkContextOwner() override {
+    DCHECK_CURRENTLY_ON(WebThread::IO);
+    if (request_context_)
+      request_context_->RemoveObserver(this);
   }
 
- private:
-  static void CreateLoaderAndStartOnIO(
-      scoped_refptr<net::URLRequestContextGetter> request_getter,
-      network::mojom::URLLoaderRequest request,
-      int32_t routing_id,
-      int32_t request_id,
-      uint32_t options,
-      const network::ResourceRequest& resource_request,
-      network::mojom::URLLoaderClientPtrInfo client_info,
-      const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) {
-    // Object deletes itself when the pipe or the URLRequestContext goes away.
-    network::mojom::URLLoaderClientPtr client(std::move(client_info));
-    new network::URLLoader(
-        request_getter, nullptr, std::move(request), options, resource_request,
-        false, std::move(client),
-        static_cast<net::NetworkTrafficAnnotationTag>(traffic_annotation), 0,
-        nullptr, nullptr);
+  void InitializeOnIOThread(
+      network::mojom::NetworkContextRequest network_context_request) {
+    DCHECK_CURRENTLY_ON(WebThread::IO);
+    DCHECK(!network_context_);
+
+    network_context_ = std::make_unique<network::NetworkContext>(
+        nullptr, std::move(network_context_request), request_context_);
+    request_context_->AddObserver(this);
+  }
+
+  // net::URLRequestContextGetterObserver implementation:
+  void OnContextShuttingDown() override {
+    DCHECK_CURRENTLY_ON(WebThread::IO);
+
+    // Cancels any pending requests owned by the NetworkContext.
+    network_context_.reset();
+
+    request_context_->RemoveObserver(this);
+    request_context_ = nullptr;
   }
+
+ private:
   scoped_refptr<net::URLRequestContextGetter> request_context_;
+  std::unique_ptr<network::NetworkContext> network_context_;
 };
 
 // static
@@ -182,6 +188,11 @@ BrowserState::~BrowserState() {
       << "Attempting to destroy a BrowserState that never called "
       << "Initialize()";
 
+  if (network_context_) {
+    web::WebThread::DeleteSoon(web::WebThread::IO, FROM_HERE,
+                               network_context_owner_.release());
+  }
+
   RemoveBrowserStateFromUserIdMap(this);
 
   // Delete the URLDataManagerIOSBackend instance on the IO thread if it has
@@ -199,8 +210,20 @@ BrowserState::~BrowserState() {
 
 network::mojom::URLLoaderFactory* BrowserState::GetURLLoaderFactory() {
   if (!url_loader_factory_) {
-    url_loader_factory_ =
-        std::make_unique<URLLoaderFactory>(GetRequestContext());
+    DCHECK(!network_context_);
+    DCHECK(!network_context_owner_);
+
+    network_context_owner_ =
+        std::make_unique<NetworkContextOwner>(GetRequestContext());
+    WebThread::PostTask(
+        web::WebThread::IO, FROM_HERE,
+        base::BindOnce(&NetworkContextOwner::InitializeOnIOThread,
+                       // This is safe, since the NetworkContextOwner will be
+                       // deleted on the IO thread.
+                       base::Unretained(network_context_owner_.get()),
+                       mojo::MakeRequest(&network_context_)));
+    network_context_->CreateURLLoaderFactory(
+        mojo::MakeRequest(&url_loader_factory_), 0 /* process_id */);
   }
 
   return url_loader_factory_.get();

ios/web/public/BUILD.gn

@@ -8,6 +8,8 @@ source_set("public") {
   public_deps = [
     ":user_agent",
     "//net",
+    "//services/network/public/cpp",
+    "//services/network/public/mojom",
   ]
 
   deps = [

ios/web/public/DEPS

@@ -2,6 +2,7 @@ include_rules = [
   # web interfaces cannot depend on private web code.
   "-ios/web",
   "+ios/web/public",
+  "+services/network/public/mojom",
   "+services/service_manager/embedder/runner",
 ]
 

ios/web/public/browser_state.h

@@ -8,6 +8,8 @@
 #include <memory>
 
 #include "base/supports_user_data.h"
+#include "services/network/public/mojom/network_service.mojom.h"
+#include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "services/service_manager/embedder/embedded_service_info.h"
 
 namespace base {
@@ -98,7 +100,7 @@ class BrowserState : public base::SupportsUserData {
                          const base::FilePath& path);
 
  private:
-  class URLLoaderFactory;
+  class NetworkContextOwner;
   friend class URLDataManagerIOS;
   friend class URLRequestChromeJob;
 
@@ -108,7 +110,12 @@ class BrowserState : public base::SupportsUserData {
   // Not intended for usage outside of //web.
   URLDataManagerIOSBackend* GetURLDataManagerIOSBackendOnIOThread();
 
-  std::unique_ptr<URLLoaderFactory> url_loader_factory_;
+  network::mojom::URLLoaderFactoryPtr url_loader_factory_;
+  network::mojom::NetworkContextPtr network_context_;
+
+  // Owns the network::NetworkContext that backs |url_loader_factory_|. Created
+  // on the UI thread, destroyed on the IO thread.
+  std::unique_ptr<NetworkContextOwner> network_context_owner_;
 
   // The URLDataManagerIOSBackend instance associated with this BrowserState.
   // Created and destroyed on the IO thread, and should be accessed only from

services/network/public/cpp/BUILD.gn

@@ -35,6 +35,7 @@ component("cpp") {
 
   public_deps = [
     ":cpp_base",
+    "//net",
     "//services/network/public/mojom",
     "//url/ipc:url_ipc",
   ]
@@ -44,7 +45,6 @@ component("cpp") {
     "//components/prefs",
     "//ipc",
     "//mojo/common",
-    "//net",
     "//services/proxy_resolver/public/mojom",
   ]