iOS: Add NetworkContext wrapper for the SystemURLRequestContext

net-export is being moved to be NetworkContext-based as part of servification, so a NetworkContext
object is needed for the URLRequestContext that the net-export UI on iOS uses.

This is largely based on https://chromium-review.googlesource.com/952342, which added such
wrappers for BrowserContext URLRequestContexts, and much of the CL is just moving out the
NetworkContextOwner that added into its own file.

Bug: 767450
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I7c71b508c54389dc3aa30973cd1ea131b48d64d9
Reviewed-on: https://chromium-review.googlesource.com/1030053
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554523}

ios/chrome/browser/DEPS

@@ -103,6 +103,7 @@ include_rules = [
   "+rlz/buildflags",
   "+services/identity/public",
   "+services/metrics/public",
+  "+services/network/public/mojom",
   "+services/service_manager/public",
   "+third_party/breakpad/breakpad/src/client/ios",
   "+third_party/breakpad/breakpad/src/common",

ios/chrome/browser/application_context.h

@@ -37,6 +37,12 @@ namespace net_log {
 class ChromeNetLog;
 }
 
+namespace network {
+namespace mojom {
+class NetworkContext;
+}
+}  // namespace network
+
 namespace network_time {
 class NetworkTimeTracker;
 }
@@ -84,6 +90,10 @@ class ApplicationContext {
   // Gets the URL request context associated with this application.
   virtual net::URLRequestContextGetter* GetSystemURLRequestContext() = 0;
 
+  // Gets the NetworkContext object associated with the same context as
+  // GetSystemURLRequestContext().
+  virtual network::mojom::NetworkContext* GetSystemNetworkContext() = 0;
+
   // Gets the locale used by the application.
   virtual const std::string& GetApplicationLocale() = 0;
 

ios/chrome/browser/application_context_impl.cc

@@ -105,6 +105,11 @@ void ApplicationContextImpl::StartTearDown() {
   if (local_state_) {
     local_state_->CommitPendingWrite();
   }
+
+  if (network_context_) {
+    web::WebThread::DeleteSoon(web::WebThread::IO, FROM_HERE,
+                               network_context_owner_.release());
+  }
 }
 
 void ApplicationContextImpl::PostDestroyThreads() {
@@ -191,6 +196,15 @@ ApplicationContextImpl::GetSystemURLRequestContext() {
   return ios_chrome_io_thread_->system_url_request_context_getter();
 }
 
+network::mojom::NetworkContext*
+ApplicationContextImpl::GetSystemNetworkContext() {
+  if (!network_context_) {
+    network_context_owner_ = std::make_unique<web::NetworkContextOwner>(
+        GetSystemURLRequestContext(), &network_context_);
+  }
+  return network_context_.get();
+}
+
 const std::string& ApplicationContextImpl::GetApplicationLocale() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!application_locale_.empty());

ios/chrome/browser/application_context_impl.h

@@ -12,6 +12,8 @@
 #include "base/memory/ref_counted.h"
 #include "base/threading/thread_checker.h"
 #include "ios/chrome/browser/application_context.h"
+#include "ios/web/public/network_context_owner.h"
+#include "services/network/public/mojom/network_service.mojom.h"
 
 namespace base {
 class CommandLine;
@@ -45,6 +47,7 @@ class ApplicationContextImpl : public ApplicationContext {
   bool WasLastShutdownClean() override;
   PrefService* GetLocalState() override;
   net::URLRequestContextGetter* GetSystemURLRequestContext() override;
+  network::mojom::NetworkContext* GetSystemNetworkContext() override;
   const std::string& GetApplicationLocale() override;
   ios::ChromeBrowserStateManager* GetChromeBrowserStateManager() override;
   metrics_services_manager::MetricsServicesManager* GetMetricsServicesManager()
@@ -85,6 +88,11 @@ class ApplicationContextImpl : public ApplicationContext {
   // Sequenced task runner for local state related I/O tasks.
   const scoped_refptr<base::SequencedTaskRunner> local_state_task_runner_;
 
+  network::mojom::NetworkContextPtr network_context_;
+
+  // Created on the UI thread, destroyed on the IO thread.
+  std::unique_ptr<web::NetworkContextOwner> network_context_owner_;
+
   bool was_last_shutdown_clean_;
 
   DISALLOW_COPY_AND_ASSIGN(ApplicationContextImpl);

ios/chrome/test/testing_application_context.h

@@ -37,6 +37,7 @@ class TestingApplicationContext : public ApplicationContext {
 
   PrefService* GetLocalState() override;
   net::URLRequestContextGetter* GetSystemURLRequestContext() override;
+  network::mojom::NetworkContext* GetSystemNetworkContext() override;
   const std::string& GetApplicationLocale() override;
   ios::ChromeBrowserStateManager* GetChromeBrowserStateManager() override;
   metrics_services_manager::MetricsServicesManager* GetMetricsServicesManager()

ios/chrome/test/testing_application_context.mm

@@ -86,6 +86,13 @@ TestingApplicationContext::GetSystemURLRequestContext() {
   return nullptr;
 }
 
+network::mojom::NetworkContext*
+TestingApplicationContext::GetSystemNetworkContext() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  NOTREACHED();
+  return nullptr;
+}
+
 const std::string& TestingApplicationContext::GetApplicationLocale() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!application_locale_.empty());

ios/web/BUILD.gn

@@ -62,6 +62,7 @@ source_set("web") {
     "crw_navigation_item_storage.mm",
     "features.mm",
     "load_committed_details.cc",
+    "network_context_owner.cc",
     "service_manager_connection_impl.cc",
     "service_manager_connection_impl.h",
     "service_manager_context.h",
@@ -207,6 +208,7 @@ source_set("ios_web_general_unittests") {
   sources = [
     "browser_state_unittest.cc",
     "history_state_util_unittest.mm",
+    "network_context_owner_unittest.cc",
     "service_manager_connection_impl_unittest.cc",
     "test/web_test_unittest.mm",
     "url_scheme_util_unittest.mm",

ios/web/browser_state.mm

@@ -14,6 +14,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/process/process_handle.h"
 #include "ios/web/public/certificate_policy_cache.h"
+#include "ios/web/public/network_context_owner.h"
 #include "ios/web/public/service_manager_connection.h"
 #include "ios/web/public/service_names.mojom.h"
 #include "ios/web/public/web_client.h"
@@ -107,58 +108,6 @@ class BrowserStateServiceManagerConnectionHolder
 
 }  // namespace
 
-// 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 NetworkContextOwner(net::URLRequestContextGetter* request_context)
-      : request_context_(request_context) {
-    DCHECK_CURRENTLY_ON(WebThread::UI);
-  }
-
-  ~NetworkContextOwner() override {
-    DCHECK_CURRENTLY_ON(WebThread::IO);
-    if (request_context_)
-      request_context_->RemoveObserver(this);
-  }
-
-  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_->GetURLRequestContext());
-    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
 scoped_refptr<CertificatePolicyCache> BrowserState::GetCertificatePolicyCache(
     BrowserState* browser_state) {
@@ -214,15 +163,8 @@ network::mojom::URLLoaderFactory* BrowserState::GetURLLoaderFactory() {
     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_owner_ = std::make_unique<NetworkContextOwner>(
+        GetRequestContext(), &network_context_);
     network_context_->CreateURLLoaderFactory(
         mojo::MakeRequest(&url_loader_factory_), 0 /* process_id */);
   }

ios/web/network_context_owner.cc

+// Copyright 2017 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.
+
+#include "ios/web/public/network_context_owner.h"
+
+#include <memory>
+
+#include "ios/web/public/web_thread.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_context_getter_observer.h"
+#include "services/network/network_context.h"
+
+namespace web {
+
+NetworkContextOwner::NetworkContextOwner(
+    net::URLRequestContextGetter* request_context,
+    network::mojom::NetworkContextPtr* network_context_client)
+    : request_context_(request_context) {
+  DCHECK_CURRENTLY_ON(WebThread::UI);
+  web::WebThread::PostTask(
+      web::WebThread::IO, FROM_HERE,
+      base::BindOnce(&NetworkContextOwner::InitializeOnIOThread,
+                     // This is safe, since |this| will be deleted on the IO
+                     // thread, which would have to happen afterwards.
+                     base::Unretained(this),
+                     mojo::MakeRequest(network_context_client)));
+}
+
+NetworkContextOwner::~NetworkContextOwner() {
+  DCHECK_CURRENTLY_ON(WebThread::IO);
+  if (request_context_)
+    request_context_->RemoveObserver(this);
+}
+
+void NetworkContextOwner::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_->GetURLRequestContext());
+  request_context_->AddObserver(this);
+}
+
+void NetworkContextOwner::OnContextShuttingDown() {
+  DCHECK_CURRENTLY_ON(WebThread::IO);
+
+  // Cancels any pending requests owned by the NetworkContext.
+  network_context_.reset();
+
+  request_context_->RemoveObserver(this);
+  request_context_ = nullptr;
+}
+
+}  // namespace web

ios/web/network_context_owner_unittest.cc

+// 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.
+
+#include "ios/web/public/network_context_owner.h"
+
+#include "base/run_loop.h"
+#include "ios/web/public/test/test_web_thread_bundle.h"
+#include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_test_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+namespace web {
+
+class NetworkContextOwnerTest : public PlatformTest {
+ protected:
+  NetworkContextOwnerTest()
+      : saw_connection_error_(false),
+        context_getter_(base::MakeRefCounted<net::TestURLRequestContextGetter>(
+            WebThread::GetTaskRunnerForThread(WebThread::IO))) {}
+
+  ~NetworkContextOwnerTest() override {
+    // Tests should cleanup after themselves.
+    EXPECT_EQ(network_context_owner_.get(), nullptr);
+  }
+
+  void WatchForErrors() {
+    ASSERT_TRUE(network_context_.is_bound());
+    network_context_.set_connection_error_handler(base::BindOnce(
+        &NetworkContextOwnerTest::SawError, base::Unretained(this)));
+  }
+
+  void SawError() { saw_connection_error_ = true; }
+
+  bool saw_connection_error_;
+  TestWebThreadBundle test_web_thread_bundle_;
+  scoped_refptr<net::TestURLRequestContextGetter> context_getter_;
+  network::mojom::NetworkContextPtr network_context_;
+  std::unique_ptr<NetworkContextOwner> network_context_owner_;
+};
+
+// Test that NetworkContextOwner actually creates a NetworkContext owner and
+// connects a pipe to it, and destroys its end of the pipe when it's gone.
+TEST_F(NetworkContextOwnerTest, Basic) {
+  EXPECT_FALSE(network_context_.is_bound());
+  network_context_owner_ = std::make_unique<NetworkContextOwner>(
+      context_getter_.get(), &network_context_);
+  EXPECT_TRUE(network_context_.is_bound());
+  WatchForErrors();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(saw_connection_error_);
+
+  web::WebThread::DeleteSoon(web::WebThread::IO, FROM_HERE,
+                             network_context_owner_.release());
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(saw_connection_error_);  // other end gone
+}
+
+// Test to make sure that explicit shutdown of URLRequestContextGetter destroys
+// the NetworkContext object as expected.
+TEST_F(NetworkContextOwnerTest, ShutdownHandling) {
+  EXPECT_FALSE(network_context_.is_bound());
+  network_context_owner_ = std::make_unique<NetworkContextOwner>(
+      context_getter_.get(), &network_context_);
+  EXPECT_TRUE(network_context_.is_bound());
+  WatchForErrors();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(saw_connection_error_);
+
+  web::WebThread::PostTask(
+      web::WebThread::IO, FROM_HERE,
+      base::BindOnce(
+          &net::TestURLRequestContextGetter::NotifyContextShuttingDown,
+          context_getter_));
+
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(saw_connection_error_);  // other end gone post-shutdown.
+
+  web::WebThread::DeleteSoon(web::WebThread::IO, FROM_HERE,
+                             network_context_owner_.release());
+}
+
+}  // namespace web

ios/web/public/browser_state.h

@@ -32,6 +32,7 @@ class Connector;
 
 namespace web {
 class CertificatePolicyCache;
+class NetworkContextOwner;
 class ServiceManagerConnection;
 class URLDataManagerIOS;
 class URLDataManagerIOSBackend;
@@ -100,7 +101,6 @@ class BrowserState : public base::SupportsUserData {
                          const base::FilePath& path);
 
  private:
-  class NetworkContextOwner;
   friend class URLDataManagerIOS;
   friend class URLRequestChromeJob;
 

ios/web/public/network_context_owner.h

+// Copyright 2017 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 IOS_WEB_PUBLIC_NETWORK_CONTEXT_OWNER_H_
+#define IOS_WEB_PUBLIC_NETWORK_CONTEXT_OWNER_H_
+
+#include <memory>
+
+#include "ios/web/public/web_thread.h"
+#include "net/url_request/url_request_context_getter.h"
+#include "net/url_request/url_request_context_getter_observer.h"
+#include "services/network/network_context.h"
+
+namespace web {
+
+// Class that owns a NetworkContext wrapping the URLRequestContext.
+// This allows using the URLLoaderFactory and NetworkContext APIs
+// while still issuing requests with a URLRequestContext desired,
+// whether a standard //net one, or one created by a BrowserState subclass.
+//
+// Created on the UI thread on first use, so the UI-thread living objects
+// like 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 NetworkContextOwner : public net::URLRequestContextGetterObserver {
+ public:
+  // This initiates creation of the NetworkContext object on I/O thread and
+  // connects the pipe in |network_context_client| to it.
+  NetworkContextOwner(
+      net::URLRequestContextGetter* request_context,
+      network::mojom::NetworkContextPtr* network_context_client);
+
+  ~NetworkContextOwner() override;
+
+  // net::URLRequestContextGetterObserver implementation:
+  void OnContextShuttingDown() override;
+
+ private:
+  void InitializeOnIOThread(
+      network::mojom::NetworkContextRequest network_context_request);
+
+  scoped_refptr<net::URLRequestContextGetter> request_context_;
+  std::unique_ptr<network::NetworkContext> network_context_;
+};
+
+}  // namespace web
+
+#endif  // IOS_WEB_PUBLIC_NETWORK_CONTEXT_OWNER_H_

net/url_request/url_request_test_util.cc

@@ -165,11 +165,23 @@ TestURLRequestContextGetter::TestURLRequestContextGetter(
 TestURLRequestContextGetter::~TestURLRequestContextGetter() = default;
 
 TestURLRequestContext* TestURLRequestContextGetter::GetURLRequestContext() {
+  if (is_shut_down_)
+    return nullptr;
+
   if (!context_.get())
     context_.reset(new TestURLRequestContext);
   return context_.get();
 }
 
+void TestURLRequestContextGetter::NotifyContextShuttingDown() {
+  // This should happen before call to base NotifyContextShuttingDown() per that
+  // method's doc comments.
+  is_shut_down_ = true;
+
+  URLRequestContextGetter::NotifyContextShuttingDown();
+  context_ = nullptr;
+}
+
 scoped_refptr<base::SingleThreadTaskRunner>
 TestURLRequestContextGetter::GetNetworkTaskRunner() const {
   return network_task_runner_;

net/url_request/url_request_test_util.h

@@ -128,12 +128,16 @@ class TestURLRequestContextGetter : public URLRequestContextGetter {
   scoped_refptr<base::SingleThreadTaskRunner> GetNetworkTaskRunner()
       const override;
 
+  // see NotifyContextShuttingDown() in the base class.
+  void NotifyContextShuttingDown();
+
  protected:
   ~TestURLRequestContextGetter() override;
 
  private:
   const scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
   std::unique_ptr<TestURLRequestContext> context_;
+  bool is_shut_down_ = false;
 };
 
 //-----------------------------------------------------------------------------