Port dev tools WebUI from URLFetcher to SimpleURLLoader

This needed to be done at some point. The motivation here was to fix an
extensions WebRequest API test with the Network Service enabled.

This also plumbs through a signal so extensions API test JS code can
detect whether Network Service is enabled. This is used to support
conditional expectations in WebRequest tests since the test-only
event behavior may differ slightly between Network Service and old
networking.

Bug: 721414
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I009061f038eb3171721fe7645444695a77f3cfd3
Reviewed-on: https://chromium-review.googlesource.com/940447
Commit-Queue: Ken Rockot <rockot@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#540183}

chrome/browser/extensions/api/web_request/web_request_apitest.cc

@@ -47,6 +47,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/page_type.h"
 #include "content/public/test/browser_test_utils.h"
+#include "content/public/test/url_loader_interceptor.h"
 #include "extensions/browser/api/web_request/web_request_api.h"
 #include "extensions/browser/blocked_action_type.h"
 #include "extensions/browser/extension_system.h"
@@ -69,6 +70,7 @@
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_filter.h"
 #include "net/url_request/url_request_interceptor.h"
+#include "services/network/public/cpp/features.h"
 #include "third_party/WebKit/public/platform/WebInputEvent.h"
 
 #if defined(OS_CHROMEOS)
@@ -283,22 +285,33 @@ class DevToolsFrontendInWebRequestApiTest : public ExtensionApiTest {
     host_resolver()->AddRule("*", "127.0.0.1");
 
     int port = embedded_test_server()->port();
-    base::RunLoop run_loop;
-    content::BrowserThread::PostTaskAndReply(
-        content::BrowserThread::IO, FROM_HERE,
-        base::BindOnce(&SetUpDevToolsFrontendInterceptorOnIO, port,
-                       test_root_dir_),
-        run_loop.QuitClosure());
-    run_loop.Run();
+
+    if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+      url_loader_interceptor_ = std::make_unique<content::URLLoaderInterceptor>(
+          base::BindRepeating(&DevToolsFrontendInWebRequestApiTest::OnIntercept,
+                              base::Unretained(this), port));
+    } else {
+      base::RunLoop run_loop;
+      content::BrowserThread::PostTaskAndReply(
+          content::BrowserThread::IO, FROM_HERE,
+          base::BindOnce(&SetUpDevToolsFrontendInterceptorOnIO, port,
+                         test_root_dir_),
+          run_loop.QuitClosure());
+      run_loop.Run();
+    }
   }
 
   void TearDownOnMainThread() override {
-    base::RunLoop run_loop;
-    content::BrowserThread::PostTaskAndReply(
-        content::BrowserThread::IO, FROM_HERE,
-        base::BindOnce(&TearDownDevToolsFrontendInterceptorOnIO),
-        run_loop.QuitClosure());
-    run_loop.Run();
+    if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+      url_loader_interceptor_.reset();
+    } else {
+      base::RunLoop run_loop;
+      content::BrowserThread::PostTaskAndReply(
+          content::BrowserThread::IO, FROM_HERE,
+          base::BindOnce(&TearDownDevToolsFrontendInterceptorOnIO),
+          run_loop.QuitClosure());
+      run_loop.Run();
+    }
     ExtensionApiTest::TearDownOnMainThread();
   }
 
@@ -317,7 +330,49 @@ class DevToolsFrontendInWebRequestApiTest : public ExtensionApiTest {
   }
 
  private:
