Manage SafeBrowsingNetworkContexts on a per-profile basis

This CL creates a Finch flag for separating SafeBrowsingNetworkContexts
and depending on that flag, creates separate network contexts for each
Profile. Future CLs will port users of the SafeBrowsingNetworkContext
to the Profile-scoped contexts. (For example,
https://chromium-review.googlesource.com/c/chromium/src/+/2044716)

Bug: 1049833
Change-Id: I36e9ecd43e3ce5ceb6333850bcde7ea61065bd5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083576
Commit-Queue: Daniel Rubery <drubery@chromium.org>
Reviewed-by: Bettina Dea <bdea@chromium.org>
Auto-Submit: Daniel Rubery <drubery@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747380}

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -41,6 +41,7 @@
 #include "components/safe_browsing/core/browser/safe_browsing_network_context.h"
 #include "components/safe_browsing/core/common/safebrowsing_constants.h"
 #include "components/safe_browsing/core/db/database_manager.h"
+#include "components/safe_browsing/core/features.h"
 #include "components/safe_browsing/core/file_type_policies.h"
 #include "components/safe_browsing/core/ping_manager.h"
 #include "components/safe_browsing/core/realtime/policy_engine.h"
@@ -170,6 +171,26 @@ SafeBrowsingService::GetURLLoaderFactory() {
   return network_context_->GetURLLoaderFactory();
 }
 
+network::mojom::NetworkContext* SafeBrowsingService::GetNetworkContext(
+    Profile* profile) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!base::FeatureList::IsEnabled(kSafeBrowsingSeparateNetworkContexts))
+    return GetNetworkContext();
+
+  return services_delegate_->GetSafeBrowsingNetworkContext(profile)
+      ->GetNetworkContext();
+}
+
+scoped_refptr<network::SharedURLLoaderFactory>
+SafeBrowsingService::GetURLLoaderFactory(Profile* profile) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!base::FeatureList::IsEnabled(kSafeBrowsingSeparateNetworkContexts))
+    return GetURLLoaderFactory();
+
+  return services_delegate_->GetSafeBrowsingNetworkContext(profile)
+      ->GetURLLoaderFactory();
+}
+
 void SafeBrowsingService::FlushNetworkInterfaceForTesting() {
   if (network_context_)
     network_context_->FlushForTesting();
@@ -362,12 +383,14 @@ void SafeBrowsingService::OnProfileWillBeDestroyed(Profile* profile) {
   services_delegate_->RemovePasswordProtectionService(profile);
   services_delegate_->RemoveTelemetryService(profile);
   services_delegate_->RemoveBinaryUploadService(profile);
+  services_delegate_->RemoveSafeBrowsingNetworkContext(profile);
 }
 
 void SafeBrowsingService::CreateServicesForProfile(Profile* profile) {
   services_delegate_->CreatePasswordProtectionService(profile);
   services_delegate_->CreateTelemetryService(profile);
   services_delegate_->CreateBinaryUploadService(profile);
+  services_delegate_->CreateSafeBrowsingNetworkContext(profile);
   observed_profiles_.Add(profile);
 }
 

chrome/browser/safe_browsing/safe_browsing_service.h

@@ -125,9 +125,17 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
 
   // NetworkContext and URLLoaderFactory used for safe browsing requests.
   // Called on UI thread.
+  // TODO(crbug/1049833): Transition all callers of these functions to the
+  // per-profile methods below.
   network::mojom::NetworkContext* GetNetworkContext();
   virtual scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory();
 
+  // Get the NetworkContext or URLLoaderFactory attached to |profile|. Called on
+  // UI thread.
+  network::mojom::NetworkContext* GetNetworkContext(Profile* profile);
+  virtual scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory(
+      Profile* profile);
+
   // Flushes above two interfaces to avoid races in tests.
   void FlushNetworkInterfaceForTesting();
 

chrome/browser/safe_browsing/services_delegate.cc

@@ -4,22 +4,28 @@
 
 #include "chrome/browser/safe_browsing/services_delegate.h"
 
+#include <memory>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/history/history_service_factory.h"
+#include "chrome/browser/net/system_network_context_manager.h"
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #include "chrome/browser/safe_browsing/telemetry/telemetry_service.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/keyed_service/core/service_access_type.h"
 #include "components/safe_browsing/buildflags.h"
+#include "components/safe_browsing/core/browser/safe_browsing_network_context.h"
 #include "components/safe_browsing/core/db/v4_local_database_manager.h"
+#include "components/safe_browsing/core/features.h"
 #include "content/public/browser/browser_thread.h"
 #include "services/network/public/cpp/shared_url_loader_factory.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 #include "services/preferences/public/mojom/tracked_preference_validation_delegate.mojom.h"
 
 namespace safe_browsing {
@@ -64,6 +70,64 @@ PasswordProtectionService* ServicesDelegate::GetPasswordProtectionService(
 void ServicesDelegate::ShutdownServices() {
   // Delete the ChromePasswordProtectionService instances.
   password_protection_service_map_.clear();
+
+  // Delete the NetworkContexts and associated ProxyConfigMonitors
+  network_context_map_.clear();
+  proxy_config_monitor_map_.clear();
+}
+
+void ServicesDelegate::CreateSafeBrowsingNetworkContext(Profile* profile) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
+
+  if (!base::FeatureList::IsEnabled(kSafeBrowsingSeparateNetworkContexts))
+    return;
+
+  auto it = network_context_map_.find(profile);
+  DCHECK(it == network_context_map_.end());
+  network_context_map_[profile] = std::make_unique<SafeBrowsingNetworkContext>(
+      profile->GetPath(),
+      base::BindRepeating(&ServicesDelegate::CreateNetworkContextParams,
+                          base::Unretained(this), profile));
+  proxy_config_monitor_map_[profile] =
+      std::make_unique<ProxyConfigMonitor>(profile);
+}
+
+void ServicesDelegate::RemoveSafeBrowsingNetworkContext(Profile* profile) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  DCHECK(profile);
+
+  if (!base::FeatureList::IsEnabled(kSafeBrowsingSeparateNetworkContexts))
+    return;
+
+  auto it = network_context_map_.find(profile);
+  if (it != network_context_map_.end()) {
+    it->second->ServiceShuttingDown();
+    network_context_map_.erase(it);
+  }
+
+  auto proxy_it = proxy_config_monitor_map_.find(profile);
+  if (proxy_it != proxy_config_monitor_map_.end())
+    proxy_config_monitor_map_.erase(proxy_it);
+}
+
+SafeBrowsingNetworkContext* ServicesDelegate::GetSafeBrowsingNetworkContext(
+    Profile* profile) const {
+  DCHECK(profile);
+  DCHECK(base::FeatureList::IsEnabled(kSafeBrowsingSeparateNetworkContexts));
+  auto it = network_context_map_.find(profile);
+  DCHECK(it != network_context_map_.end());
+  return it->second.get();
+}
+
+network::mojom::NetworkContextParamsPtr
+ServicesDelegate::CreateNetworkContextParams(Profile* profile) {
+  auto params = SystemNetworkContextManager::GetInstance()
+                    ->CreateDefaultNetworkContextParams();
+  auto it = proxy_config_monitor_map_.find(profile);
+  DCHECK(it != proxy_config_monitor_map_.end());
+  it->second->AddToNetworkContextParams(params.get());
+  return params;
 }
 
 }  // namespace safe_browsing

