Reland "Reset URLLoaderFactory bindings for web request proxy"

This is a reland of 574add02

Added DevToolsFrontendInWebRequestApiTest.HiddenRequests to the filter
file, will fix it in a follow up.

Original change's description:
> Reset URLLoaderFactory bindings for web request proxy
>
> Previously, if a request was made before any web request listeners or
> rules were added, the URLLoaderFactory would not be proxied through the
> browser process to run the WebRequest code. Now, if it is detected that
> WebRequest listeners/rules are added, we reset the bindings so they will
> be recreated and proxied through the browser process.
>
> Observers were added for a few classes so WebRequestAPI can listen to
> rule changes.
>
> Changes in URLLoaderFactoryGetter were needed due to crbug.com/613371.
>
> Bug: 857577
> Cq-Include-Trybots: luci.chromium.try:linux_mojo
> Change-Id: I7805be86512545b496e30b9693374981fdc2633e
> Reviewed-on: https://chromium-review.googlesource.com/1139048
> Commit-Queue: Clark DuVall <cduvall@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Reviewed-by: Ken Rockot <rockot@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#577036}

TBR=rockot@chromium.org,kinuko@chromium.org

Bug: 857577
Change-Id: I83f266cfcd572ccbde36405d7cff501f92122b2d
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Reviewed-on: https://chromium-review.googlesource.com/1147042Reviewed-by: Clark DuVall <cduvall@chromium.org>
Commit-Queue: Clark DuVall <cduvall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577310}

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

@@ -268,15 +268,17 @@ class ExtensionWebRequestApiTest : public ExtensionApiTest {
       const char* expected_content_regular_window,
       const char* exptected_content_incognito_window);
 
-  // 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.
-  void ResetStoragePartitionURLLoaderFactory() {
-    base::RunLoop().RunUntilIdle();
+  network::mojom::URLLoaderFactoryPtr CreateURLLoaderFactory() {
+    network::mojom::URLLoaderFactoryParamsPtr params =
+        network::mojom::URLLoaderFactoryParams::New();
+    params->process_id = network::mojom::kBrowserProcessId;
+    params->is_corb_enabled = false;
+    network::mojom::URLLoaderFactoryPtr loader_factory;
     content::BrowserContext::GetDefaultStoragePartition(profile())
-        ->ResetURLLoaderFactoryForBrowserProcessForTesting();
+        ->GetNetworkContext()
+        ->CreateURLLoaderFactory(mojo::MakeRequest(&loader_factory),
+                                 std::move(params));
+    return loader_factory;
   }
 };
 
@@ -1039,9 +1041,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
     EXPECT_EQ(200, loader->ResponseInfo()->headers->response_code());
   };
 
-  // TODO(https://crbug.com/857577): remove this call.
-  ResetStoragePartitionURLLoaderFactory();
-
   // Now perform a request to "client1.google.com" from the browser process.
   // This should *not* be visible to the WebRequest API. We should still have
   // only seen the single render-initiated request from the first half of the
@@ -1362,9 +1361,6 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
         }
       };
 
-  // TODO(https://crbug.com/857577): remove this call.
-  ResetStoragePartitionURLLoaderFactory();
-
   // Next, try a series of requests through URLRequestFetchers (rather than a
   // renderer).
   auto* url_loader_factory =
@@ -1452,6 +1448,37 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, MinimumAccessInitiator) {
   }
 }
 
+IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
+                       WebRequestApiClearsBindingOnFirstListener) {
+  if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
+  auto loader_factory = CreateURLLoaderFactory();
+  bool has_connection_error = false;
+  loader_factory.set_connection_error_handler(
+      base::BindLambdaForTesting([&]() { has_connection_error = true; }));
+
+  auto* web_request_api =
+      extensions::BrowserContextKeyedAPIFactory<extensions::WebRequestAPI>::Get(
+          profile());
+  web_request_api->OnListenerAdded(
+      EventListenerInfo("name", "id1", GURL(), profile()));
+  content::BrowserContext::GetDefaultStoragePartition(profile())
+      ->FlushNetworkInterfaceForTesting();
+  EXPECT_TRUE(has_connection_error);
+
+  // The second time there should be no connection error.
+  loader_factory = CreateURLLoaderFactory();
+  has_connection_error = false;
+  loader_factory.set_connection_error_handler(
+      base::BindLambdaForTesting([&]() { has_connection_error = true; }));
+  web_request_api->OnListenerAdded(
+      EventListenerInfo("name", "id2", GURL(), profile()));
+  content::BrowserContext::GetDefaultStoragePartition(profile())
+      ->FlushNetworkInterfaceForTesting();
+  EXPECT_FALSE(has_connection_error);
+}
+
 // Test fixture which sets a custom NTP Page.
 class NTPInterceptionWebRequestAPITest : public ExtensionApiTest {
  public:

chrome/browser/extensions/background_xhr_browsertest.cc

@@ -15,6 +15,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/network_service_instance.h"
 #include "extensions/browser/browsertest_util.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_urls.h"
@@ -59,6 +60,16 @@ class BackgroundXhrTest : public ExtensionBrowserTest {
     GURL test_url = net::AppendQueryParameter(extension->GetResourceURL(path),
                                               "url", url.spec());
     ui_test_utils::NavigateToURL(browser(), test_url);
+    content::FlushNetworkServiceInstanceForTesting();
+    constexpr char kSendXHRScript[] = R"(
+      var xhr = new XMLHttpRequest();
+      xhr.open('GET', '%s');
+      xhr.send();
+      domAutomationController.send('');
+    )";
+    browsertest_util::ExecuteScriptInBackgroundPage(
+        profile(), extension->id(),
+        base::StringPrintf(kSendXHRScript, url.spec().c_str()));
     ASSERT_TRUE(catcher.GetNextResult());
   }
 };