+  bool OnIntercept(int test_server_port,
+                   content::URLLoaderInterceptor::RequestParams* params) {
+    // See comments in DevToolsFrontendInterceptor above. The devtools remote
+    // frontend URLs are hardcoded into Chrome and are requested by some of the
+    // tests here to exercise their behavior with respect to WebRequest.
+    //
+    // We treat any URL request not targeting the test server as targeting the
+    // remote frontend, and we intercept them to fulfill from test data rather
+    // than hitting the network.
+    if (params->url_request.url.EffectiveIntPort() == test_server_port)
+      return false;
+
+    std::string status_line;
+    std::string contents;
+    GetFileContents(
+        test_root_dir_.AppendASCII(params->url_request.url.path().substr(1)),
+        &status_line, &contents);
+    content::URLLoaderInterceptor::WriteResponse(status_line, contents,
+                                                 params->client.get());
+    return true;
+  }
+
+  static void GetFileContents(const base::FilePath& path,
+                              std::string* status_line,
+                              std::string* contents) {
+    base::ScopedAllowBlockingForTesting allow_io;
+    if (!base::ReadFileToString(path, contents)) {
+      *status_line = "HTTP/1.0 404 Not Found\n\n";
+      return;
+    }
+
+    std::string content_type;
+    if (path.Extension() == FILE_PATH_LITERAL(".html"))
+      content_type = "Content-type: text/html\n";
+    else if (path.Extension() == FILE_PATH_LITERAL(".js"))
+      content_type = "Content-type: application/javascript\n";
+
+    *status_line =
+        base::StringPrintf("HTTP/1.0 200 OK\n%s\n", content_type.c_str());
+  }
+
   base::FilePath test_root_dir_;