chrome/browser/safe_browsing/services_delegate.h

@@ -12,8 +12,10 @@
 #include "chrome/browser/safe_browsing/chrome_password_protection_service.h"
 #include "chrome/browser/safe_browsing/incident_reporting/delayed_analysis_callback.h"
 #include "components/safe_browsing/content/password_protection/password_protection_service.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 
 class Profile;
+class ProxyConfigMonitor;
 
 namespace content {
 class DownloadManager;
@@ -41,6 +43,7 @@ struct ResourceRequestInfo;
 class SafeBrowsingService;
 class SafeBrowsingDatabaseManager;
 struct V4ProtocolConfig;
+class SafeBrowsingNetworkContext;
 
 // Abstraction to help organize code for mobile vs full safe browsing modes.
 // This helper class should be owned by a SafeBrowsingService, and it handles
@@ -134,21 +137,37 @@ class ServicesDelegate {
   virtual BinaryUploadService* GetBinaryUploadService(
       Profile* profile) const = 0;
 
+  virtual void CreateSafeBrowsingNetworkContext(Profile* profile);
+  virtual void RemoveSafeBrowsingNetworkContext(Profile* profile);
+  virtual SafeBrowsingNetworkContext* GetSafeBrowsingNetworkContext(
+      Profile* profile) const;
+
   virtual std::string GetSafetyNetId() const = 0;
 
  protected:
+  network::mojom::NetworkContextParamsPtr CreateNetworkContextParams(
+      Profile* profile);
+
   // Unowned pointer
   SafeBrowsingService* const safe_browsing_service_;
 
   // Unowned pointer
   ServicesCreator* const services_creator_;
 
+  std::unique_ptr<ProxyConfigMonitor> proxy_config_monitor_;
+
   // Tracks existing Profiles, and their corresponding
   // ChromePasswordProtectionService instances.
   // Accessed on UI thread.
   base::flat_map<Profile*, std::unique_ptr<ChromePasswordProtectionService>>
       password_protection_service_map_;
 
+  // Tracks existing Profiles, and their corresponding
+  // SafeBrowsingNetworkContexts. Accessed on UI thread.
+  base::flat_map<Profile*, std::unique_ptr<SafeBrowsingNetworkContext>>
+      network_context_map_;
+  base::flat_map<Profile*, std::unique_ptr<ProxyConfigMonitor>>
+      proxy_config_monitor_map_;
 };
 
 }  // namespace safe_browsing

components/safe_browsing/core/features.cc

@@ -87,6 +87,9 @@ const base::Feature kRealTimeUrlLookupEnabledWithToken{
     "SafeBrowsingRealTimeUrlLookupEnabledWithToken",
     base::FEATURE_DISABLED_BY_DEFAULT};
 
+const base::Feature kSafeBrowsingSeparateNetworkContexts{
+    "SafeBrowsingSeparateNetworkContexts", base::FEATURE_DISABLED_BY_DEFAULT};
+
 const base::Feature kSendOnFocusPing {
   "SafeBrowsingSendOnFocusPing",
       base::FEATURE_ENABLED_BY_DEFAULT
@@ -135,6 +138,7 @@ constexpr struct {
     {&kPromptAppForDeepScanning, true},
     {&kRealTimeUrlLookupEnabled, true},
     {&kRealTimeUrlLookupEnabledWithToken, true},
+    {&kSafeBrowsingSeparateNetworkContexts, true},
     {&kSendOnFocusPing, true},
     {&kSendPasswordReusePing, true},
     {&kSuspiciousSiteTriggerQuotaFeature, true},

components/safe_browsing/core/features.h

@@ -69,6 +69,10 @@ extern const base::Feature kPasswordProtectionForSignedInUsers;
 // Controls whether Chrome prompts Advanced Protection users for deep scanning.
 extern const base::Feature kPromptAppForDeepScanning;
 
+// Controls whether Safe Browsing uses separate NetworkContexts for each
+// profile.
+extern const base::Feature kSafeBrowsingSeparateNetworkContexts;
+
 // Controls whether Chrome sends on focus ping.
 extern const base::Feature kSendOnFocusPing;