WebLayer: Move ownership of user_pref_service_ into BrowserContextImpl

Currently, user_prefs::UserPrefs::Get() is only needed within the
context of WebLayer's safe browsing implementation on Android. However,
it will shortly also be needed by WebLayer's SSL implementation (when we
change that implementation to be based on the shared
//components-level one rather than being a fork). SSL interstitials
run on all platforms (concretely, Linux as well as Android). In
preparation for that change, this CL moves the creation and ownership of
WebLayer's user prefs service into BrowserContextImpl. This change both
ensures that the PrefService is created and registered on all platforms
rather than only Android and also aligns WebLayer with how user pref
service creation and ownership is structured in other embedders in
which user_prefs::UserPrefs::Get() is broadly used (e.g.,
Android WebView and Chrome).

Bug: 1026591
Change-Id: Ibab57af127aca76f3944389896f538b1f2571e9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930658Reviewed-by: Tim Volodine <timvolodine@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719073}

weblayer/BUILD.gn

@@ -228,9 +228,11 @@ jumbo_static_library("weblayer_lib") {
     "//cc",
     "//components/crash/content/app",
     "//components/crash/content/browser",
+    "//components/prefs",
     "//components/security_interstitials/content:security_interstitial_page",
     "//components/security_interstitials/content/renderer:security_interstitial_page_controller",
     "//components/security_interstitials/core",
+    "//components/user_prefs",
     "//components/version_info",
     "//components/web_cache/browser",
     "//content:resources",
@@ -282,14 +284,12 @@ jumbo_static_library("weblayer_lib") {
       "//android_webview:generate_aw_resources",
       "//android_webview:generate_aw_strings",
       "//components/crash/android:crashpad_main",
-      "//components/prefs",
       "//components/safe_browsing",
       "//components/safe_browsing/android:remote_database_manager",
       "//components/safe_browsing/android:safe_browsing_api_handler",
       "//components/safe_browsing/browser",
       "//components/safe_browsing/browser:network_context",
       "//components/safe_browsing/db:database_manager",
-      "//components/user_prefs",
       "//components/version_info/android:channel_getter",
     ]
   }

weblayer/browser/content_browser_client_impl.cc

