Revert "Remove remaining components/domain_reliability dependency on content/...

Revert "Remove remaining components/domain_reliability dependency on content/ and components/keyed_service."

This reverts commit bd611e3e.

Reason for revert: It looks like this CL broke CFI bots:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20CFI/11301

Original change's description:
> Remove remaining components/domain_reliability dependency on content/ and components/keyed_service.
> 
> This way it can run in the network process, similar to Network Error Logging.
> 
> Bug: 853251
> Change-Id: I403a5dcc3cae12b592a444c7e23f71c552b58e1f
> Reviewed-on: https://chromium-review.googlesource.com/c/1334848
> Reviewed-by: Misha Efimov <mef@chromium.org>
> Commit-Queue: John Abd-El-Malek <jam@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#608175}

TBR=jam@chromium.org,mef@chromium.org

Change-Id: I57829e413ad57ec667882069f77bf2d97faec4a3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 853251
Reviewed-on: https://chromium-review.googlesource.com/c/1337215Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608268}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -496,9 +496,8 @@ class MockDomainReliabilityService : public DomainReliabilityService {
 
   std::unique_ptr<DomainReliabilityMonitor> CreateMonitor(
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
-      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
-      const domain_reliability::DomainReliabilityContext::UploadAllowedCallback&
-          upload_allowed_callback) override {
+      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner)
+      override {
     NOTREACHED();
     return std::unique_ptr<DomainReliabilityMonitor>();
   }
@@ -546,21 +545,6 @@ class MockDomainReliabilityService : public DomainReliabilityService {
   base::Callback<bool(const GURL&)> last_filter_;
 };
 