+  std::unique_ptr<content::URLLoaderInterceptor> url_loader_interceptor_;
 };
 
 IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, WebRequestApi) {
@@ -1297,7 +1352,21 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, MinimumAccessInitiator) {
 
 // Ensure that devtools frontend requests are hidden from the webRequest API.
 IN_PROC_BROWSER_TEST_F(DevToolsFrontendInWebRequestApiTest, HiddenRequests) {
-  ASSERT_TRUE(RunExtensionSubtest("webrequest", "test_devtools.html"))
+  // Test expectations differ with the Network Service because of the way
+  // request interception is done for the test. In the legacy networking path a
+  // URLRequestMockHTTPJob is used, which does not generate
+  // |onBeforeHeadersSent| events. With the Network Service enabled, requests
+  // issued to HTTP URLs by these tests look like real HTTP requests and
+  // therefore do generate |onBeforeHeadersSent| events.
+  //
+  // These tests adjust their expectations accordingly based on whether or not
+  // the Network Service is enabled.
+  const char* network_service_arg =
+      base::FeatureList::IsEnabled(network::features::kNetworkService)
+          ? "NetworkServiceEnabled"
+          : "NetworkServiceDisabled";
+  ASSERT_TRUE(RunExtensionSubtestWithArg("webrequest", "test_devtools.html",
+                                         network_service_arg))
       << message_;
 }
 

chrome/browser/extensions/extension_apitest.cc

@@ -253,16 +253,34 @@ bool ExtensionApiTest::ExtensionSubtestsAreSkipped() {
 
 bool ExtensionApiTest::RunExtensionSubtest(const std::string& extension_name,
                                            const std::string& page_url) {
-  return RunExtensionSubtest(extension_name, page_url, kFlagEnableFileAccess);
+  return RunExtensionSubtestWithArgAndFlags(extension_name, page_url, nullptr,
+                                            kFlagEnableFileAccess);
 }
 
 bool ExtensionApiTest::RunExtensionSubtest(const std::string& extension_name,
                                            const std::string& page_url,
                                            int flags) {
+  return RunExtensionSubtestWithArgAndFlags(extension_name, page_url, nullptr,
+                                            flags);
+}
+
+bool ExtensionApiTest::RunExtensionSubtestWithArg(
+    const std::string& extension_name,
+    const std::string& page_url,
+    const char* custom_arg) {
+  return RunExtensionSubtestWithArgAndFlags(extension_name, page_url,
+                                            custom_arg, kFlagEnableFileAccess);
+}
+
+bool ExtensionApiTest::RunExtensionSubtestWithArgAndFlags(
+    const std::string& extension_name,
+    const std::string& page_url,
+    const char* custom_arg,
+    int flags) {
   DCHECK(!page_url.empty()) << "Argument page_url is required.";
   if (ExtensionSubtestsAreSkipped())
     return true;
-  return RunExtensionTestImpl(extension_name, page_url, NULL, flags);
+  return RunExtensionTestImpl(extension_name, page_url, custom_arg, flags);
 }
 
 bool ExtensionApiTest::RunPageTest(const std::string& page_url) {

chrome/browser/extensions/extension_apitest.h

@@ -128,6 +128,18 @@ class ExtensionApiTest : public ExtensionBrowserTest {
                            const std::string& page_url,
                            int flags);
 
+  // As above but with support for injecting a custom argument into the test
+  // config.
+  bool RunExtensionSubtestWithArg(const std::string& extension_name,
+                                  const std::string& page_url,
+                                  const char* custom_arg);
+
+  // As above but with support for custom flags defined in Flags above.
+  bool RunExtensionSubtestWithArgAndFlags(const std::string& extension_name,
+                                          const std::string& page_url,
+                                          const char* custom_arg,
+                                          int flags);
+
   // Load |page_url| and wait for pass / fail notification from the extension
   // API on the page.
   bool RunPageTest(const std::string& page_url);

chrome/browser/ui/webui/devtools_ui.cc

@@ -4,17 +4,21 @@
 
 #include "chrome/browser/ui/webui/devtools_ui.h"
 
+#include <list>
+#include <utility>
+
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/devtools/url_constants.h"
-#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/url_constants.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/devtools_frontend_host.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/url_data_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
@@ -22,9 +26,8 @@
 #include "net/base/filename_util.h"
 #include "net/base/load_flags.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
-#include "net/url_request/url_fetcher.h"
-#include "net/url_request/url_fetcher_delegate.h"
-#include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/resource_request.h"
+#include "services/network/public/cpp/simple_url_loader.h"
 #include "third_party/WebKit/public/public_features.h"
 
 using content::BrowserThread;
@@ -37,7 +40,11 @@ std::string PathWithoutParams(const std::string& path) {
       .path().substr(1);
 }
 
-const char kHttpNotFound[] = "HTTP/1.1 404 Not Found\n\n";
+scoped_refptr<base::RefCountedMemory> CreateNotFoundResponse() {
+  const char kHttpNotFound[] = "HTTP/1.1 404 Not Found\n\n";
+  return base::MakeRefCounted<base::RefCountedStaticMemory>(
+      kHttpNotFound, strlen(kHttpNotFound));
+}
 
 // DevToolsDataSource ---------------------------------------------------------
 
@@ -73,12 +80,13 @@ std::string GetMimeTypeForPath(const std::string& path) {
 // 2. /remote/: remote DevTools frontend is served from App Engine.
 // 3. /custom/: custom DevTools frontend is served from the server as specified
 //    by the --custom-devtools-frontend flag.
-class DevToolsDataSource : public content::URLDataSource,
-                           public net::URLFetcherDelegate {
+class DevToolsDataSource : public content::URLDataSource {
  public:
   using GotDataCallback = content::URLDataSource::GotDataCallback;
 
-  explicit DevToolsDataSource(net::URLRequestContextGetter* request_context);
+  explicit DevToolsDataSource(
+      scoped_refptr<content::SharedURLLoaderFactory> url_loader_factory)
+      : url_loader_factory_(std::move(url_loader_factory)) {}
 
   // content::URLDataSource implementation.
   std::string GetSource() const override;
@@ -89,14 +97,18 @@ class DevToolsDataSource : public content::URLDataSource,
       const GotDataCallback& callback) override;
 
  private:
+  struct PendingRequest;
+
+  ~DevToolsDataSource() override = default;
+
   // content::URLDataSource overrides.
   std::string GetMimeType(const std::string& path) const override;
   bool ShouldAddContentSecurityPolicy() const override;
   bool ShouldDenyXFrameOptions() const override;
   bool ShouldServeMimeTypeAsContentTypeHeader() const override;
 
-  // net::URLFetcherDelegate overrides.
-  void OnURLFetchComplete(const net::URLFetcher* source) override;
+  void OnLoadComplete(std::list<PendingRequest>::iterator request_iter,
+                      std::unique_ptr<std::string> response_body);
 
   // Serves bundled DevTools frontend from ResourceBundle.
   void StartBundledDataRequest(const std::string& path,
@@ -110,28 +122,33 @@ class DevToolsDataSource : public content::URLDataSource,
   void StartCustomDataRequest(const GURL& url,
                               const GotDataCallback& callback);
 
-  ~DevToolsDataSource() override;
+  void StartNetworkRequest(
+      const GURL& url,
+      const net::NetworkTrafficAnnotationTag& traffic_annotation,
+      int load_flags,
+      const GotDataCallback& callback);
 
-  scoped_refptr<net::URLRequestContextGetter> request_context_;
+  struct PendingRequest {
+    PendingRequest() = default;
+    PendingRequest(PendingRequest&& other) = default;
+    PendingRequest& operator=(PendingRequest&& other) = default;
 
-  using PendingRequestsMap = std::map<const net::URLFetcher*, GotDataCallback>;
-  PendingRequestsMap pending_;
+    ~PendingRequest() {
+      if (callback)
+        callback.Run(CreateNotFoundResponse());
+    }
 
-  DISALLOW_COPY_AND_ASSIGN(DevToolsDataSource);
-};
+    GotDataCallback callback;
+    std::unique_ptr<network::SimpleURLLoader> loader;
 
-DevToolsDataSource::DevToolsDataSource(
-    net::URLRequestContextGetter* request_context)
-    : request_context_(request_context) {
-}
+    DISALLOW_COPY_AND_ASSIGN(PendingRequest);
+  };
 
-DevToolsDataSource::~DevToolsDataSource() {
-  for (const auto& pair : pending_) {
-    delete pair.first;
-    pair.second.Run(
-        new base::RefCountedStaticMemory(kHttpNotFound, strlen(kHttpNotFound)));
-  }
-}
+  scoped_refptr<content::SharedURLLoaderFactory> url_loader_factory_;
+  std::list<PendingRequest> pending_requests_;
+
+  DISALLOW_COPY_AND_ASSIGN(DevToolsDataSource);
+};
 
 std::string DevToolsDataSource::GetSource() const {
   return chrome::kChromeUIDevToolsHost;
@@ -171,8 +188,7 @@ void DevToolsDataSource::StartDataRequest(
       StartRemoteDataRequest(url, callback);
     } else {
       DLOG(ERROR) << "Refusing to load invalid remote front-end URL";
-      callback.Run(new base::RefCountedStaticMemory(kHttpNotFound,
-                                                    strlen(kHttpNotFound)));
+      callback.Run(CreateNotFoundResponse());
     }
     return;
   }
@@ -261,20 +277,15 @@ void DevToolsDataSource::StartRemoteDataRequest(
             }
           }
         })");
-  net::URLFetcher* fetcher = net::URLFetcher::Create(url, net::URLFetcher::GET,
-                                                     this, traffic_annotation)
-                                 .release();
-  pending_[fetcher] = callback;
-  fetcher->SetRequestContext(request_context_.get());
-  fetcher->Start();
+
+  StartNetworkRequest(url, traffic_annotation, net::LOAD_NORMAL, callback);
 }
 
 void DevToolsDataSource::StartCustomDataRequest(
     const GURL& url,
     const content::URLDataSource::GotDataCallback& callback) {
   if (!url.is_valid()) {
-    callback.Run(
-        new base::RefCountedStaticMemory(kHttpNotFound, strlen(kHttpNotFound)));
+    callback.Run(CreateNotFoundResponse());
     return;
   }
   net::NetworkTrafficAnnotationTag traffic_annotation =
@@ -305,24 +316,38 @@ void DevToolsDataSource::StartCustomDataRequest(
             }
           }
         })");