chrome/test/data/extensions/background_xhr/background.js

 // Copyright 2015 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.
-
-chrome.runtime.onMessage.addListener(function(message, sender, sendResponse) {
-  if (message.type == "xhr") {
-    var xhr = new XMLHttpRequest();
-    xhr.open(message.method, message.url);
-    xhr.send();
-  } else {
-    console.error("Unknown message: " + JSON.stringify(message));
-  }
-});

chrome/test/data/extensions/background_xhr/test_http_auth.js

@@ -2,9 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// TODO(davidben): When URLSearchParams is stable and implemented, switch this
-// (and a lot of other test code) to it. https://crbug.com/303152
-var url = decodeURIComponent(/url=([^&]*)/.exec(location.search)[1]);
+var url = new URL(location.href).searchParams.get('url');
 var filter = {urls: [url], types: ["xmlhttprequest"]};
 
 chrome.webRequest.onCompleted.addListener(function(details) {
@@ -19,5 +17,3 @@ chrome.webRequest.onCompleted.addListener(function(details) {
 chrome.webRequest.onErrorOccurred.addListener(function(details) {
   chrome.test.notifyFail("Request failed");
 }, filter);
-
-chrome.runtime.sendMessage({type: "xhr", method: "GET", url: url});

chrome/test/data/extensions/background_xhr/test_tls_client_auth.js

@@ -2,9 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// TODO(davidben): When URLSearchParams is stable and implemented, switch this
-// (and a lot of other test code) to it. https://crbug.com/303152
-var url = decodeURIComponent(/url=([^&]*)/.exec(location.search)[1]);
+var url = new URL(location.href).searchParams.get('url');
 var filter = {urls: [url], types: ["xmlhttprequest"]};
 
 chrome.webRequest.onCompleted.addListener(function(details) {
@@ -19,5 +17,3 @@ chrome.webRequest.onErrorOccurred.addListener(function(details) {
 
   chrome.test.notifyPass();
 }, filter);
-
-chrome.runtime.sendMessage({type: "xhr", method: "GET", url: url});

content/browser/loader/navigation_url_loader_impl_unittest.cc

@@ -145,6 +145,10 @@ class NavigationURLLoaderImplTest : public testing::Test {
   }
 
   ~NavigationURLLoaderImplTest() override {
+    // The context needs to be deleted before ServiceManagerConnection is
+    // destroyed, so the storage partition in the context does not try to
+    // reconnect to the network service after ServiceManagerConnection is dead.
+    browser_context_.reset();
     ServiceManagerConnection::DestroyForProcess();
   }
 

content/browser/storage_partition_impl.cc

@@ -688,23 +688,8 @@ StoragePartitionImpl::GetMediaURLRequestContext() {
 }
 
 network::mojom::NetworkContext* StoragePartitionImpl::GetNetworkContext() {
-  if (!network_context_.is_bound() || network_context_.encountered_error()) {
-    network_context_ = GetContentClient()->browser()->CreateNetworkContext(
-        browser_context_, is_in_memory_, relative_partition_path_);
-    if (!network_context_) {
-      // TODO(mmenke): Remove once https://crbug.com/827928 is fixed.
-      CHECK(url_request_context_);
-
-      DCHECK(!base::FeatureList::IsEnabled(network::features::kNetworkService));
-      DCHECK(!network_context_owner_);
-      network_context_owner_ = std::make_unique<NetworkContextOwner>();
-      BrowserThread::PostTask(
-          BrowserThread::IO, FROM_HERE,
-          base::BindOnce(&NetworkContextOwner::Initialize,
-                         base::Unretained(network_context_owner_.get()),
-                         MakeRequest(&network_context_), url_request_context_));
-    }
-  }
+  if (!network_context_.is_bound())
+    InitNetworkContext();
   return network_context_.get();
 }
 
@@ -1238,6 +1223,26 @@ void StoragePartitionImpl::GetQuotaSettings(
                                                   std::move(callback));
 }
 
+void StoragePartitionImpl::InitNetworkContext() {
+  network_context_ = GetContentClient()->browser()->CreateNetworkContext(
+      browser_context_, is_in_memory_, relative_partition_path_);
+  if (!network_context_) {
+    // TODO(mmenke): Remove once https://crbug.com/827928 is fixed.
+    CHECK(url_request_context_);
+
+    DCHECK(!base::FeatureList::IsEnabled(network::features::kNetworkService));
+    DCHECK(!network_context_owner_);
+    network_context_owner_ = std::make_unique<NetworkContextOwner>();
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(&NetworkContextOwner::Initialize,
+                       base::Unretained(network_context_owner_.get()),
+                       MakeRequest(&network_context_), url_request_context_));
+  }
+  network_context_.set_connection_error_handler(base::BindOnce(
+      &StoragePartitionImpl::InitNetworkContext, weak_factory_.GetWeakPtr()));
+}
+
 network::mojom::URLLoaderFactory*
 StoragePartitionImpl::GetURLLoaderFactoryForBrowserProcessInternal() {
   // Create the URLLoaderFactory as needed, but make sure not to reuse a

content/browser/storage_partition_impl.h

@@ -274,6 +274,10 @@ class CONTENT_EXPORT StoragePartitionImpl
   // storage configuration info.
   void GetQuotaSettings(storage::OptionalQuotaSettingsCallback callback);
 
+  // Called to initialize |network_context_| when |GetNetworkContext()| is
+  // first called or there is an error.
+  void InitNetworkContext();
+
   network::mojom::URLLoaderFactory*
   GetURLLoaderFactoryForBrowserProcessInternal();
 

content/browser/url_loader_factory_getter.cc

@@ -111,13 +111,18 @@ URLLoaderFactoryGetter::URLLoaderFactoryForIOThreadInfo::CreateFactory() {
 
 // -----------------------------------------------------------------------------
 
-URLLoaderFactoryGetter::URLLoaderFactoryGetter() {}
+URLLoaderFactoryGetter::URLLoaderFactoryGetter() : weak_factory_(this) {}
 
 void URLLoaderFactoryGetter::Initialize(StoragePartitionImpl* partition) {
   DCHECK(partition);
-  DCHECK(!pending_network_factory_request_.is_pending());
-
   partition_ = partition;
+  Reinitialize();
+}
+
+void URLLoaderFactoryGetter::Reinitialize() {
+  if (!partition_)
+    return;
+  DCHECK(!pending_network_factory_request_.is_pending());
   network::mojom::URLLoaderFactoryPtr network_factory;
   pending_network_factory_request_ = MakeRequest(&network_factory);
 
@@ -128,7 +133,8 @@ void URLLoaderFactoryGetter::Initialize(StoragePartitionImpl* partition) {
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
-      base::BindOnce(&URLLoaderFactoryGetter::InitializeOnIOThread, this,
+      base::BindOnce(&URLLoaderFactoryGetter::InitializeOnIOThread,
+                     weak_factory_.GetWeakPtr(),
                      network_factory.PassInterface()));
 }
 
@@ -166,16 +172,18 @@ URLLoaderFactoryGetter::GetURLLoaderFactory() {
   if (test_factory_)
     return test_factory_;
 
-  if (!network_factory_.is_bound() || network_factory_.encountered_error()) {
-    BrowserThread::PostTask(
-        BrowserThread::UI, FROM_HERE,
-        base::BindOnce(
-            &URLLoaderFactoryGetter::HandleNetworkFactoryRequestOnUIThread,
-            this, mojo::MakeRequest(&network_factory_)));
-  }
+  if (!network_factory_.is_bound())
+    HandleURLLoaderFactoryConnectionError();
   return network_factory_.get();
 }
 
+void URLLoaderFactoryGetter::HandleURLLoaderFactoryConnectionError() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&URLLoaderFactoryGetter::Reinitialize, this));
+}
+
 void URLLoaderFactoryGetter::CloneNetworkFactory(
     network::mojom::URLLoaderFactoryRequest network_factory_request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -201,18 +209,18 @@ void URLLoaderFactoryGetter::SetGetNetworkFactoryCallbackForTesting(
 void URLLoaderFactoryGetter::FlushNetworkInterfaceOnIOThreadForTesting() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   base::RunLoop run_loop;
-  BrowserThread::PostTaskAndReply(
+  BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&URLLoaderFactoryGetter::FlushNetworkInterfaceForTesting,
-                     this),
-      run_loop.QuitClosure());
+                     this, run_loop.QuitClosure()));
   run_loop.Run();
 }
 
-void URLLoaderFactoryGetter::FlushNetworkInterfaceForTesting() {
+void URLLoaderFactoryGetter::FlushNetworkInterfaceForTesting(
+    base::OnceClosure callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (network_factory_)
-    network_factory_.FlushForTesting();
+    network_factory_.FlushAsyncForTesting(std::move(callback));
 }
 
 URLLoaderFactoryGetter::~URLLoaderFactoryGetter() {}
@@ -220,6 +228,9 @@ URLLoaderFactoryGetter::~URLLoaderFactoryGetter() {}
 void URLLoaderFactoryGetter::InitializeOnIOThread(
     network::mojom::URLLoaderFactoryPtrInfo network_factory) {
   network_factory_.Bind(std::move(network_factory));
+  network_factory_.set_connection_error_handler(base::BindOnce(
+      &URLLoaderFactoryGetter::HandleURLLoaderFactoryConnectionError,
+      weak_factory_.GetWeakPtr()));
 }
 
 void URLLoaderFactoryGetter::HandleNetworkFactoryRequestOnUIThread(

content/browser/url_loader_factory_getter.h

@@ -107,8 +107,15 @@ class URLLoaderFactoryGetter
   // The pointer shouldn't be cached.
   network::mojom::URLLoaderFactory* GetURLLoaderFactory();
 
-  // Call |network_factory_.FlushForTesting()|. For test use only.
-  void FlushNetworkInterfaceForTesting();
+  // Called when there is a connection error on |network_factory_|.
+  void HandleURLLoaderFactoryConnectionError();
+
+  // Called either during initialization or when an error occurs.
+  void Reinitialize();
+
+  // Call |network_factory_.FlushForTesting()|. For test use only. When the
+  // flush is complete, |callback| will be called.
+  void FlushNetworkInterfaceForTesting(base::OnceClosure callback);
 
   // Bound with appropriate URLLoaderFactories at HandleFactoryRequests().
   network::mojom::URLLoaderFactoryRequest pending_network_factory_request_;
@@ -122,6 +129,8 @@ class URLLoaderFactoryGetter
   // when it's going away.
   StoragePartitionImpl* partition_ = nullptr;
 
+  base::WeakPtrFactory<URLLoaderFactoryGetter> weak_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(URLLoaderFactoryGetter);
 };
 

extensions/browser/api/declarative/rules_cache_delegate.cc

@@ -104,6 +104,9 @@ void RulesCacheDelegate::UpdateRules(const std::string& extension_id,
 
   DCHECK(value.is_list());
   has_nonempty_ruleset_ = !value.GetList().empty();
+  for (auto& observer : observers_)
+    observer.OnUpdateRules();
+
   if (type_ == Type::kEphemeral)
     return;
 
@@ -123,6 +126,16 @@ bool RulesCacheDelegate::HasRules() const {
   return has_nonempty_ruleset_;
 }
 
+void RulesCacheDelegate::AddObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
+}
+
+void RulesCacheDelegate::RemoveObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.RemoveObserver(observer);
+}
+
 void RulesCacheDelegate::CheckIfReady() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (notified_registry_ || !waiting_for_extensions_.empty())

extensions/browser/api/declarative/rules_cache_delegate.h

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/gtest_prod_util.h"
+#include "base/observer_list.h"
 #include "base/values.h"
 #include "content/public/browser/browser_thread.h"
 
@@ -27,6 +28,15 @@ class RulesRegistry;
 // registering rules on initialization will be logged with UMA.
 class RulesCacheDelegate {
  public:
+  class Observer {
+   public:
+    // Called when |UpdateRules| is called on the |RulesCacheDelegate|.
+    virtual void OnUpdateRules() = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+
   // Determines the type of a cache, indicating whether or not its rules are
   // persisted to storage.
   enum class Type {
@@ -60,6 +70,10 @@ class RulesCacheDelegate {
   // Indicates whether or not this registry has any registered rules cached.
   bool HasRules() const;
 
+  // Adds or removes an observer.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   base::WeakPtr<RulesCacheDelegate> GetWeakPtr() {
     DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
     return weak_ptr_factory_.GetWeakPtr();
@@ -127,6 +141,8 @@ class RulesCacheDelegate {
   // We notified the RulesRegistry that the rules are loaded.
   bool notified_registry_;
 
+  base::ObserverList<Observer> observers_;
+
   // Use this factory to generate weak pointers bound to the UI thread.
   base::WeakPtrFactory<RulesCacheDelegate> weak_ptr_factory_;
 };

extensions/browser/api/declarative/rules_registry_service.cc

@@ -158,11 +158,27 @@ bool RulesRegistryService::HasAnyRegisteredRules() const {
                      });
 }
 
+void RulesRegistryService::AddObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
+}
+
+void RulesRegistryService::RemoveObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.RemoveObserver(observer);
+}
+
 void RulesRegistryService::SimulateExtensionUninstalled(
     const Extension* extension) {
   NotifyRegistriesHelper(&RulesRegistry::OnExtensionUninstalled, extension);
 }
 
+void RulesRegistryService::OnUpdateRules() {
+  // Forward rule updates to observers.
+  for (auto& observer : observers_)
+    observer.OnUpdateRules();
+}
+
 scoped_refptr<RulesRegistry>
 RulesRegistryService::RegisterWebRequestRulesRegistry(
     int rules_registry_id,
@@ -179,6 +195,7 @@ RulesRegistryService::RegisterWebRequestRulesRegistry(
       base::MakeRefCounted<WebRequestRulesRegistry>(
           browser_context_, web_request_cache_delegate.get(),
           rules_registry_id);
+  web_request_cache_delegate->AddObserver(this);
   cache_delegates_.push_back(std::move(web_request_cache_delegate));
   RegisterRulesRegistry(web_request_rules_registry);
   content::BrowserThread::PostTask(
@@ -217,6 +234,7 @@ void RulesRegistryService::EnsureDefaultRulesRegistriesRegistered() {
       ExtensionsAPIClient::Get()->CreateContentRulesRegistry(
           browser_context_, content_rules_cache_delegate.get());
   if (content_rules_registry) {
+    content_rules_cache_delegate->AddObserver(this);
     cache_delegates_.push_back(std::move(content_rules_cache_delegate));
     RegisterRulesRegistry(content_rules_registry);
     content_rules_registry_ = content_rules_registry.get();

extensions/browser/api/declarative/rules_registry_service.h

@@ -33,7 +33,8 @@ namespace extensions {
 // This class owns all RulesRegistries implementations of an ExtensionService.
 // This class lives on the UI thread.
 class RulesRegistryService : public BrowserContextKeyedAPI,
-                             public ExtensionRegistryObserver {
+                             public ExtensionRegistryObserver,
+                             public RulesCacheDelegate::Observer {
  public:
   static const int kDefaultRulesRegistryID;
   static const int kInvalidRulesRegistryID;
@@ -49,6 +50,15 @@ class RulesRegistryService : public BrowserContextKeyedAPI,
     }
   };
 
+  class Observer {
+   public:
+    // Called when any of the |cache_delegates_| have rule updates.
+    virtual void OnUpdateRules() = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+
   explicit RulesRegistryService(content::BrowserContext* context);
   ~RulesRegistryService() override;
 
@@ -92,6 +102,10 @@ class RulesRegistryService : public BrowserContextKeyedAPI,
   // Indicates whether any registry has rules registered.
   bool HasAnyRegisteredRules() const;
 
+  // Adds or removes an observer.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
   // For testing.
   void SimulateExtensionUninstalled(const Extension* extension);
 
@@ -120,6 +134,9 @@ class RulesRegistryService : public BrowserContextKeyedAPI,
                               const Extension* extension,
                               extensions::UninstallReason reason) override;
 
+  // RulesCacheDelegate::Observer implementation.
+  void OnUpdateRules() override;
+
   // Iterates over all registries, and calls |notification_callback| on them
   // with |extension| as the argument. If a registry lives on a different
   // thread, the call is posted to that thread, so no guarantee of synchronous
@@ -152,6 +169,8 @@ class RulesRegistryService : public BrowserContextKeyedAPI,
 
   content::BrowserContext* browser_context_;
 
+  base::ObserverList<Observer> observers_;
+
   DISALLOW_COPY_AND_ASSIGN(RulesRegistryService);
 };
 

extensions/browser/api/declarative_net_request/rules_monitor_service.cc

@@ -78,6 +78,16 @@ bool RulesMonitorService::HasRegisteredRuleset(
          extensions_with_rulesets_.end();
 }
 
+void RulesMonitorService::AddObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
+}
+
+void RulesMonitorService::RemoveObserver(Observer* observer) {
+  DCHECK(observer);
+  observers_.RemoveObserver(observer);
+}
+
 // Helper to pass information related to the ruleset being loaded.
 struct RulesMonitorService::LoadRulesetInfo {
   LoadRulesetInfo(scoped_refptr<const Extension> extension,
@@ -328,6 +338,8 @@ void RulesMonitorService::OnRulesetLoaded(
     return;
 
   extensions_with_rulesets_.insert(info.extension->id());
+  for (auto& observer : observers_)
+    observer.OnRulesetLoaded();
 
   base::OnceClosure load_ruleset_on_io = base::BindOnce(
       &LoadRulesetOnIOThread, info.extension->id(), std::move(matcher),

extensions/browser/api/declarative_net_request/rules_monitor_service.h

@@ -36,6 +36,15 @@ class RulesetMatcher;
 class RulesMonitorService : public BrowserContextKeyedAPI,
                             public ExtensionRegistryObserver {
  public:
+  class Observer {
+   public:
+    // Called when this service loads a new ruleset.
+    virtual void OnRulesetLoaded() = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+
   // BrowserContextKeyedAPI implementation.
   static BrowserContextKeyedAPIFactory<RulesMonitorService>*
   GetFactoryInstance();
@@ -46,6 +55,10 @@ class RulesMonitorService : public BrowserContextKeyedAPI,
   // the given |extension_id|.
   bool HasRegisteredRuleset(const ExtensionId& extension_id) const;
 
+  // Adds or removes an observer.
+  void AddObserver(Observer* observer);
+  void RemoveObserver(Observer* observer);
+
  private:
   struct LoadRulesetInfo;
   class FileSequenceState;
@@ -89,6 +102,8 @@ class RulesMonitorService : public BrowserContextKeyedAPI,
   ExtensionRegistry* const extension_registry_;
   WarningService* const warning_service_;
 
+  base::ObserverList<Observer> observers_;
+
   // Must be the last member variable. See WeakPtrFactory documentation for
   // details.
   base::WeakPtrFactory<RulesMonitorService> weak_factory_;

extensions/browser/api/web_request/web_request_api.cc

@@ -31,13 +31,12 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_request_info.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/resource_type.h"
 #include "extensions/browser/api/activity_log/web_request_constants.h"
-#include "extensions/browser/api/declarative/rules_registry_service.h"
-#include "extensions/browser/api/declarative_net_request/rules_monitor_service.h"
 #include "extensions/browser/api/declarative_net_request/ruleset_manager.h"
 #include "extensions/browser/api/declarative_webrequest/request_stage.h"
 #include "extensions/browser/api/declarative_webrequest/webrequest_constants.h"
@@ -447,7 +446,10 @@ WebRequestAPI::WebRequestAPI(content::BrowserContext* context)
     : browser_context_(context),
       info_map_(ExtensionSystem::Get(browser_context_)->info_map()),
       proxies_(base::MakeRefCounted<ProxySet>()),
-      request_id_generator_(base::MakeRefCounted<RequestIDGenerator>()) {
+      request_id_generator_(base::MakeRefCounted<RequestIDGenerator>()),
+      may_have_proxies_(MayHaveProxies()),
+      rules_monitor_observer_(this),
+      rules_registry_observer_(this) {
   EventRouter* event_router = EventRouter::Get(browser_context_);
   for (size_t i = 0; i < arraysize(kWebRequestEvents); ++i) {
     // Observe the webRequest event.
@@ -459,6 +461,13 @@ WebRequestAPI::WebRequestAPI(content::BrowserContext* context)
         0, sizeof(kWebRequestEventPrefix) - 1, webview::kWebViewEventPrefix);
     event_router->RegisterObserver(this, event_name);
   }
+
+  rules_monitor_observer_.Add(
+      BrowserContextKeyedAPIFactory<
+          declarative_net_request::RulesMonitorService>::Get(browser_context_));
+  rules_registry_observer_.Add(
+      BrowserContextKeyedAPIFactory<RulesRegistryService>::Get(
+          browser_context_));
 }
 
 WebRequestAPI::~WebRequestAPI() {
@@ -469,6 +478,8 @@ WebRequestAPI::~WebRequestAPI() {
 
 void WebRequestAPI::Shutdown() {
   EventRouter::Get(browser_context_)->UnregisterObserver(this);
+  rules_monitor_observer_.RemoveAll();
+  rules_registry_observer_.RemoveAll();
 }
 
 static base::LazyInstance<
@@ -484,12 +495,14 @@ WebRequestAPI::GetFactoryInstance() {
 void WebRequestAPI::OnListenerAdded(const EventListenerInfo& details) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   ++listener_count_;
+  UpdateMayHaveProxies();
 }
 
 void WebRequestAPI::OnListenerRemoved(const EventListenerInfo& details) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   --listener_count_;
   DCHECK_GE(listener_count_, 0);
+  UpdateMayHaveProxies();
 
   // Note that details.event_name includes the sub-event details (e.g. "/123").
   // TODO(fsamuel): <webview> events will not be removed through this code path.
@@ -586,17 +599,8 @@ void WebRequestAPI::MaybeProxyWebSocket(
     network::mojom::WebSocketRequest* request,
     network::mojom::AuthenticationHandlerPtr* auth_handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  const auto* rules_registry_service =
-      BrowserContextKeyedAPIFactory<RulesRegistryService>::Get(
-          browser_context_);
-  const auto* rules_monitor_service = BrowserContextKeyedAPIFactory<
-      declarative_net_request::RulesMonitorService>::Get(browser_context_);
-  if (!base::FeatureList::IsEnabled(network::features::kNetworkService) ||
-      (listener_count_ == 0 &&
-       !rules_registry_service->HasAnyRegisteredRules() &&
-       !rules_monitor_service->HasAnyRegisteredRulesets())) {
+  if (!MayHaveProxies())
     return;
-  }
 
   network::mojom::WebSocketPtrInfo proxied_socket_ptr_info;
   auto proxied_request = std::move(*request);
@@ -621,6 +625,8 @@ bool WebRequestAPI::MayHaveProxies() const {
   if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
     return false;
 
+  // If any other conditions are added here, make sure to call
+  // |UpdateMayHaveProxies()| when those conditions may change.
   const auto* rules_registry_service =
       BrowserContextKeyedAPIFactory<RulesRegistryService>::Get(
           browser_context_);
@@ -631,6 +637,27 @@ bool WebRequestAPI::MayHaveProxies() const {
          rules_monitor_service->HasAnyRegisteredRulesets();
 }
 
+void WebRequestAPI::UpdateMayHaveProxies() {
+  if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
+    return;
+
+  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;
+}
+
+void WebRequestAPI::OnRulesetLoaded() {
+  UpdateMayHaveProxies();
+}
+
+void WebRequestAPI::OnUpdateRules() {
+  UpdateMayHaveProxies();
+}
+
 // Represents a single unique listener to an event, along with whatever filter
 // parameters and extra_info_spec were specified at the time the listener was
 // added.

extensions/browser/api/web_request/web_request_api.h

@@ -24,6 +24,8 @@
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/global_request_id.h"
 #include "extensions/browser/api/declarative/rules_registry.h"
+#include "extensions/browser/api/declarative/rules_registry_service.h"
+#include "extensions/browser/api/declarative_net_request/rules_monitor_service.h"
 #include "extensions/browser/api/declarative_webrequest/request_stage.h"
 #include "extensions/browser/api/web_request/web_request_api_helpers.h"
 #include "extensions/browser/api/web_request/web_request_permissions.h"
@@ -72,8 +74,11 @@ class WebRequestRulesRegistry;
 // work is done by ExtensionWebRequestEventRouter below. This class observes
 // extensions::EventRouter to deal with event listeners. There is one instance
 // per BrowserContext which is shared with incognito.
-class WebRequestAPI : public BrowserContextKeyedAPI,
-                      public EventRouter::Observer {
+class WebRequestAPI
+    : public BrowserContextKeyedAPI,
+      public EventRouter::Observer,
+      public declarative_net_request::RulesMonitorService::Observer,
+      public RulesRegistryService::Observer {
  public:
   // A callback used to asynchronously respond to an intercepted authentication
   // request when the Network Service is enabled. If |should_cancel| is true
@@ -236,6 +241,16 @@ class WebRequestAPI : public BrowserContextKeyedAPI,
   // installed to support the API with Network Service enabled.
   bool MayHaveProxies() const;
 
+  // Checks if |MayHaveProxies()| has changed from false to true, and resets
+  // URLLoaderFactories if so.
+  void UpdateMayHaveProxies();
+
+  // RulesMonitorService::Observer implementation.
+  void OnRulesetLoaded() override;
+
+  // RulesRegistryService::Observer implementation.
+  void OnUpdateRules() override;
+
   // A count of active event listeners registered in this BrowserContext. This
   // is eventually consistent with the state of
   int listener_count_ = 0;
@@ -248,6 +263,16 @@ class WebRequestAPI : public BrowserContextKeyedAPI,
 
   scoped_refptr<RequestIDGenerator> request_id_generator_;
 
+  // Stores the last result of |MayHaveProxies()|, so it can be used in
+  // |UpdateMayHaveProxies()|.
+  bool may_have_proxies_;
+
+  ScopedObserver<declarative_net_request::RulesMonitorService,
+                 declarative_net_request::RulesMonitorService::Observer>
+      rules_monitor_observer_;
+  ScopedObserver<RulesRegistryService, RulesRegistryService::Observer>
+      rules_registry_observer_;
+
   DISALLOW_COPY_AND_ASSIGN(WebRequestAPI);
 };
 

extensions/browser/api/web_request/web_request_proxying_url_loader_factory.cc

@@ -337,16 +337,15 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::ContinueAuthRequest(
   }
 
   info_->AddResponseInfoFromResourceResponse(current_response_);
+  auto continuation =
+      base::BindRepeating(&InProgressRequest::OnAuthRequestHandled,
+                          weak_factory_.GetWeakPtr(), base::Passed(&callback));
 
   auth_credentials_.emplace();
   net::NetworkDelegate::AuthRequiredResponse response =
       ExtensionWebRequestEventRouter::GetInstance()->OnAuthRequired(
           factory_->browser_context_, factory_->info_map_, &info_.value(),
-          *auth_info,
-          base::BindRepeating(&InProgressRequest::OnAuthRequestHandled,
-                              weak_factory_.GetWeakPtr(),
-                              base::Passed(&callback)),
-          &auth_credentials_.value());
+          *auth_info, continuation, &auth_credentials_.value());
 
   // At least one extension has a blocking handler for this request, so we'll
   // just wait for them to finish. |OnAuthRequestHandled()| will be invoked
@@ -357,10 +356,7 @@ void WebRequestProxyingURLLoaderFactory::InProgressRequest::ContinueAuthRequest(
   // We're not touching this auth request. Let the default browser behavior
   // proceed.
   DCHECK_EQ(response, net::NetworkDelegate::AUTH_REQUIRED_RESPONSE_NO_ACTION);
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
-      base::BindOnce(std::move(callback), base::nullopt,
-                     false /* should_cancel */));
+  continuation.Run(response);
 }
 
 void WebRequestProxyingURLLoaderFactory::InProgressRequest::

services/network/cors/cors_url_loader_factory.cc

@@ -96,6 +96,10 @@ void CORSURLLoaderFactory::Clone(mojom::URLLoaderFactoryRequest request) {
   bindings_.AddBinding(this, std::move(request));
 }
 
+void CORSURLLoaderFactory::ClearBindings() {
+  bindings_.CloseAllBindings();
+}
+
 void CORSURLLoaderFactory::DeleteIfNeeded() {
   if (!context_)
     return;

services/network/cors/cors_url_loader_factory.h

@@ -48,6 +48,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoaderFactory final
   void OnLoaderCreated(std::unique_ptr<mojom::URLLoader> loader);
   void DestroyURLLoader(mojom::URLLoader* loader);
 
+  // Clears the bindings for this factory, but does not touch any in-progress
+  // URLLoaders.
+  void ClearBindings();
+
  private:
   // Implements mojom::URLLoaderFactory.
   void CreateLoaderAndStart(mojom::URLLoaderRequest request,

services/network/network_context.cc

@@ -512,7 +512,7 @@ void NetworkContext::DisableQuic() {
 }
 
 void NetworkContext::DestroyURLLoaderFactory(
-    mojom::URLLoaderFactory* url_loader_factory) {
+    cors::CORSURLLoaderFactory* url_loader_factory) {
   auto it = url_loader_factories_.find(url_loader_factory);
   DCHECK(it != url_loader_factories_.end());
   url_loader_factories_.erase(it);
@@ -831,6 +831,11 @@ void NetworkContext::PreconnectSockets(uint32_t num_streams,
       base::saturated_cast<int32_t>(num_streams), request_info);
 }
 
+void NetworkContext::ResetURLLoaderFactories() {
+  for (const auto& factory : url_loader_factories_)
+    factory->ClearBindings();
+}
+
 // ApplyContextParamsToBuilder represents the core configuration for
 // translating |network_context_params| into a set of configuration that can
 // be used to build a request context. All new initialization should be done

services/network/network_context.h

@@ -58,6 +58,10 @@ class ResourceSchedulerClient;
 class URLRequestContextBuilderMojo;
 class WebSocketFactory;
 
+namespace cors {
+class CORSURLLoaderFactory;
+}  // namespace cors
+
 // A NetworkContext creates and manages access to a URLRequestContext.
 //
 // When the network service is enabled, NetworkContexts are created through
@@ -203,13 +207,14 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
                          const GURL& url,
                          int32_t load_flags,
                          bool privacy_mode_enabled) override;
+  void ResetURLLoaderFactories() override;
 
   // Disables use of QUIC by the NetworkContext.
   void DisableQuic();
 
   // Destroys the specified factory. Called by the factory itself when it has
   // no open pipes.
-  void DestroyURLLoaderFactory(mojom::URLLoaderFactory* url_loader_factory);
+  void DestroyURLLoaderFactory(cors::CORSURLLoaderFactory* url_loader_factory);
 
  private:
   class ContextNetworkDelegate;
@@ -274,7 +279,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
 
   // This must be below |url_request_context_| so that the URLRequestContext
   // outlives all the URLLoaderFactories and URLLoaders that depend on it.
-  std::set<std::unique_ptr<mojom::URLLoaderFactory>, base::UniquePtrComparator>
+  std::set<std::unique_ptr<cors::CORSURLLoaderFactory>,
+           base::UniquePtrComparator>
       url_loader_factories_;
 
   mojo::StrongBindingSet<mojom::NetLogExporter> net_log_exporter_bindings_;

services/network/public/mojom/network_context.mojom

@@ -523,6 +523,11 @@ interface NetworkContext {
                     int32 load_flags,
                     bool privacy_mode_enabled);
 
+  // Destroys all URLLoaderFactory bindings, which should then be regenerated.
+  // This should be called if there is a change to the proxies which should be
+  // used on URLLoaders.
+  ResetURLLoaderFactories();
+
   [Sync]
   // Adds explicitly-specified data as if it was processed from an
   // HSTS header.

services/network/test/test_network_context.h

@@ -111,6 +111,7 @@ class TestNetworkContext : public mojom::NetworkContext {
                          const GURL& url,
                          int32_t load_flags,
                          bool privacy_mode_enabled) override {}
+  void ResetURLLoaderFactories() override {}
 };
 
 }  // namespace network

services/network/test/test_url_loader_factory_unittest.cc

@@ -214,11 +214,10 @@ TEST_F(TestURLLoaderFactoryTest, NumPending2) {
 
 TEST_F(TestURLLoaderFactoryTest, SimulateResponse) {
   std::string url = "http://foo/";
-  std::string cookie_line = "my_cookie=myvalue";
   network::URLLoaderCompletionStatus ok_status(net::OK);
   ResourceResponseHead response_head =
       CreateResourceResponseHead(net::HTTP_NOT_FOUND);
-  AddCookiesToResourceResponseHead({cookie_line}, &response_head);
+  response_head.headers->AddHeader("Foo: Bar");
 
   // By default no request is pending.
   EXPECT_FALSE(factory()->SimulateResponseForPendingRequest(
@@ -238,16 +237,11 @@ TEST_F(TestURLLoaderFactoryTest, SimulateResponse) {
   ASSERT_TRUE(client()->response_head().headers);
   EXPECT_EQ(net::HTTP_NOT_FOUND,
             client()->response_head().headers->response_code());
-  // Our cookie should be set.
-  int cookie_count = 0;
+  // Our header should be set.
   std::string value;
-  size_t iter = 0;
-  while (client()->response_head().headers->EnumerateHeader(&iter, "Set-Cookie",
-                                                            &value)) {
-    EXPECT_EQ(cookie_line, value);
-    cookie_count++;
-  }
-  EXPECT_EQ(1, cookie_count);
+  EXPECT_TRUE(
+      client()->response_head().headers->GetNormalizedHeader("Foo", &value));
+  EXPECT_EQ("Bar", value);
   std::string response;
   EXPECT_TRUE(
       mojo::BlockingCopyToString(client()->response_body_release(), &response));

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

@@ -208,18 +208,11 @@
 -ExtensionWebRequestApiTest.WebRequestDeclarative2
 -ExtensionWebRequestApiTest.WebRequestDiceHeaderProtection
 -ExtensionWebRequestApiTest.WebRequestTypes
+# Needs synchronization to wait until after URLLoaderFactories are rebound.
+-DevToolsFrontendInWebRequestApiTest.HiddenRequests
 # Note WebRequestUnloadImmediately is disabled on Linux
 -ExtensionWebRequestApiTest.WebRequestUnloadImmediately
 
-# WebRequest implementation needs to handle extension
-# installation/uninstallation events.
-# http://crbug.com/857577
--BackgroundXhrTest.HttpAuth
--BackgroundXhrTest.TlsClientAuth
-
-# http://crbug.com/853733
--DeclarativeNetRequestBrowserTest.PageAllowingAPI_BrowserRequests/0
-
 # http://crbug.com/705114
 # Remove streams concept from code and replace with data pipe passing.
 -MimeHandlerViewTests/MimeHandlerViewTest.Abort/0