Fix safe browsing's IO thread URLLoaderFactory not handling crashes.

SafeBrowsingService::CreateURLLoaderFactoryForIO created a factory directly through NetworkService,
which means that if the network process crashed the interface pointer wouldn't work anymore.
Instead use SafeBrowsingService::GetURLLoaderFactory(), through
CrossThreadSharedURLLoaderFactoryInfo, which correctly handles this case.

Bug: 971274
Change-Id: Ia6915e1e7aa4121130ab34b30587827adb54e96f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1645456Reviewed-by: Varun Khaneja <vakh@chromium.org>
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666363}

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -48,6 +48,7 @@
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/resource_request_info.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "services/network/public/cpp/cross_thread_shared_url_loader_factory_info.h"
 #include "services/network/public/cpp/features.h"
 #include "services/preferences/public/mojom/tracked_preference_validation_delegate.mojom.h"
 
@@ -240,21 +241,6 @@ void SafeBrowsingService::FlushNetworkInterfaceForTesting() {
     network_context_->FlushForTesting();
 }
 
-scoped_refptr<network::SharedURLLoaderFactory>
-SafeBrowsingService::GetURLLoaderFactoryOnIOThread() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  if (!shared_url_loader_factory_on_io_) {
-    base::PostTaskWithTraits(
-        FROM_HERE, {BrowserThread::UI},
-        base::BindOnce(&SafeBrowsingService::CreateURLLoaderFactoryForIO, this,
-                       MakeRequest(&url_loader_factory_on_io_)));
-    shared_url_loader_factory_on_io_ =
-        base::MakeRefCounted<network::WeakWrapperSharedURLLoaderFactory>(
-            url_loader_factory_on_io_.get());
-  }
-  return shared_url_loader_factory_on_io_;
-}
-
 const scoped_refptr<SafeBrowsingUIManager>& SafeBrowsingService::ui_manager()
     const {
   return ui_manager_;
@@ -362,7 +348,8 @@ void SafeBrowsingService::SetDatabaseManagerForTest(
   services_delegate_->SetDatabaseManagerForTest(database_manager);
 }
 
-void SafeBrowsingService::StartOnIOThread() {
+void SafeBrowsingService::StartOnIOThread(
+    std::unique_ptr<network::SharedURLLoaderFactoryInfo> url_loader_factory) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (enabled_)
     return;
@@ -370,7 +357,8 @@ void SafeBrowsingService::StartOnIOThread() {
 
   V4ProtocolConfig v4_config = GetV4ProtocolConfig();
 
-  services_delegate_->StartOnIOThread(GetURLLoaderFactoryOnIOThread(),
+  services_delegate_->StartOnIOThread(
+      network::SharedURLLoaderFactory::Create(std::move(url_loader_factory)),
       v4_config);
 }
 
@@ -382,10 +370,6 @@ void SafeBrowsingService::StopOnIOThread(bool shutdown) {
   if (enabled_) {
     enabled_ = false;
   }
-
-  if (shared_url_loader_factory_on_io_)
-    shared_url_loader_factory_on_io_->Detach();
-  url_loader_factory_on_io_.reset();
 }
 
 void SafeBrowsingService::Start() {
@@ -398,7 +382,10 @@ void SafeBrowsingService::Start() {
 
   base::PostTaskWithTraits(
       FROM_HERE, {BrowserThread::IO},
-      base::BindOnce(&SafeBrowsingService::StartOnIOThread, this));
+      base::BindOnce(
+          &SafeBrowsingService::StartOnIOThread, this,
+          std::make_unique<network::CrossThreadSharedURLLoaderFactoryInfo>(
+              GetURLLoaderFactory())));
 }
 
 void SafeBrowsingService::Stop(bool shutdown) {
@@ -526,19 +513,6 @@ void SafeBrowsingService::CreateTriggerManager() {
       g_browser_process->local_state());
 }
 
-void SafeBrowsingService::CreateURLLoaderFactoryForIO(
-    network::mojom::URLLoaderFactoryRequest request) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  if (shutdown_)
-    return;  // We've been shut down already.
-  network::mojom::URLLoaderFactoryParamsPtr params =
-      network::mojom::URLLoaderFactoryParams::New();
-  params->process_id = network::mojom::kBrowserProcessId;
-  params->is_corb_enabled = false;
-  GetNetworkContext()->CreateURLLoaderFactory(std::move(request),
-                                              std::move(params));
-}
-
 network::mojom::NetworkContextParamsPtr
 SafeBrowsingService::CreateNetworkContextParams() {
   auto params = g_browser_process->system_network_context_manager()

chrome/browser/safe_browsing/safe_browsing_service.h

@@ -27,7 +27,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
-#include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
 #include "services/network/public/mojom/network_context.mojom-forward.h"
 
 #if defined(FULL_SAFE_BROWSING)
@@ -52,6 +51,7 @@ namespace mojom {
 class NetworkContext;
 }
 class SharedURLLoaderFactory;
+class SharedURLLoaderFactoryInfo;
 }  // namespace network
 
 namespace prefs {
@@ -137,10 +137,6 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
   // Flushes above two interfaces to avoid races in tests.
   void FlushNetworkInterfaceForTesting();
 
-  // Called to get a SharedURLLoaderFactory that can be used on the IO thread.
-  scoped_refptr<network::SharedURLLoaderFactory>
-  GetURLLoaderFactoryOnIOThread();
-
   const scoped_refptr<SafeBrowsingUIManager>& ui_manager() const;
 
   virtual const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager()
@@ -228,7 +224,8 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
 
   // Called to initialize objects that are used on the io_thread.  This may be
   // called multiple times during the life of the SafeBrowsingService.
-  void StartOnIOThread();
+  void StartOnIOThread(
+      std::unique_ptr<network::SharedURLLoaderFactoryInfo> url_loader_factory);
 
   // Called to stop or shutdown operations on the io_thread. This may be called
   // multiple times to stop during the life of the SafeBrowsingService. If
@@ -266,11 +263,6 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
 
   void CreateTriggerManager();
 
-  // Called on the UI thread to create a URLLoaderFactory interface ptr for
-  // the IO thread.
-  void CreateURLLoaderFactoryForIO(
-      network::mojom::URLLoaderFactoryRequest request);
-
   // Creates a configured NetworkContextParams when the network service is in
   // use.
   network::mojom::NetworkContextParamsPtr CreateNetworkContextParams();
@@ -289,11 +281,6 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
   // SimpleURLLoader for safe browsing requests.
   std::unique_ptr<safe_browsing::SafeBrowsingNetworkContext> network_context_;
 
-  // A SharedURLLoaderFactory and its interfaceptr used on the IO thread.
-  network::mojom::URLLoaderFactoryPtr url_loader_factory_on_io_;
-  scoped_refptr<network::WeakWrapperSharedURLLoaderFactory>
-      shared_url_loader_factory_on_io_;
-
   // Provides phishing and malware statistics. Accessed on UI thread.
   std::unique_ptr<PingManager> ping_manager_;