Fixes DevToolsFrontendInWebRequestApiTest.HiddenRequests with network service

This test was timing out with network service enabled, due to the
difference in behavior of the web request API in the network service.
With network service enabled, when the first web request rule or
listener is added, we reset all current URLLoaderFactories so they will
be proxied through WebRequestAPI. They are then rebound when the
connection error handler runs. This can lead to a situation where
messages are dropped, due to the error message arriving asynchronously.
For example:

(1) First web request listener added, URLLoaderFactory bindings cleared
(2) CreateLoaderAndStart is called on an existing URLLoaderFactory
(3) Error message reaches existing URLLoaderFactories, and they are reset

Due to the way mojo pipes work, the message from (2) will be dropped.
This change makes that much less likely by clearing the URLLoaderFactory
in the storage partition directly instead of waiting for the error
handler to be called.

Bug: 721414
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I633e3ea933f9170767371fde697bc9de2a5258af
Reviewed-on: https://chromium-review.googlesource.com/1147072Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577716}

chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc

@@ -1632,15 +1632,6 @@ IN_PROC_BROWSER_TEST_P(DeclarativeNetRequestBrowserTest,
   request->resource_type = content::ResourceType::RESOURCE_TYPE_SCRIPT;
   request->render_frame_id = MSG_ROUTING_NONE;
 
-  // TODO(https://crbug.com/857577): remove this hack. When an unrelated
-  // browser issued request (typically from GaiaAuthFetcher) has run, it causes
-  // the StoragePartitionImpl to create and cache a URLLoaderFactory without the
-  // web request proxying. This resets it so one with the web request proxying
-  // is created the next time a request is made.
-  base::RunLoop().RunUntilIdle();
-  content::BrowserContext::GetDefaultStoragePartition(profile())
-      ->ResetURLLoaderFactoryForBrowserProcessForTesting();
-
   auto loader = network::SimpleURLLoader::Create(std::move(request),
                                                  TRAFFIC_ANNOTATION_FOR_TESTS);
   content::SimpleURLLoaderTestHelper loader_helper;

content/browser/storage_partition_impl.cc

@@ -1164,6 +1164,11 @@ void StoragePartitionImpl::Flush() {
     GetDOMStorageContext()->Flush();
 }
 
+void StoragePartitionImpl::ResetURLLoaderFactories() {
+  GetNetworkContext()->ResetURLLoaderFactories();
+  url_loader_factory_for_browser_process_.reset();
+}
+
 void StoragePartitionImpl::ClearBluetoothAllowedDevicesMapForTesting() {
   bluetooth_allowed_devices_map_->Clear();
 }
@@ -1183,10 +1188,6 @@ void StoragePartitionImpl::WaitForDeletionTasksForTesting() {
   }
 }
 
-void StoragePartitionImpl::ResetURLLoaderFactoryForBrowserProcessForTesting() {
-  url_loader_factory_for_browser_process_.reset();
-}
-
 BrowserContext* StoragePartitionImpl::browser_context() const {
   return browser_context_;
 }

content/browser/storage_partition_impl.h

@@ -129,10 +129,10 @@ class CONTENT_EXPORT StoragePartitionImpl
       const base::Callback<bool(const GURL&)>& url_matcher,
       base::OnceClosure callback) override;
   void Flush() override;
+  void ResetURLLoaderFactories() override;
   void ClearBluetoothAllowedDevicesMapForTesting() override;
   void FlushNetworkInterfaceForTesting() override;
   void WaitForDeletionTasksForTesting() override;
-  void ResetURLLoaderFactoryForBrowserProcessForTesting() override;
 
   BackgroundFetchContext* GetBackgroundFetchContext();
   BackgroundSyncContext* GetBackgroundSyncContext();

content/public/browser/storage_partition.h

@@ -214,6 +214,9 @@ class CONTENT_EXPORT StoragePartition {
   // unwritten data has been written out to the filesystem.
   virtual void Flush() = 0;
 
+  // Resets all URLLoaderFactories bound to this partition's network context.
+  virtual void ResetURLLoaderFactories() = 0;
+
   // Clear the bluetooth allowed devices map. For test use only.
   virtual void ClearBluetoothAllowedDevicesMapForTesting() = 0;
 
@@ -224,10 +227,6 @@ class CONTENT_EXPORT StoragePartition {
   // Wait until all deletions tasks are finished. For test use only.
   virtual void WaitForDeletionTasksForTesting() = 0;
 
-  // Used in tests to force the cached SharedURLLoaderFactory to be dropped, as
-  // a way to work-around https://crbug.com/857577.
-  virtual void ResetURLLoaderFactoryForBrowserProcessForTesting() {}
-
  protected:
   virtual ~StoragePartition() {}
 };

content/public/test/test_storage_partition.cc

@@ -131,6 +131,8 @@ void TestStoragePartition::ClearHttpAndMediaCaches(
 
 void TestStoragePartition::Flush() {}
 
+void TestStoragePartition::ResetURLLoaderFactories() {}
+
 void TestStoragePartition::ClearBluetoothAllowedDevicesMapForTesting() {}
 
 void TestStoragePartition::FlushNetworkInterfaceForTesting() {}

content/public/test/test_storage_partition.h

@@ -166,6 +166,8 @@ class TestStoragePartition : public StoragePartition {
 
   void Flush() override;
 
+  void ResetURLLoaderFactories() override;
+
   void ClearBluetoothAllowedDevicesMapForTesting() override;
   void FlushNetworkInterfaceForTesting() override;
   void WaitForDeletionTasksForTesting() override;

extensions/browser/api/web_request/web_request_api.cc

@@ -644,7 +644,6 @@ void WebRequestAPI::UpdateMayHaveProxies() {
   bool may_have_proxies = MayHaveProxies();
   if (!may_have_proxies_ && may_have_proxies) {
     content::BrowserContext::GetDefaultStoragePartition(browser_context_)
-        ->GetNetworkContext()
         ->ResetURLLoaderFactories();
   }
   may_have_proxies_ = may_have_proxies;

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

@@ -208,8 +208,6 @@
 # TODO(rockot): add support for webRequest API.
 -ExtensionWebRequestApiTest.WebRequestBlocking
 -ExtensionWebRequestApiTest.WebRequestTypes
-# Needs synchronization to wait until after URLLoaderFactories are rebound.
--DevToolsFrontendInWebRequestApiTest.HiddenRequests
 # Note WebRequestUnloadImmediately is disabled on Linux
 -ExtensionWebRequestApiTest.WebRequestUnloadImmediately