Get per-profile SB cookie on chrome://safe-browsing

This CL modifies the way the SafeBrowsingUIHandler retrieves the SB
cookie so that we can display the correct cookie per-profile.

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

chrome/browser/safe_browsing/safe_browsing_service.cc

@@ -45,6 +45,7 @@
 #include "components/safe_browsing/core/ping_manager.h"
 #include "components/safe_browsing/core/realtime/policy_engine.h"
 #include "components/safe_browsing/core/triggers/trigger_manager.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "services/network/public/cpp/cross_thread_pending_shared_url_loader_factory.h"
@@ -114,8 +115,7 @@ void SafeBrowsingService::Initialize() {
           base::BindRepeating(&SafeBrowsingService::CreateNetworkContextParams,
                               base::Unretained(this)));
 
-  WebUIInfoSingleton::GetInstance()->set_network_context(
-      network_context_.get());
+  WebUIInfoSingleton::GetInstance()->set_safe_browsing_service(this);
 
   ui_manager_ = CreateUIManager();
 
@@ -155,6 +155,8 @@ void SafeBrowsingService::ShutDown() {
 
   services_delegate_->ShutdownServices();
 
+  WebUIInfoSingleton::GetInstance()->set_safe_browsing_service(nullptr);
+
   network_context_->ServiceShuttingDown();
   proxy_config_monitor_.reset();
 }
@@ -173,12 +175,14 @@ SafeBrowsingService::GetURLLoaderFactory() {
 }
 
 network::mojom::NetworkContext* SafeBrowsingService::GetNetworkContext(
-    Profile* profile) {
+    content::BrowserContext* browser_context) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (!base::FeatureList::IsEnabled(kSafeBrowsingSeparateNetworkContexts))
     return GetNetworkContext();
 
-  return services_delegate_->GetSafeBrowsingNetworkContext(profile)
+  return services_delegate_
+      ->GetSafeBrowsingNetworkContext(
+          Profile::FromBrowserContext(browser_context))
       ->GetNetworkContext();
 }
 

chrome/browser/safe_browsing/safe_browsing_service.h

@@ -129,9 +129,10 @@ class SafeBrowsingService : public SafeBrowsingServiceInterface,
   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);
+  // Get the NetworkContext or URLLoaderFactory attached to |browser_context|.
+  // Called on UI thread.
+  network::mojom::NetworkContext* GetNetworkContext(
+      content::BrowserContext* browser_context) override;
   virtual scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory(
       Profile* profile);
 

components/safe_browsing/content/web_ui/BUILD.gn