-class DomainReliablityKeyedServiceWrapper : public KeyedService {
- public:
-  explicit DomainReliablityKeyedServiceWrapper(
-      std::unique_ptr<DomainReliabilityService> service)
-      : service_(std::move(service)) {}
-  ~DomainReliablityKeyedServiceWrapper() override = default;
-
-  DomainReliabilityService* service() { return service_.get(); }
-
- private:
-  std::unique_ptr<DomainReliabilityService> service_;
-
-  DISALLOW_COPY_AND_ASSIGN(DomainReliablityKeyedServiceWrapper);
-};
-
 struct TestingDomainReliabilityServiceFactoryUserData
     : public base::SupportsUserData::Data {
   TestingDomainReliabilityServiceFactoryUserData(
@@ -594,8 +578,7 @@ std::unique_ptr<KeyedService> TestingDomainReliabilityServiceFactoryFunction(
   EXPECT_FALSE(data->attached);
 
   data->attached = true;
-  return std::make_unique<DomainReliablityKeyedServiceWrapper>(
-      base::WrapUnique(data->service));
+  return base::WrapUnique(data->service);
 }
 
 std::unique_ptr<KeyedService> BuildProtocolHandlerRegistry(

chrome/browser/domain_reliability/service_factory.cc

@@ -11,7 +11,6 @@
 #include "chrome/common/chrome_switches.h"
 #include "components/domain_reliability/service.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
-#include "components/keyed_service/core/keyed_service.h"
 #include "content/public/browser/browser_context.h"
 
 #if defined(OS_CHROMEOS)
@@ -32,32 +31,15 @@ const char kFieldTrialValueEnable[] = "enable";
 // Identifies Chrome as the source of Domain Reliability uploads it sends.
 const char kUploadReporterString[] = "chrome";
 
-class KeyedServiceWrapper : public KeyedService {
- public:
-  explicit KeyedServiceWrapper(DomainReliabilityService* service)
-      : service_(service) {}
-  ~KeyedServiceWrapper() override = default;
-
-  DomainReliabilityService* service() { return service_; }
-
- private:
-  DomainReliabilityService* service_;
-
-  DISALLOW_COPY_AND_ASSIGN(KeyedServiceWrapper);
-};
-
 }  // namespace
 
 // static
 DomainReliabilityService*
 DomainReliabilityServiceFactory::GetForBrowserContext(
     content::BrowserContext* context) {
-  auto* wrapper = static_cast<KeyedServiceWrapper*>(
+  return static_cast<DomainReliabilityService*>(
       GetInstance()->GetServiceForBrowserContext(context,
                                                  /* create = */ true));
-  if (!wrapper)
-    return nullptr;
-  return wrapper->service();
 }
 
 // static
@@ -78,8 +60,7 @@ KeyedService* DomainReliabilityServiceFactory::BuildServiceInstanceFor(
   if (!ShouldCreateService())
     return NULL;
 
-  return new KeyedServiceWrapper(
-      DomainReliabilityService::Create(kUploadReporterString));
+  return DomainReliabilityService::Create(kUploadReporterString, context);
 }
 
 bool DomainReliabilityServiceFactory::ShouldCreateService() const {

chrome/browser/profiles/profile_impl.cc

@@ -123,8 +123,6 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/dom_storage_context.h"
 #include "content/public/browser/notification_service.h"
-#include "content/public/browser/permission_controller.h"
-#include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/url_data_source.h"
@@ -328,31 +326,6 @@ std::unique_ptr<service_manager::Service> CreateAppService(Profile* profile) {
 }
 #endif
 
-void CheckDomainReliablityUploadAllowedOnUIThread(
-    Profile* profile,
-    const GURL& origin,
-    base::OnceCallback<void(bool)> callback) {
-  // Profile is safe since ProfileImpl always outlives IO thread.
-  content::PermissionController* permission_controller =
-      content::BrowserContext::GetPermissionController(profile);
-  DCHECK(permission_controller);
-  bool allowed = permission_controller->GetPermissionStatus(
-                     content::PermissionType::BACKGROUND_SYNC, origin,
-                     origin) == blink::mojom::PermissionStatus::GRANTED;
-  base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO},
-                           base::BindOnce(std::move(callback), allowed));
-}
-
-void CheckDomainReliablityUploadAllowedOnIOThread(
-    Profile* profile,
-    const GURL& origin,
-    base::OnceCallback<void(bool)> callback) {
-  base::PostTaskWithTraits(
-      FROM_HERE, {BrowserThread::UI},
-      base::BindOnce(CheckDomainReliablityUploadAllowedOnUIThread, profile,
-                     origin, std::move(callback)));
-}
-
 }  // namespace
 
 // static
@@ -1503,8 +1476,7 @@ ProfileImpl::CreateDomainReliabilityMonitor(PrefService* local_state) {
 
   return service->CreateMonitor(
       base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::UI}),
-      base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::IO}),
-      base::BindRepeating(CheckDomainReliablityUploadAllowedOnIOThread, this));
+      base::CreateSingleThreadTaskRunnerWithTraits({BrowserThread::IO}));
 }
 
 std::unique_ptr<service_manager::Service> ProfileImpl::CreateIdentityService() {

chrome/browser/profiles/profile_impl_io_data.cc

@@ -174,8 +174,7 @@ void ProfileImplIOData::Handle::Init(
 
   io_data_->InitializeMetricsEnabledStateOnUIThread();
   if (io_data_->lazy_params_->domain_reliability_monitor)
-    io_data_->lazy_params_->domain_reliability_monitor
-        ->InitializeOnNetworkThread();
+    io_data_->lazy_params_->domain_reliability_monitor->MoveToNetworkThread();
 
   io_data_->set_data_reduction_proxy_io_data(
       CreateDataReductionProxyChromeIOData(

components/domain_reliability/BUILD.gn

@@ -86,6 +86,9 @@ jumbo_component("domain_reliability") {
   deps = [
     ":bake_in_configs",
     "//base",
+    "//components/keyed_service/core",
+    "//content/public/browser",
+    "//content/public/common",
     "//net",
     "//url",
   ]
@@ -114,6 +117,8 @@ jumbo_source_set("unit_tests") {
     ":domain_reliability",
     "//base",
     "//base/test:test_support",
+    "//content/public/browser",
+    "//content/test:test_support",
     "//net:test_support",
     "//testing/gtest",
   ]

components/domain_reliability/DEPS

@@ -4,5 +4,9 @@
 
 include_rules = [
   "+net",
+  "+components/keyed_service/core",
+  "+content/public/browser",
+  "+content/public/test",
+  "+third_party/blink/public",
 ]
 

components/domain_reliability/monitor.cc

@@ -78,7 +78,7 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
     const std::string& upload_reporter_string,
     const DomainReliabilityContext::UploadAllowedCallback&
         upload_allowed_callback,
-    const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
+    const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
     const scoped_refptr<base::SingleThreadTaskRunner>& network_thread)
     : time_(new ActualTime()),
       upload_reporter_string_(upload_reporter_string),
@@ -87,19 +87,19 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
           DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
       dispatcher_(time_.get()),
       context_manager_(this),
-      main_task_runner_(main_thread),
+      pref_task_runner_(pref_thread),
       network_task_runner_(network_thread),
       moved_to_network_thread_(false),
       discard_uploads_set_(false),
       weak_factory_(this) {
-  DCHECK(OnMainThread());
+  DCHECK(OnPrefThread());
 }
 
 DomainReliabilityMonitor::DomainReliabilityMonitor(
     const std::string& upload_reporter_string,
     const DomainReliabilityContext::UploadAllowedCallback&
         upload_allowed_callback,
-    const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
+    const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
     const scoped_refptr<base::SingleThreadTaskRunner>& network_thread,
     std::unique_ptr<MockableTime> time)
     : time_(std::move(time)),
@@ -109,12 +109,12 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
           DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
       dispatcher_(time_.get()),
       context_manager_(this),
-      main_task_runner_(main_thread),
+      pref_task_runner_(pref_thread),
       network_task_runner_(network_thread),
       moved_to_network_thread_(false),
       discard_uploads_set_(false),
       weak_factory_(this) {
-  DCHECK(OnMainThread());
+  DCHECK(OnPrefThread());
 }
 
 DomainReliabilityMonitor::~DomainReliabilityMonitor() {
@@ -122,12 +122,12 @@ DomainReliabilityMonitor::~DomainReliabilityMonitor() {
     net::NetworkChangeNotifier::RemoveNetworkChangeObserver(this);
     DCHECK(OnNetworkThread());
   } else {
-    DCHECK(OnMainThread());
+    DCHECK(OnPrefThread());
   }
 }
 
-void DomainReliabilityMonitor::InitializeOnNetworkThread() {
-  DCHECK(OnMainThread());
+void DomainReliabilityMonitor::MoveToNetworkThread() {
+  DCHECK(OnPrefThread());
   DCHECK(!moved_to_network_thread_);
 
   network_task_runner_->PostTask(

components/domain_reliability/monitor.h

@@ -51,14 +51,14 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor
     : public net::NetworkChangeNotifier::NetworkChangeObserver,
       DomainReliabilityContext::Factory {
  public:
-  // Creates a Monitor. |main_thread| is the current thread, which may or may
-  // not be the same as |network_thread|. |network_thread| is the thread
+  // Creates a Monitor. |local_state_pref_service| must live on |pref_thread|
+  // (which should be the current thread); |network_thread| is the thread
   // on which requests will actually be monitored and reported.
   DomainReliabilityMonitor(
       const std::string& upload_reporter_string,
       const DomainReliabilityContext::UploadAllowedCallback&
           upload_allowed_callback,
-      const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
+      const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
       const scoped_refptr<base::SingleThreadTaskRunner>& network_thread);
 
   // Same, but specifies a mock interface for time functions for testing.
@@ -66,25 +66,25 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor
       const std::string& upload_reporter_string,
       const DomainReliabilityContext::UploadAllowedCallback&
           upload_allowed_callback,
-      const scoped_refptr<base::SingleThreadTaskRunner>& main_thread,
+      const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
       const scoped_refptr<base::SingleThreadTaskRunner>& network_thread,
       std::unique_ptr<MockableTime> time);
 
-  // Must be called from the main thread if |InitializeOnNetworkThread| was not
+  // Must be called from the pref thread if |MoveToNetworkThread| was not
   // called, or from the network thread if it was called.
   ~DomainReliabilityMonitor() override;
 
   // Must be called before |InitURLRequestContext| on the same thread on which
   // the Monitor was constructed. Moves (most of) the Monitor to the network
   // thread passed in the constructor.
-  void InitializeOnNetworkThread();
+  void MoveToNetworkThread();
 
   // All public methods below this point must be called on the network thread
-  // after |InitializeOnNetworkThread| is called on the main thread.
+  // after |MoveToNetworkThread| is called on the pref thread.
 
   // Initializes the Monitor's URLRequestContextGetter.
   //
-  // Must be called on the network thread, after |InitializeOnNetworkThread|.
+  // Must be called on the network thread, after |MoveToNetworkThread|.
   void InitURLRequestContext(net::URLRequestContext* url_request_context);
 
   // Same, but for unittests where the Getter is readily available.
@@ -177,8 +177,8 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor
 
   void MaybeHandleHeader(const RequestInfo& info);
 
-  bool OnMainThread() const {
-    return main_task_runner_->BelongsToCurrentThread();
+  bool OnPrefThread() const {
+    return pref_task_runner_->BelongsToCurrentThread();
   }
   bool OnNetworkThread() const {
     return network_task_runner_->BelongsToCurrentThread();
@@ -195,7 +195,7 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor
   std::unique_ptr<DomainReliabilityUploader> uploader_;
   DomainReliabilityContextManager context_manager_;
 
-  scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> pref_task_runner_;
   scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
 
   bool moved_to_network_thread_;

components/domain_reliability/monitor_unittest.cc

@@ -54,17 +54,17 @@ class DomainReliabilityMonitorTest : public testing::Test {
   typedef DomainReliabilityMonitor::RequestInfo RequestInfo;
 
   DomainReliabilityMonitorTest()
-      : main_task_runner(new base::TestSimpleTaskRunner()),
+      : pref_task_runner_(new base::TestSimpleTaskRunner()),
         network_task_runner_(new base::TestSimpleTaskRunner()),
         url_request_context_getter_(
             new net::TestURLRequestContextGetter(network_task_runner_)),
         time_(new MockTime()),
         monitor_("test-reporter",
                  DomainReliabilityContext::UploadAllowedCallback(),
-                 main_task_runner,
+                 pref_task_runner_,
                  network_task_runner_,
                  std::unique_ptr<MockableTime>(time_)) {
-    monitor_.InitializeOnNetworkThread();
+    monitor_.MoveToNetworkThread();
     monitor_.InitURLRequestContext(url_request_context_getter_);
     monitor_.SetDiscardUploads(false);
   }
@@ -104,7 +104,7 @@ class DomainReliabilityMonitorTest : public testing::Test {
     return monitor_.AddContextForTesting(std::move(config));
   }
 
-  scoped_refptr<base::TestSimpleTaskRunner> main_task_runner;
+  scoped_refptr<base::TestSimpleTaskRunner> pref_task_runner_;
   scoped_refptr<base::TestSimpleTaskRunner> network_task_runner_;
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
   MockTime* time_;

components/domain_reliability/service.cc

@@ -15,7 +15,11 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "components/domain_reliability/context.h"
 #include "components/domain_reliability/monitor.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/permission_controller.h"
+#include "content/public/browser/permission_type.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "third_party/blink/public/platform/modules/permissions/permission_status.mojom.h"
 #include "url/gurl.h"
 
 namespace domain_reliability {
@@ -47,8 +51,11 @@ std::unique_ptr<base::Value> GetWebUIDataOnNetworkTaskRunner(
 class DomainReliabilityServiceImpl : public DomainReliabilityService {
  public:
   explicit DomainReliabilityServiceImpl(
-      const std::string& upload_reporter_string)
-      : upload_reporter_string_(upload_reporter_string), weak_factory_(this) {}
+      const std::string& upload_reporter_string,
+      content::BrowserContext* browser_context)
+      : upload_reporter_string_(upload_reporter_string),
+        browser_context_(browser_context),
+        weak_factory_(this) {}
 
   ~DomainReliabilityServiceImpl() override {}
 
@@ -56,15 +63,18 @@ class DomainReliabilityServiceImpl : public DomainReliabilityService {
 
   std::unique_ptr<DomainReliabilityMonitor> CreateMonitor(
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
-      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
-      const DomainReliabilityContext::UploadAllowedCallback&
-          upload_allowed_callback) override {
+      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner)
+      override {
     DCHECK(!network_task_runner_);
 
     std::unique_ptr<DomainReliabilityMonitor> monitor(
-        new DomainReliabilityMonitor(upload_reporter_string_,
-                                     upload_allowed_callback, ui_task_runner,
-                                     network_task_runner));
+        new DomainReliabilityMonitor(
+            upload_reporter_string_,
+            base::Bind(
+                &DomainReliabilityServiceImpl::CheckUploadAllowedOnIOThread,
+                weak_factory_.GetWeakPtr(), base::RetainedRef(ui_task_runner),
+                base::RetainedRef(network_task_runner)),
+            ui_task_runner, network_task_runner));
 
     monitor_ = monitor->MakeWeakPtr();
     network_task_runner_ = network_task_runner;
@@ -123,7 +133,36 @@ class DomainReliabilityServiceImpl : public DomainReliabilityService {
   }
 
  private:
+  static void CheckUploadAllowedOnIOThread(
+      base::WeakPtr<DomainReliabilityServiceImpl> service,
+      scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
+      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
+      const GURL& origin,
+      base::OnceCallback<void(bool)> callback) {
+    ui_task_runner->PostTask(
+        FROM_HERE,
+        base::BindOnce(
+            &DomainReliabilityServiceImpl::CheckUploadAllowedOnUIThread,
+            service, base::RetainedRef(network_task_runner), origin,
+            std::move(callback)));
+  }
+
+  void CheckUploadAllowedOnUIThread(
+      base::SingleThreadTaskRunner* network_task_runner,
+      const GURL& origin,
+      base::OnceCallback<void(bool)> callback) {
+    content::PermissionController* permission_controller =
+        content::BrowserContext::GetPermissionController(browser_context_);
+    DCHECK(permission_controller);
+    bool allowed = permission_controller->GetPermissionStatus(
+                       content::PermissionType::BACKGROUND_SYNC, origin,
+                       origin) == blink::mojom::PermissionStatus::GRANTED;
+    network_task_runner->PostTask(FROM_HERE,
+                                  base::BindOnce(std::move(callback), allowed));
+  }
+
   std::string upload_reporter_string_;
+  content::BrowserContext* browser_context_;
   base::WeakPtr<DomainReliabilityMonitor> monitor_;
   scoped_refptr<base::SingleThreadTaskRunner> network_task_runner_;
   base::WeakPtrFactory<DomainReliabilityServiceImpl> weak_factory_;
@@ -131,8 +170,10 @@ class DomainReliabilityServiceImpl : public DomainReliabilityService {
 
 // static
 DomainReliabilityService* DomainReliabilityService::Create(
-    const std::string& upload_reporter_string) {
-  return new DomainReliabilityServiceImpl(upload_reporter_string);
+    const std::string& upload_reporter_string,
+    content::BrowserContext* browser_context) {
+  return new DomainReliabilityServiceImpl(upload_reporter_string,
+                                          browser_context);
 }
 
 DomainReliabilityService::~DomainReliabilityService() {}

components/domain_reliability/service.h

@@ -16,6 +16,7 @@
 #include "components/domain_reliability/config.h"
 #include "components/domain_reliability/context.h"
 #include "components/domain_reliability/domain_reliability_export.h"
+#include "components/keyed_service/core/keyed_service.h"
 
 class GURL;
 
@@ -23,6 +24,10 @@ namespace base {
 class Value;
 }  // namespace base
 
+namespace content {
+class BrowserContext;
+}  // namespace content
+
 namespace net {
 class URLRequestContextGetter;
 }  // namespace net
@@ -31,30 +36,29 @@ namespace domain_reliability {
 
 class DomainReliabilityMonitor;
 
-// DomainReliabilityService manages a Monitor that (could) live on another
-// thread (as provided by the URLRequestContextGetter's task runner) and
-// proxies (selected) method calls to it. Destruction of the Monitor (on that
-// thread) is the responsibility of the caller.
-class DOMAIN_RELIABILITY_EXPORT DomainReliabilityService {
+// DomainReliabilityService is a KeyedService that manages a Monitor that lives
+// on another thread (as provided by the URLRequestContextGetter's task runner)
+// and proxies (selected) method calls to it. Destruction of the Monitor (on
+// that thread) is the responsibility of the caller.
+class DOMAIN_RELIABILITY_EXPORT DomainReliabilityService
+    : public KeyedService {
  public:
   // Creates a DomainReliabilityService that will contain a Monitor with the
   // given upload reporter string.
   static DomainReliabilityService* Create(
-      const std::string& upload_reporter_string);
+      const std::string& upload_reporter_string,
+      content::BrowserContext* browser_context);
 
-  virtual ~DomainReliabilityService();
+  ~DomainReliabilityService() override;
 
   // Initializes the Service: given the task runner on which Monitor methods
   // should be called, creates the Monitor and returns it. Can be called at
   // most once, and must be called before any of the below methods can be
   // called. The caller is responsible for destroying the Monitor on the given
-  // task runner when it is no longer needed. |upload_allowed_callback| is
-  // called on the network thread before every upload to check if it's allowed.
+  // task runner when it is no longer needed.
   virtual std::unique_ptr<DomainReliabilityMonitor> CreateMonitor(
       scoped_refptr<base::SingleThreadTaskRunner> ui_task_runner,
-      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner,
-      const DomainReliabilityContext::UploadAllowedCallback&
-          upload_allowed_callback) = 0;
+      scoped_refptr<base::SingleThreadTaskRunner> network_task_runner) = 0;
 
   // Clears browsing data on the associated Monitor. |Init()| must have been
   // called first.