-  net::URLFetcher* fetcher = net::URLFetcher::Create(url, net::URLFetcher::GET,
-                                                     this, traffic_annotation)
-                                 .release();
-  pending_[fetcher] = callback;
-  fetcher->SetRequestContext(request_context_.get());
-  fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE);
-  fetcher->Start();
+
+  StartNetworkRequest(url, traffic_annotation, net::LOAD_DISABLE_CACHE,
+                      callback);
 }
 
-void DevToolsDataSource::OnURLFetchComplete(const net::URLFetcher* source) {
-  DCHECK(source);
-  PendingRequestsMap::iterator it = pending_.find(source);
-  DCHECK(it != pending_.end());
-  std::string response;
-  source->GetResponseAsString(&response);
-  delete source;
-  it->second.Run(base::RefCountedString::TakeString(&response));
-  pending_.erase(it);
+void DevToolsDataSource::StartNetworkRequest(
+    const GURL& url,
+    const net::NetworkTrafficAnnotationTag& traffic_annotation,
+    int load_flags,
+    const GotDataCallback& callback) {
+  auto request = std::make_unique<network::ResourceRequest>();
+  request->url = url;
+  request->load_flags = load_flags;
+
+  auto request_iter = pending_requests_.emplace(pending_requests_.begin());
+  request_iter->callback = callback;
+  request_iter->loader =
+      network::SimpleURLLoader::Create(std::move(request), traffic_annotation);
+  request_iter->loader->DownloadToStringOfUnboundedSizeUntilCrashAndDie(
+      url_loader_factory_.get(),
+      base::BindOnce(&DevToolsDataSource::OnLoadComplete,
+                     base::Unretained(this), request_iter));
+}
+
+void DevToolsDataSource::OnLoadComplete(
+    std::list<PendingRequest>::iterator request_iter,
+    std::unique_ptr<std::string> response_body) {
+  std::move(request_iter->callback)
+      .Run(response_body
+               ? base::RefCountedString::TakeString(response_body.get())
+               : CreateNotFoundResponse());
+  pending_requests_.erase(request_iter);
 }
 
 }  // namespace