@@ -241,7 +241,7 @@ ContentBrowserClientImpl::CreateURLLoaderThrottles(
       // Note: Initialize() needs to happen on UI thread.
       safe_browsing_service_ =
           std::make_unique<SafeBrowsingService>(GetUserAgent());
-      safe_browsing_service_->Initialize(browser_context);
+      safe_browsing_service_->Initialize();
     }
 
     result.push_back(safe_browsing_service_->CreateURLLoaderThrottle(

weblayer/browser/profile_impl.cc

@@ -11,6 +11,12 @@
 #include "base/strings/string_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "build/build_config.h"
+#include "components/prefs/in_memory_pref_store.h"
+#include "components/prefs/pref_registry_simple.h"
+#include "components/prefs/pref_service.h"
+#include "components/prefs/pref_service_factory.h"
+#include "components/safe_browsing/common/safe_browsing_prefs.h"
+#include "components/user_prefs/user_prefs.h"
 #include "components/web_cache/browser/web_cache_manager.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browsing_data_remover.h"
@@ -97,6 +103,8 @@ class ProfileImpl::BrowserContextImpl : public content::BrowserContext {
   BrowserContextImpl(const base::FilePath& path) : path_(path) {
     resource_context_ = std::make_unique<ResourceContextImpl>();
     content::BrowserContext::Initialize(this, path_);
+
+    CreateUserPrefService();
   }
 
   ~BrowserContextImpl() override { NotifyWillBeDestroyed(this); }
@@ -170,10 +178,32 @@ class ProfileImpl::BrowserContextImpl : public content::BrowserContext {
   }
 
  private:
+  // Creates a simple in-memory pref service.
+  // TODO(timvolodine): Investigate whether WebLayer needs persistent pref
+  // service.
+  void CreateUserPrefService() {
+    auto pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
+    RegisterPrefs(pref_registry.get());
+
+    PrefServiceFactory pref_service_factory;
+    pref_service_factory.set_user_prefs(
+        base::MakeRefCounted<InMemoryPrefStore>());
+    user_pref_service_ = pref_service_factory.Create(pref_registry);
+    // Note: UserPrefs::Set also ensures that the user_pref_service_ has not
+    // been set previously.
+    user_prefs::UserPrefs::Set(this, user_pref_service_.get());
+  }
+
+  // Registers the preferences that WebLayer accesses.
+  void RegisterPrefs(PrefRegistrySimple* pref_registry) {
+    safe_browsing::RegisterProfilePrefs(pref_registry);
+  }
+
   base::FilePath path_;
   std::unique_ptr<ResourceContextImpl> resource_context_;
   DownloadManagerDelegateImpl download_delegate_;
   SSLHostStateDelegateImpl ssl_host_state_delegate_;
+  std::unique_ptr<PrefService> user_pref_service_;
 
   DISALLOW_COPY_AND_ASSIGN(BrowserContextImpl);
 };

weblayer/browser/safe_browsing/safe_browsing_service.cc

@@ -7,17 +7,11 @@
 #include "base/bind.h"
 #include "base/path_service.h"
 #include "base/task/post_task.h"
-#include "components/prefs/in_memory_pref_store.h"
-#include "components/prefs/pref_registry_simple.h"
-#include "components/prefs/pref_service.h"
-#include "components/prefs/pref_service_factory.h"
 #include "components/safe_browsing/android/remote_database_manager.h"
 #include "components/safe_browsing/android/safe_browsing_api_handler_bridge.h"
 #include "components/safe_browsing/browser/browser_url_loader_throttle.h"
 #include "components/safe_browsing/browser/mojo_safe_browsing_impl.h"
 #include "components/safe_browsing/browser/safe_browsing_network_context.h"
-#include "components/user_prefs/user_prefs.h"
-#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_context.h"
@@ -44,7 +38,7 @@ SafeBrowsingService::SafeBrowsingService(const std::string& user_agent)
 
 SafeBrowsingService::~SafeBrowsingService() {}
 
-void SafeBrowsingService::Initialize(content::BrowserContext* browser_context) {
+void SafeBrowsingService::Initialize() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   if (network_context_) {
@@ -52,23 +46,6 @@ void SafeBrowsingService::Initialize(content::BrowserContext* browser_context) {
     return;
   }
 
-  DCHECK(browser_context);
-
-  // Create a simple in-memory pref service, and register safe-browsing
-  // preferences. Currently this is only required for safebrowsing to work (due
-  // to RealTimePolicyEngine::IsEnabledByPolicy being checked during
-  // safebrowsing throttle creation).
-  // TODO(timvolodine): separate this out into general persistent pref service.
-  auto pref_registry = base::MakeRefCounted<PrefRegistrySimple>();
-  safe_browsing::RegisterProfilePrefs(pref_registry.get());
-  PrefServiceFactory pref_service_factory;
-  pref_service_factory.set_user_prefs(
-      base::MakeRefCounted<InMemoryPrefStore>());
-  user_pref_service_ = pref_service_factory.Create(pref_registry);
-  // Note: UserPrefs::Set also ensures that the user_pref_service_ has not been
-  // set previously.
-  user_prefs::UserPrefs::Set(browser_context, user_pref_service_.get());
-
   safe_browsing_api_handler_.reset(
       new safe_browsing::SafeBrowsingApiHandlerBridge());
   safe_browsing::SafeBrowsingApiHandler::SetInstance(

weblayer/browser/safe_browsing/safe_browsing_service.h

@@ -12,7 +12,6 @@
 #include "weblayer/browser/safe_browsing/safe_browsing_ui_manager.h"
 
 namespace content {
-class BrowserContext;
 class ResourceContext;
 }
 
@@ -42,7 +41,7 @@ class SafeBrowsingService {
   ~SafeBrowsingService();
 
   // Executed on UI thread
-  void Initialize(content::BrowserContext* browser_context);
+  void Initialize();
   std::unique_ptr<blink::URLLoaderThrottle> CreateURLLoaderThrottle(
       content::ResourceContext* resource_context,
       const base::RepeatingCallback<content::WebContents*()>& wc_getter,
@@ -86,8 +85,6 @@ class SafeBrowsingService {
   std::unique_ptr<safe_browsing::SafeBrowsingApiHandler>
       safe_browsing_api_handler_;
 
-  std::unique_ptr<PrefService> user_pref_service_;
-
   std::string user_agent_;
 
   DISALLOW_COPY_AND_ASSIGN(SafeBrowsingService);