@@ -19,6 +19,7 @@ static_library("web_ui") {
     "//components/safe_browsing:buildflags",
     "//components/safe_browsing/core:csd_proto",
     "//components/safe_browsing/core:features",
+    "//components/safe_browsing/core:public",
     "//components/safe_browsing/core:realtimeapi_proto",
     "//components/safe_browsing/core:webprotect_proto",
     "//components/safe_browsing/core/browser:network_context",

components/safe_browsing/content/web_ui/safe_browsing_ui.cc

@@ -32,6 +32,7 @@
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/safe_browsing/core/features.h"
 #include "components/safe_browsing/core/proto/csd.pb.h"
+#include "services/network/public/mojom/cookie_manager.mojom.h"
 #if BUILDFLAG(FULL_SAFE_BROWSING)
 #include "components/enterprise/common/proto/connectors.pb.h"
 #include "components/safe_browsing/core/proto/webprotect.pb.h"
@@ -342,11 +343,15 @@ void WebUIInfoSingleton::UnregisterWebUIInstance(SafeBrowsingUIHandler* webui) {
   MaybeClearData();
 }
 
-network::mojom::CookieManager* WebUIInfoSingleton::GetCookieManager() {
-  if (!cookie_manager_remote_)
-    InitializeCookieManager();
+mojo::Remote<network::mojom::CookieManager>
+WebUIInfoSingleton::GetCookieManager(content::BrowserContext* browser_context) {
+  mojo::Remote<network::mojom::CookieManager> cookie_manager_remote;
+  if (sb_service_) {
+    sb_service_->GetNetworkContext(browser_context)
+        ->GetCookieManager(cookie_manager_remote.BindNewPipeAndPassReceiver());
+  }
 
-  return cookie_manager_remote_.get();
+  return cookie_manager_remote;
 }
 
 void WebUIInfoSingleton::ClearListenerForTesting() {
@@ -354,23 +359,6 @@ void WebUIInfoSingleton::ClearListenerForTesting() {
   MaybeClearData();
 }
 
-void WebUIInfoSingleton::InitializeCookieManager() {
-  DCHECK(network_context_);
-
-  // Reset |cookie_manager_remote_|, and only re-initialize it if we have a
-  // listening SafeBrowsingUIHandler.
-  cookie_manager_remote_.reset();
-
-  if (HasListener()) {
-    network_context_->GetNetworkContext()->GetCookieManager(
-        cookie_manager_remote_.BindNewPipeAndPassReceiver());
-
-    // base::Unretained is safe because |this| owns |cookie_manager_remote_|.
-    cookie_manager_remote_.set_disconnect_handler(base::BindOnce(
-        &WebUIInfoSingleton::InitializeCookieManager, base::Unretained(this)));
-  }
-}
-
 void WebUIInfoSingleton::MaybeClearData() {
   if (!HasListener()) {
     ClearCSBRRsSent();
@@ -1717,7 +1705,9 @@ void SafeBrowsingUIHandler::GetCookie(const base::ListValue* args) {
   std::string callback_id;
   args->GetString(0, &callback_id);
 
-  WebUIInfoSingleton::GetInstance()->GetCookieManager()->GetAllCookies(
+  cookie_manager_remote_ =
+      WebUIInfoSingleton::GetInstance()->GetCookieManager(browser_context_);
+  cookie_manager_remote_->GetAllCookies(
       base::BindOnce(&SafeBrowsingUIHandler::OnGetCookie,
                      weak_factory_.GetWeakPtr(), std::move(callback_id)));
 }

components/safe_browsing/content/web_ui/safe_browsing_ui.h

@@ -13,6 +13,7 @@
 #include "components/safe_browsing/core/proto/csd.pb.h"
 #include "components/safe_browsing/core/proto/realtimeapi.pb.h"
 #include "components/safe_browsing/core/proto/webui.pb.h"
+#include "components/safe_browsing/core/safe_browsing_service_interface.h"
 #include "components/sync/protocol/user_event_specifics.pb.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/browser/web_ui_data_source.h"
@@ -222,6 +223,8 @@ class SafeBrowsingUIHandler : public content::WebUIMessageHandler {
 
   content::BrowserContext* browser_context_;
 
+  mojo::Remote<network::mojom::CookieManager> cookie_manager_remote_;
+
   // List that keeps all the WebUI listener objects.
   static std::vector<SafeBrowsingUIHandler*> webui_list_;
 
@@ -445,10 +448,11 @@ class WebUIInfoSingleton {
     return reporting_events_;
   }
 
-  network::mojom::CookieManager* GetCookieManager();
+  mojo::Remote<network::mojom::CookieManager> GetCookieManager(
+      content::BrowserContext* browser_context);
 
-  void set_network_context(SafeBrowsingNetworkContext* network_context) {
-    network_context_ = network_context;
+  void set_safe_browsing_service(SafeBrowsingServiceInterface* sb_service) {
+    sb_service_ = sb_service;
   }
 
   void AddListenerForTesting() { has_test_listener_ = true; }
@@ -459,8 +463,6 @@ class WebUIInfoSingleton {
   WebUIInfoSingleton();
   ~WebUIInfoSingleton();
 
-  void InitializeCookieManager();
-
   void MaybeClearData();
 
   friend struct base::DefaultSingletonTraits<WebUIInfoSingleton>;
@@ -532,11 +534,8 @@ class WebUIInfoSingleton {
   // The current referrer chain provider, if any. Can be nullptr.
   ReferrerChainProvider* referrer_chain_provider_ = nullptr;
 
-  // The current NetworkContext for Safe Browsing pings.
-  SafeBrowsingNetworkContext* network_context_ = nullptr;
-
-  // The current CookieManager for the Safe Browsing cookie.
-  mojo::Remote<network::mojom::CookieManager> cookie_manager_remote_;
+  // The Safe Browsing service.
+  SafeBrowsingServiceInterface* sb_service_ = nullptr;
 
   // Whether there is a test listener.
   bool has_test_listener_ = false;

components/safe_browsing/core/safe_browsing_service_interface.h

@@ -7,7 +7,9 @@
 
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
+#include "services/network/public/mojom/network_context.mojom.h"
 
 namespace safe_browsing {
 
@@ -31,6 +33,9 @@ class SafeBrowsingServiceInterface
   // Create an instance of the safe browsing service.
   static SafeBrowsingServiceInterface* CreateSafeBrowsingService();
 
+  virtual network::mojom::NetworkContext* GetNetworkContext(
+      content::BrowserContext* browser_context) = 0;
+
  protected:
   SafeBrowsingServiceInterface() {}
   virtual ~SafeBrowsingServiceInterface() {}