@@ -377,11 +402,11 @@ bool DevToolsUI::IsFrontendResourceURL(const GURL& url) {
 DevToolsUI::DevToolsUI(content::WebUI* web_ui)
     : WebUIController(web_ui), bindings_(web_ui->GetWebContents()) {
   web_ui->SetBindings(0);
-  Profile* profile = Profile::FromWebUI(web_ui);
-  content::URLDataSource::Add(
-      profile,
-      new DevToolsDataSource(profile->GetRequestContext()));
+  auto factory = content::BrowserContext::GetDefaultStoragePartition(
+                     web_ui->GetWebContents()->GetBrowserContext())
+                     ->GetURLLoaderFactoryForBrowserProcess();
+  content::URLDataSource::Add(web_ui->GetWebContents()->GetBrowserContext(),
+                              new DevToolsDataSource(std::move(factory)));
 }
 
-DevToolsUI::~DevToolsUI() {
-}
+DevToolsUI::~DevToolsUI() = default;

chrome/test/data/extensions/api_test/webrequest/framework.js

@@ -8,6 +8,7 @@ var expectedEventData;
 var capturedEventData;
 var capturedUnexpectedData;
 var expectedEventOrder;
+var networkServiceState = "unknown";
 var tabId;
 var tabIdMap;
 var frameIdMap;
@@ -58,14 +59,22 @@ function runTestsForTab(tests, tab) {
 
 // Creates an "about:blank" tab and runs |tests| with this tab as default.
 function runTests(tests) {
-  var waitForAboutBlank = function(_, info, tab) {
-    if (info.status == "complete" && tab.url == "about:blank") {
-      chrome.tabs.onUpdated.removeListener(waitForAboutBlank);
-      runTestsForTab(tests, tab);
-    }
-  };
-  chrome.tabs.onUpdated.addListener(waitForAboutBlank);
-  chrome.tabs.create({url: "about:blank"});
+  chrome.test.getConfig(function(config) {
+    var waitForAboutBlank = function(_, info, tab) {
+      if (info.status == "complete" && tab.url == "about:blank") {
+        chrome.tabs.onUpdated.removeListener(waitForAboutBlank);
+        runTestsForTab(tests, tab);
+      }
+    };
+
+    if (config.customArg === "NetworkServiceEnabled")
+      networkServiceState = "enabled";
+    else if (config.customArg === "NetworkServiceDisabled")
+      networkServiceState = "disabled";
+
+    chrome.tabs.onUpdated.addListener(waitForAboutBlank);
+    chrome.tabs.create({url: "about:blank"});
+  });
 }
 
 // Returns an URL from the test server, fixing up the port. Must be called
@@ -140,6 +149,24 @@ function expect(data, order, filter, extraInfoSpec) {
   capturedEventData = [];
   capturedUnexpectedData = [];
   expectedEventOrder = order || [];
+
+  expectedEventData = expectedEventData.filter(function(event) {
+    if (!event.details.requiredNetworkServiceState)
+      return true;
+
+    if (networkServiceState == "unknown") {
+      chrome.test.fail("Test expectations specify a Network Service " +
+          "requirement, but the Network Service was neither explicitly set " +
+          "as enabled or disabled by the test runner. This test should be " +
+          "run with the custom argument NetworkServiceEnabled or " +
+          "NetworkServiceDisabled.");
+    }
+
+    var requiredState = event.details.requiredNetworkServiceState;
+    delete event.details.requiredNetworkServiceState;
+    return networkServiceState === requiredState;
+  });
+
   if (expectedEventData.length > 0) {
     eventsCaptured = chrome.test.callbackAdded();
   }

chrome/test/data/extensions/api_test/webrequest/test_devtools.js

@@ -221,6 +221,38 @@ function expectMockedTabNavigationEvents(url) {
             initiator: getServerDomain(initiators.BROWSER_INITIATED)
           }
         },
+        {
+          label: 'onBeforeSendHeaders-1',
+          event: 'onBeforeSendHeaders',
+          details: {
+            type: 'main_frame',
+            url,
+            initiator: getServerDomain(initiators.BROWSER_INITIATED),
+            requiredNetworkServiceState: "enabled"
+          }
+        },
+        {
+          label: 'onSendHeaders-1',
+          event: 'onSendHeaders',
+          details: {
+            type: 'main_frame',
+            url,
+            initiator: getServerDomain(initiators.BROWSER_INITIATED),
+            requiredNetworkServiceState: "enabled"
+          }
+        },
+        {
+          label: 'onHeadersReceived-1',
+          event: 'onHeadersReceived',
+          details: {
+            type: 'main_frame',
+            url,
+            statusCode: 200,
+            statusLine: 'HTTP/1.0 200 OK',
+            initiator: getServerDomain(initiators.BROWSER_INITIATED),
+            requiredNetworkServiceState: "enabled"
+          }
+        },
         {
           label: 'onResponseStarted-1',
           event: 'onResponseStarted',
@@ -257,6 +289,38 @@ function expectMockedTabNavigationEvents(url) {
             initiator: frontendOrigin
           }
         },
+        {
+          label: 'onBeforeSendHeaders-2',
+          event: 'onBeforeSendHeaders',
+          details: {
+            type: 'script',
+            url: scriptUrl,
+            initiator: frontendOrigin,
+            requiredNetworkServiceState: "enabled"
+          }
+        },
+        {
+          label: 'onSendHeaders-2',
+          event: 'onSendHeaders',
+          details: {
+            type: 'script',
+            url: scriptUrl,
+            initiator: frontendOrigin,
+            requiredNetworkServiceState: "enabled"
+          }
+        },
+        {
+          label: 'onHeadersReceived-2',
+          event: 'onHeadersReceived',
+          details: {
+            type: 'script',
+            url: scriptUrl,
+            statusCode: 200,
+            statusLine: 'HTTP/1.0 200 OK',
+            initiator: frontendOrigin,
+            requiredNetworkServiceState: "enabled"
+          }
+        },
         {
           label: 'onResponseStarted-2',
           event: 'onResponseStarted',

content/public/test/url_loader_interceptor.cc

@@ -11,6 +11,7 @@
 #include "content/browser/storage_partition_impl.h"
 #include "content/browser/url_loader_factory_getter.h"
 #include "content/public/browser/browser_thread.h"
+#include "mojo/public/cpp/bindings/binding_set.h"
 #include "net/http/http_util.h"
 #include "services/network/public/cpp/features.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
@@ -29,10 +30,21 @@ class URLLoaderInterceptor::Interceptor
               const OriginalFactoryGetter& original_factory_getter)
       : parent_(parent),
         process_id_getter_(process_id_getter),
-        original_factory_getter_(original_factory_getter) {}
+        original_factory_getter_(original_factory_getter) {
+    bindings_.set_connection_error_handler(base::BindRepeating(
+        &Interceptor::OnConnectionError, base::Unretained(this)));
+  }
 
   ~Interceptor() override {}
 
+  void BindRequest(network::mojom::URLLoaderFactoryRequest request) {
+    bindings_.AddBinding(this, std::move(request));
+  }
+
+  void SetConnectionErrorHandler(base::OnceClosure handler) {
+    error_handler_ = std::move(handler);
+  }
+
  private:
   // network::mojom::URLLoaderFactory implementation:
   void CreateLoaderAndStart(network::mojom::URLLoaderRequest request,
@@ -78,12 +90,19 @@ class URLLoaderInterceptor::Interceptor
   }
 
   void Clone(network::mojom::URLLoaderFactoryRequest request) override {
-    NOTREACHED();
+    BindRequest(std::move(request));
+  }
+
+  void OnConnectionError() {
+    if (bindings_.empty() && error_handler_)
+      std::move(error_handler_).Run();
   }
 
   URLLoaderInterceptor* parent_;
   ProcessIdGetter process_id_getter_;
   OriginalFactoryGetter original_factory_getter_;
+  mojo::BindingSet<network::mojom::URLLoaderFactory> bindings_;
+  base::OnceClosure error_handler_;
 
   DISALLOW_COPY_AND_ASSIGN(Interceptor);
 };
@@ -128,8 +147,9 @@ class URLLoaderInterceptor::BrowserProcessWrapper {
             base::BindRepeating([]() { return 0; }),
             base::BindRepeating(&BrowserProcessWrapper::GetOriginalFactory,
                                 base::Unretained(this))),
-        binding_(&interceptor_, std::move(factory_request)),
-        original_factory_(std::move(original_factory)) {}
+        original_factory_(std::move(original_factory)) {
+    interceptor_.BindRequest(std::move(factory_request));
+  }
 
   ~BrowserProcessWrapper() {}
 
@@ -139,7 +159,6 @@ class URLLoaderInterceptor::BrowserProcessWrapper {
   }
 
   Interceptor interceptor_;
-  mojo::Binding<network::mojom::URLLoaderFactory> binding_;
   network::mojom::URLLoaderFactoryPtr original_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserProcessWrapper);
