[BackgroundSync] Add delegate.

Add a BackgroundSync delegate to let components embedder customize the
logic in components/background_sync.

Use this delegate to customize UKM logging logic, and remove the
dependency on UkmBackgroundService from BackgroundSyncControllerImpl,
which will eventually be moved to components.

background_sync_metrics* will be moved to components/ in a follow-up CL.

Bug: 1087486
Change-Id: Ia6221f9db842ba8a2da90a1ef3fe721bc113b858
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2443089Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814301}

chrome/browser/BUILD.gn

@@ -163,6 +163,8 @@ static_library("browser") {
     "background_sync/background_sync_controller_factory.h",
     "background_sync/background_sync_controller_impl.cc",
     "background_sync/background_sync_controller_impl.h",
+    "background_sync/background_sync_delegate_impl.cc",
+    "background_sync/background_sync_delegate_impl.h",
     "background_sync/background_sync_metrics.cc",
     "background_sync/background_sync_metrics.h",
     "background_sync/periodic_background_sync_permission_context.cc",

chrome/browser/background_sync/background_sync_controller_impl.cc

@@ -7,10 +7,10 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/time/time.h"
+#include "chrome/browser/background_sync/background_sync_delegate_impl.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/engagement/site_engagement_service.h"
-#include "chrome/browser/metrics/ukm_background_recorder_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
 #include "components/content_settings/core/common/content_settings.h"
@@ -62,10 +62,13 @@ BackgroundSyncControllerImpl::BackgroundSyncControllerImpl(Profile* profile)
     : SiteEngagementObserver(SiteEngagementService::Get(profile)),
       profile_(profile),
       site_engagement_service_(SiteEngagementService::Get(profile)),
-      background_sync_metrics_(
-          ukm::UkmBackgroundRecorderFactory::GetForProfile(profile_)) {
+      background_sync_delegate_(
+          std::make_unique<BackgroundSyncDelegateImpl>(profile_)) {
   DCHECK(profile_);
   DCHECK(site_engagement_service_);
+
+  background_sync_metrics_ =
+      std::make_unique<BackgroundSyncMetrics>(background_sync_delegate_.get());
   HostContentSettingsMapFactory::GetForProfile(profile_)->AddObserver(this);
 }
 
@@ -239,7 +242,7 @@ void BackgroundSyncControllerImpl::NotifyOneShotBackgroundSyncRegistered(
     bool is_reregistered) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  background_sync_metrics_.MaybeRecordOneShotSyncRegistrationEvent(
+  background_sync_metrics_->MaybeRecordOneShotSyncRegistrationEvent(
       origin, can_fire, is_reregistered);
 }
 
@@ -249,7 +252,7 @@ void BackgroundSyncControllerImpl::NotifyPeriodicBackgroundSyncRegistered(
     bool is_reregistered) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  background_sync_metrics_.MaybeRecordPeriodicSyncRegistrationEvent(
+  background_sync_metrics_->MaybeRecordPeriodicSyncRegistrationEvent(
       origin, min_interval, is_reregistered);
 }
 
@@ -260,7 +263,7 @@ void BackgroundSyncControllerImpl::NotifyOneShotBackgroundSyncCompleted(
     int max_attempts) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  background_sync_metrics_.MaybeRecordOneShotSyncCompletionEvent(
+  background_sync_metrics_->MaybeRecordOneShotSyncCompletionEvent(
       origin, status_code, num_attempts, max_attempts);
 }
 
@@ -271,7 +274,7 @@ void BackgroundSyncControllerImpl::NotifyPeriodicBackgroundSyncCompleted(
     int max_attempts) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  background_sync_metrics_.MaybeRecordPeriodicSyncEventCompletion(
+  background_sync_metrics_->MaybeRecordPeriodicSyncEventCompletion(
       origin, status_code, num_attempts, max_attempts);
 }
 

chrome/browser/background_sync/background_sync_controller_impl.h

@@ -16,6 +16,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/background_sync/background_sync_metrics.h"
 #include "chrome/browser/engagement/site_engagement_observer.h"
+#include "components/background_sync/background_sync_delegate.h"
 #include "components/content_settings/core/browser/content_settings_observer.h"
 #include "components/keep_alive_registry/keep_alive_types.h"
 #include "components/keep_alive_registry/scoped_keep_alive.h"
@@ -161,7 +162,9 @@ class BackgroundSyncControllerImpl : public content::BackgroundSyncController,
   // Same lifetime as |profile_|.
   SiteEngagementService* site_engagement_service_;
 
-  BackgroundSyncMetrics background_sync_metrics_;
+  std::unique_ptr<background_sync::BackgroundSyncDelegate>
+      background_sync_delegate_;
+  std::unique_ptr<BackgroundSyncMetrics> background_sync_metrics_;
 
   std::set<url::Origin> suspended_periodic_sync_origins_;
   std::set<url::Origin> periodic_sync_origins_;

chrome/browser/background_sync/background_sync_delegate_impl.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/background_sync/background_sync_delegate_impl.h"
+
+#include "chrome/browser/metrics/ukm_background_recorder_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "url/origin.h"
+
+BackgroundSyncDelegateImpl::BackgroundSyncDelegateImpl(Profile* profile)
+    : ukm_background_service_(
+          ukm::UkmBackgroundRecorderFactory::GetForProfile(profile)) {
+  DCHECK(ukm_background_service_);
+}
+
+BackgroundSyncDelegateImpl::~BackgroundSyncDelegateImpl() = default;
+
+void BackgroundSyncDelegateImpl::GetUkmSourceId(
+    const url::Origin& origin,
+    base::OnceCallback<void(base::Optional<ukm::SourceId>)> callback) {
+  ukm_background_service_->GetBackgroundSourceIdIfAllowed(origin,
+                                                          std::move(callback));
+}

chrome/browser/background_sync/background_sync_delegate_impl.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_DELEGATE_IMPL_H_
+#define CHROME_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_DELEGATE_IMPL_H_
+
+#include "components/background_sync/background_sync_delegate.h"
+
+class Profile;
+
+namespace ukm {
+class UkmBackgroundRecorderService;
+}  // namespace ukm
+
+namespace url {
+class Origin;
+}  // namespace url
+
+// Chrome's customization of the logic in components/background_sync
+class BackgroundSyncDelegateImpl
+    : public background_sync::BackgroundSyncDelegate {
+ public:
+  explicit BackgroundSyncDelegateImpl(Profile* profile);
+  ~BackgroundSyncDelegateImpl() override;
+
+  void GetUkmSourceId(const url::Origin& origin,
+                      base::OnceCallback<void(base::Optional<ukm::SourceId>)>
+                          callback) override;
+
+ private:
+  ukm::UkmBackgroundRecorderService* ukm_background_service_;
+};
+
+#endif  // CHROME_BROWSER_BACKGROUND_SYNC_BACKGROUND_SYNC_DELEGATE_IMPL_H_

chrome/browser/background_sync/background_sync_metrics.cc

@@ -5,16 +5,16 @@
 #include "chrome/browser/background_sync/background_sync_metrics.h"
 
 #include "base/bind.h"
-#include "chrome/browser/metrics/ukm_background_recorder_service.h"
+#include "components/background_sync/background_sync_delegate.h"
 #include "services/metrics/public/cpp/metrics_utils.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
 #include "url/origin.h"
 
 BackgroundSyncMetrics::BackgroundSyncMetrics(
-    ukm::UkmBackgroundRecorderService* ukm_background_service)
-    : ukm_background_service_(ukm_background_service) {
-  DCHECK(ukm_background_service_);
+    background_sync::BackgroundSyncDelegate* delegate)
+    : delegate_(delegate) {
+  DCHECK(delegate_);
 }
 
 BackgroundSyncMetrics::~BackgroundSyncMetrics() = default;
@@ -23,7 +23,8 @@ void BackgroundSyncMetrics::MaybeRecordOneShotSyncRegistrationEvent(
     const url::Origin& origin,
     bool can_fire,
     bool is_reregistered) {
-  ukm_background_service_->GetBackgroundSourceIdIfAllowed(
+  DCHECK(delegate_);
+  delegate_->GetUkmSourceId(
       origin,
       base::BindOnce(
           &BackgroundSyncMetrics::DidGetBackgroundSourceId,
@@ -37,7 +38,8 @@ void BackgroundSyncMetrics::MaybeRecordPeriodicSyncRegistrationEvent(
     const url::Origin& origin,
     int min_interval,
     bool is_reregistered) {
-  ukm_background_service_->GetBackgroundSourceIdIfAllowed(
+  DCHECK(delegate_);
+  delegate_->GetUkmSourceId(
       origin,
       base::BindOnce(
           &BackgroundSyncMetrics::DidGetBackgroundSourceId,
@@ -52,7 +54,8 @@ void BackgroundSyncMetrics::MaybeRecordOneShotSyncCompletionEvent(
     blink::ServiceWorkerStatusCode status_code,
     int num_attempts,
     int max_attempts) {
-  ukm_background_service_->GetBackgroundSourceIdIfAllowed(
+  DCHECK(delegate_);
+  delegate_->GetUkmSourceId(
       origin, base::BindOnce(
                   &BackgroundSyncMetrics::DidGetBackgroundSourceId,
                   weak_ptr_factory_.GetWeakPtr(),
@@ -67,7 +70,8 @@ void BackgroundSyncMetrics::MaybeRecordPeriodicSyncEventCompletion(
     blink::ServiceWorkerStatusCode status_code,
     int num_attempts,
     int max_attempts) {
-  ukm_background_service_->GetBackgroundSourceIdIfAllowed(
+  DCHECK(delegate_);
+  delegate_->GetUkmSourceId(
       origin, base::BindOnce(
                   &BackgroundSyncMetrics::DidGetBackgroundSourceId,
                   weak_ptr_factory_.GetWeakPtr(),

chrome/browser/background_sync/background_sync_metrics.h

@@ -8,7 +8,6 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
 #include "services/metrics/public/cpp/ukm_source_id.h"
 #include "third_party/blink/public/common/service_worker/service_worker_status_code.h"
 
@@ -17,9 +16,9 @@ namespace {
 constexpr double kUkmEventDataBucketSpacing = 2.0;
 }  // namespace
 
-namespace ukm {
-class UkmBackgroundRecorderService;
-}  // namespace ukm
+namespace background_sync {
+class BackgroundSyncDelegate;
+}  // namespace background_sync
 
 namespace url {
 class Origin;
@@ -31,7 +30,7 @@ class BackgroundSyncMetrics {
   using RecordCallback = base::OnceCallback<void(ukm::SourceId)>;
 
   explicit BackgroundSyncMetrics(
-      ukm::UkmBackgroundRecorderService* ukm_background_service);
+      background_sync::BackgroundSyncDelegate* delegate);
   ~BackgroundSyncMetrics();
 
   void MaybeRecordOneShotSyncRegistrationEvent(const url::Origin& origin,
@@ -78,7 +77,7 @@ class BackgroundSyncMetrics {
       int max_attempts,
       ukm::SourceId source_id);
 
-  ukm::UkmBackgroundRecorderService* ukm_background_service_;
+  background_sync::BackgroundSyncDelegate* delegate_;
 
   // Used to signal tests that a UKM event has been recorded.
   base::OnceClosure ukm_event_recorded_for_testing_;

chrome/browser/background_sync/background_sync_metrics_browsertest.cc

@@ -6,7 +6,7 @@
 
 #include <memory>
 
-#include "chrome/browser/metrics/ukm_background_recorder_service.h"
+#include "chrome/browser/background_sync/background_sync_delegate_impl.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -25,13 +25,12 @@ class BackgroundSyncMetricsBrowserTest : public InProcessBrowserTest {
   void SetUpOnMainThread() override {
     Profile* profile = browser()->profile();
 
-    recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
-    auto* ukm_background_service =
-        ukm::UkmBackgroundRecorderFactory::GetForProfile(profile);
-    DCHECK(ukm_background_service);
+    background_sync_delegate_ =
+        std::make_unique<BackgroundSyncDelegateImpl>(profile);
+    background_sync_metrics_ = std::make_unique<BackgroundSyncMetrics>(
+        background_sync_delegate_.get());
 
-    background_sync_metrics_ =
-        std::make_unique<BackgroundSyncMetrics>(ukm_background_service);
+    recorder_ = std::make_unique<ukm::TestAutoSetUkmRecorder>();
 
     // Adds the URL to the history so that UKM events for this origin are
     // recorded.
@@ -50,6 +49,7 @@ class BackgroundSyncMetricsBrowserTest : public InProcessBrowserTest {
 
   std::unique_ptr<ukm::TestAutoSetUkmRecorder> recorder_;
   std::unique_ptr<BackgroundSyncMetrics> background_sync_metrics_;
+  std::unique_ptr<BackgroundSyncDelegateImpl> background_sync_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundSyncMetricsBrowserTest);
 };

components/background_sync/BUILD.gn

@@ -4,6 +4,7 @@
 
 static_library("background_sync") {
   sources = [
+    "background_sync_delegate.h",
     "background_sync_permission_context.cc",
     "background_sync_permission_context.h",
   ]

components/background_sync/DEPS

@@ -3,5 +3,6 @@ include_rules = [
   "+components/permissions",
   "+content/public/browser",
   "+content/public/test",
+  "+services/metrics/public",
   "+third_party/blink/public/mojom",
 ]

components/background_sync/background_sync_delegate.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef COMPONENTS_BACKGROUND_SYNC_BACKGROUND_SYNC_DELEGATE_H_
+#define COMPONENTS_BACKGROUND_SYNC_BACKGROUND_SYNC_DELEGATE_H_
+
+#include "base/callback.h"
+#include "base/optional.h"
+#include "services/metrics/public/cpp/ukm_source_id.h"
+
+namespace url {
+class Origin;
+}  // namespace url
+
+namespace background_sync {
+
+// Allows the component embedder to override the behavior of Background Sync
+// component.
+class BackgroundSyncDelegate {
+ public:
+  virtual ~BackgroundSyncDelegate() = default;
+
+  // Gets the source_ID to log the UKM event for, and calls |callback| with that
+  // source_id, or with base::nullopt if UKM recording is not allowed.
+  virtual void GetUkmSourceId(
+      const url::Origin& origin,
+      base::OnceCallback<void(base::Optional<ukm::SourceId>)> callback) = 0;
+};
+
+}  // namespace background_sync
+
+#endif  // COMPONENTS_BACKGROUND_SYNC_BACKGROUND_SYNC_DELEGATE_H_