OOR-CORS: Make PreflightController per-profile

Existing implementation uses a single PreflightController for all
profiles, but ideally it should be per-profile.

This patch makes the controller per-profile for NetworkService, and does
it per-factory for legacy code path due to lack of NetworkContext.

Bug: 894690
Change-Id: Ief446431e4235bcecd68ba1f50528e721efed96a
Reviewed-on: https://chromium-review.googlesource.com/c/1331349Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607513}

services/network/cors/cors_url_loader.cc

@@ -55,7 +55,8 @@ CORSURLLoader::CORSURLLoader(
     const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
     mojom::URLLoaderFactory* network_loader_factory,
     const base::RepeatingCallback<void(int)>& request_finalizer,
-    const OriginAccessList* origin_access_list)
+    const OriginAccessList* origin_access_list,
+    PreflightController* preflight_controller)
     : binding_(this, std::move(loader_request)),
       routing_id_(routing_id),
       request_id_(request_id),
@@ -68,11 +69,13 @@ CORSURLLoader::CORSURLLoader(
       request_finalizer_(request_finalizer),
       traffic_annotation_(traffic_annotation),
       origin_access_list_(origin_access_list),
+      preflight_controller_(preflight_controller),
       weak_factory_(this) {
   binding_.set_connection_error_handler(base::BindOnce(
       &CORSURLLoader::OnConnectionError, base::Unretained(this)));
   DCHECK(network_loader_factory_);
   DCHECK(origin_access_list_);
+  DCHECK(preflight_controller_);
   SetCORSFlagIfNeeded();
 }
 
@@ -396,7 +399,7 @@ void CORSURLLoader::StartRequest() {
   if (request_finalizer_)
     preflight_finalizer = base::BindOnce(request_finalizer_, request_id_);
 
-  PreflightController::GetDefaultController()->PerformPreflightCheck(
+  preflight_controller_->PerformPreflightCheck(
       base::BindOnce(&CORSURLLoader::StartNetworkRequest,
                      weak_factory_.GetWeakPtr()),
       request_id_, request_, tainted_,

services/network/cors/cors_url_loader.h

@@ -9,6 +9,7 @@
 #include "base/optional.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
+#include "services/network/cors/preflight_controller.h"
 #include "services/network/public/cpp/cors/cors_error_status.h"
 #include "services/network/public/cpp/cors/preflight_timing_info.h"
 #include "services/network/public/mojom/fetch_api.mojom.h"
@@ -47,7 +48,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoader
       const net::MutableNetworkTrafficAnnotationTag& traffic_annotation,
       mojom::URLLoaderFactory* network_loader_factory,
       const base::RepeatingCallback<void(int)>& request_finalizer,
-      const OriginAccessList* origin_access_list);
+      const OriginAccessList* origin_access_list,
+      PreflightController* preflight_controller);
 
   ~CORSURLLoader() override;
 
@@ -159,6 +161,7 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoader
 
   // Outlives |this|.
   const OriginAccessList* const origin_access_list_;
+  PreflightController* preflight_controller_;
 
   // Used to run asynchronous class instance bound callbacks safely.
   base::WeakPtrFactory<CORSURLLoader> weak_factory_;

services/network/cors/cors_url_loader_factory.cc

@@ -38,6 +38,7 @@ CORSURLLoaderFactory::CORSURLLoaderFactory(
   bindings_.AddBinding(this, std::move(request));
   bindings_.set_connection_error_handler(base::BindRepeating(
       &CORSURLLoaderFactory::DeleteIfNeeded, base::Unretained(this)));
+  preflight_controller_ = context_->cors_preflight_controller();
 }
 
 CORSURLLoaderFactory::CORSURLLoaderFactory(
@@ -50,6 +51,10 @@ CORSURLLoaderFactory::CORSURLLoaderFactory(
       preflight_finalizer_(preflight_finalizer),
       origin_access_list_(origin_access_list) {
   DCHECK(origin_access_list_);
+  // Ideally this should be per-profile, but per-factory would be enough for
+  // this code path that is eventually removed.
+  owned_preflight_controller_ = std::make_unique<PreflightController>();
+  preflight_controller_ = owned_preflight_controller_.get();
 }
 
 CORSURLLoaderFactory::~CORSURLLoaderFactory() = default;
@@ -88,7 +93,7 @@ void CORSURLLoaderFactory::CreateLoaderAndStart(
                        base::Unretained(this)),
         resource_request, std::move(client), traffic_annotation,
         network_loader_factory_.get(), preflight_finalizer_,
-        origin_access_list_);
+        origin_access_list_, preflight_controller_);
     auto* raw_loader = loader.get();
     OnLoaderCreated(std::move(loader));
     raw_loader->Start();

services/network/cors/cors_url_loader_factory.h

@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "mojo/public/cpp/bindings/strong_binding_set.h"
 #include "net/traffic_annotation/network_traffic_annotation.h"
+#include "services/network/cors/preflight_controller.h"
 #include "services/network/public/cpp/cors/origin_access_list.h"
 #include "services/network/public/mojom/network_context.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
@@ -91,6 +92,13 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) CORSURLLoaderFactory final
   // it's safe.
   const OriginAccessList* const origin_access_list_;
 
+  // Usually |preflight_controoler_| is owned by NetworkContext, but we create
+  // own one if NetworkContext is not provided, e.g. for legacy code path.
+  // TODO(toyoshim): Remove owned controller once the network service is fully
+  // enabled.
+  PreflightController* preflight_controller_;
+  std::unique_ptr<PreflightController> owned_preflight_controller_;
+
   DISALLOW_COPY_AND_ASSIGN(CORSURLLoaderFactory);
 };
 

services/network/cors/preflight_controller.cc

@@ -374,12 +374,6 @@ PreflightController::CreatePreflightRequestForTesting(
   return CreatePreflightRequest(request, tainted);
 }
 
-// static
-PreflightController* PreflightController::GetDefaultController() {
-  static base::NoDestructor<PreflightController> controller;
-  return &*controller;
-}
-
 PreflightController::PreflightController() = default;
 
 PreflightController::~PreflightController() = default;

services/network/cors/preflight_controller.h

@@ -44,10 +44,6 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) PreflightController final {
       const ResourceRequest& request,
       bool tainted = false);
 
-  // Obtains the shared default controller instance.
-  // TODO(toyoshim): Find a right owner rather than a single design.
-  static PreflightController* GetDefaultController();
-
   PreflightController();
   ~PreflightController();
 

services/network/network_context.h

@@ -29,6 +29,7 @@
 #include "net/cert/cert_verify_result.h"
 #include "net/dns/dns_config_overrides.h"
 #include "net/dns/host_resolver.h"
+#include "services/network/cors/preflight_controller.h"
 #include "services/network/http_cache_data_counter.h"
 #include "services/network/http_cache_data_remover.h"
 #include "services/network/network_qualities_pref_delegate.h"
@@ -344,6 +345,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
         std::move(network_qualities_pref_delegate);
   }
 
+  cors::PreflightController* cors_preflight_controller() {
+    return &cors_preflight_controller_;
+  }
+
  private:
   class ContextNetworkDelegate;
 
@@ -512,6 +517,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
   // Manages allowed origin access lists.
   cors::OriginAccessList cors_origin_access_list_;
 
+  // Manages CORS preflight requests and its cache.
+  cors::PreflightController cors_preflight_controller_;
+
   std::unique_ptr<NetworkQualitiesPrefDelegate>
       network_qualities_pref_delegate_;