@@ -159,9 +178,9 @@ class URLLoaderInterceptor::SubresourceWrapper {
                                 process_id),
             base::BindRepeating(&SubresourceWrapper::GetOriginalFactory,
                                 base::Unretained(this))),
-        binding_(&interceptor_, std::move(factory_request)),
         original_factory_(std::move(original_factory)) {
-    binding_.set_connection_error_handler(
+    interceptor_.BindRequest(std::move(factory_request));
+    interceptor_.SetConnectionErrorHandler(
         base::BindOnce(&URLLoaderInterceptor::SubresourceWrapperBindingError,
                        base::Unretained(parent), this));
   }
@@ -174,7 +193,6 @@ class URLLoaderInterceptor::SubresourceWrapper {
   }
 
   Interceptor interceptor_;
-  mojo::Binding<network::mojom::URLLoaderFactory> binding_;
   network::mojom::URLLoaderFactoryPtr original_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SubresourceWrapper);

testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

@@ -241,7 +241,6 @@
 
 # http://crbug.com/721414
 # TODO(rockot): add support for webRequest API.
--DevToolsFrontendInWebRequestApiTest.HiddenRequests
 -ExtensionWebRequestApiTest.PostData1
 -ExtensionWebRequestApiTest.PostData2
 -ExtensionWebRequestApiTest.WebRequestBlocking