[Background Sync] Revive suspended registrations.

Revive suspended periodic Background Sync registrations.

Bug: 925297
Change-Id: Ib2d692858c37ce2619efa40cf6b074488227fcad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1710647
Commit-Queue: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#680191}

chrome/browser/background_sync/background_sync_controller_impl.cc

@@ -13,9 +13,13 @@
 #include "chrome/browser/profiles/profile.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/variations/variations_associated_data.h"
+#include "content/public/browser/background_sync_context.h"
 #include "content/public/browser/background_sync_controller.h"
 #include "content/public/browser/background_sync_parameters.h"
 #include "content/public/browser/background_sync_registration.h"
+#include "content/public/browser/browser_context.h"
+#include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/storage_partition.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
@@ -43,7 +47,8 @@ const char BackgroundSyncControllerImpl::kMinPeriodicSyncEventsInterval[] =
     "min_periodic_sync_events_interval_sec";
 
 BackgroundSyncControllerImpl::BackgroundSyncControllerImpl(Profile* profile)
-    : profile_(profile),
+    : SiteEngagementObserver(SiteEngagementService::Get(profile)),
+      profile_(profile),
       site_engagement_service_(SiteEngagementService::Get(profile)),
       background_sync_metrics_(
           ukm::UkmBackgroundRecorderFactory::GetForProfile(profile_)) {
@@ -53,6 +58,36 @@ BackgroundSyncControllerImpl::BackgroundSyncControllerImpl(Profile* profile)
 
 BackgroundSyncControllerImpl::~BackgroundSyncControllerImpl() = default;
 
+void BackgroundSyncControllerImpl::OnEngagementEvent(
+    content::WebContents* web_contents,
+    const GURL& url,
+    double score,
+    SiteEngagementService::EngagementType engagement_type) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (score == 0.0)
+    return;
+
+  auto origin = url::Origin::Create(url);
+  auto iter = suspended_periodic_sync_origins_.find(origin);
+  if (iter == suspended_periodic_sync_origins_.end())
+    return;
+
+  suspended_periodic_sync_origins_.erase(iter);
+
+  auto* storage_partition = content::BrowserContext::GetStoragePartitionForSite(
+      profile_, url, /* can_create= */ false);
+  if (!storage_partition)
+    return;
+
+  auto* background_sync_context = storage_partition->GetBackgroundSyncContext();
+  if (!background_sync_context)
+    return;
+
+  background_sync_context->RevivePeriodicBackgroundSyncRegistrations(
+      std::move(origin));
+}
+
 void BackgroundSyncControllerImpl::GetParameterOverrides(
     content::BackgroundSyncParameters* parameters) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

chrome/browser/background_sync/background_sync_controller_impl.h

@@ -15,6 +15,7 @@
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/background_sync/background_sync_metrics.h"
+#include "chrome/browser/engagement/site_engagement_observer.h"
 #include "components/keep_alive_registry/keep_alive_types.h"
 #include "components/keep_alive_registry/scoped_keep_alive.h"
 #include "components/keyed_service/core/keyed_service.h"
@@ -36,6 +37,7 @@ class SiteEngagementService;
 class GURL;
 
 class BackgroundSyncControllerImpl : public content::BackgroundSyncController,
+                                     public SiteEngagementObserver,
                                      public KeyedService {
  public:
   static const char kFieldTrialName[];
@@ -98,6 +100,13 @@ class BackgroundSyncControllerImpl : public content::BackgroundSyncController,
   void NoteSuspendedPeriodicSyncOrigins(
       std::set<url::Origin> suspended_origins) override;
 
+  // SiteEngagementObserver overrides.
+  void OnEngagementEvent(
+      content::WebContents* web_contents,
+      const GURL& url,
+      double score,
+      SiteEngagementService::EngagementType engagement_type) override;
+
  private:
   // Gets the site engagement penalty for |url|, which is inversely proportional
   // to the engagement level. The lower the engagement levels with the site,

content/browser/background_sync/background_sync_context_impl.cc

@@ -19,6 +19,7 @@
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "third_party/blink/public/mojom/background_sync/background_sync.mojom.h"
+#include "url/origin.h"
 
 namespace content {
 
@@ -154,6 +155,16 @@ void BackgroundSyncContextImpl::GetSoonestWakeupDelta(
                      std::move(callback)));
 }
 
+void BackgroundSyncContextImpl::RevivePeriodicBackgroundSyncRegistrations(
+    url::Origin origin) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  base::PostTaskWithTraits(
+      FROM_HERE, {BrowserThread::IO},
+      base::BindOnce(&BackgroundSyncContextImpl::
+                         RevivePeriodicBackgroundSyncRegistrationsOnIOThread,
+                     this, std::move(origin)));
+}
+
 base::TimeDelta BackgroundSyncContextImpl::GetSoonestWakeupDeltaOnIOThread(
     blink::mojom::BackgroundSyncType sync_type,
     base::Time last_browser_wakeup_for_periodic_sync) {
@@ -176,6 +187,16 @@ void BackgroundSyncContextImpl::DidGetSoonestWakeupDelta(
   std::move(callback).Run(soonest_wakeup_delta);
 }
 
+void BackgroundSyncContextImpl::
+    RevivePeriodicBackgroundSyncRegistrationsOnIOThread(url::Origin origin) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (!background_sync_manager_)
+    return;
+
+  background_sync_manager_->RevivePeriodicSyncRegistrations(std::move(origin));
+}
+
 void BackgroundSyncContextImpl::FireBackgroundSyncEvents(
     blink::mojom::BackgroundSyncType sync_type,
     base::OnceClosure done_closure) {

content/browser/background_sync/background_sync_context_impl.h

@@ -74,6 +74,7 @@ class CONTENT_EXPORT BackgroundSyncContextImpl
       blink::mojom::BackgroundSyncType sync_type,
       base::Time last_browser_wakeup_for_periodic_sync,
       base::OnceCallback<void(base::TimeDelta)> callback) override;
+  void RevivePeriodicBackgroundSyncRegistrations(url::Origin origin) override;
 
  protected:
   friend class base::RefCountedDeleteOnSequence<BackgroundSyncContextImpl>;
@@ -114,6 +115,8 @@ class CONTENT_EXPORT BackgroundSyncContextImpl
       base::OnceCallback<void(base::TimeDelta)> callback,
       base::TimeDelta soonest_wakeup_delta);
 
+  void RevivePeriodicBackgroundSyncRegistrationsOnIOThread(url::Origin origin);
+
   // The services are owned by this. They're either deleted
   // during ShutdownOnIO or when the channel is closed via
   // *ServiceHadConnectionError. Only accessed on the IO thread.

content/browser/background_sync/background_sync_manager.cc

@@ -1511,6 +1511,135 @@ base::Time BackgroundSyncManager::GetSoonestPeriodicSyncEventTimeForOrigin(
   return soonest_wakeup_time;
 }
 
+void BackgroundSyncManager::RevivePeriodicSyncRegistrations(
+    url::Origin origin) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (disabled_)
+    return;
+
+  op_scheduler_.ScheduleOperation(
+      CacheStorageSchedulerOp::kBackgroundSync,
+      base::BindOnce(&BackgroundSyncManager::ReviveOriginImpl,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(origin),
+                     MakeEmptyCompletion()));
+}
+
+void BackgroundSyncManager::ReviveOriginImpl(url::Origin origin,
+                                             base::OnceClosure callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (disabled_) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  std::move(callback));
+    return;
+  }
+
+  // Create a list of registrations to revive.
+  std::vector<const BackgroundSyncRegistration*> to_revive;
+  std::map<const BackgroundSyncRegistration*, int64_t>
+      service_worker_registration_ids;
+
+  for (const auto& active_registration : active_registrations_) {
+    int64_t service_worker_registration_id = active_registration.first;
+    if (active_registration.second.origin != origin)
+      continue;
+
+    for (const auto& key_and_registration :
+         active_registration.second.registration_map) {
+      const BackgroundSyncRegistration* registration =
+          &key_and_registration.second;
+      if (!registration->is_suspended())
+        continue;
+
+      to_revive.push_back(registration);
+      service_worker_registration_ids[registration] =
+          service_worker_registration_id;
+    }
+  }
+
+  if (to_revive.empty()) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE,
+                                                  std::move(callback));
+    return;
+  }
+
+  base::RepeatingClosure received_new_delays_closure = base::BarrierClosure(
+      to_revive.size(),
+      base::BindOnce(
+          &BackgroundSyncManager::DidReceiveDelaysForSuspendedRegistrations,
+          weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+
+  for (const auto* registration : to_revive) {
+    base::PostTaskWithTraitsAndReplyWithResult(
+        FROM_HERE, {BrowserThread::UI},
+        base::BindOnce(
+            &GetNextEventDelay, service_worker_context_, *registration,
+            std::make_unique<BackgroundSyncParameters>(*parameters_)),
+        base::BindOnce(&BackgroundSyncManager::ReviveDidGetNextEventDelay,
+                       weak_ptr_factory_.GetWeakPtr(),
+                       service_worker_registration_ids[registration],
+                       *registration, received_new_delays_closure));
+  }
+}
+
+void BackgroundSyncManager::ReviveDidGetNextEventDelay(
+    int64_t service_worker_registration_id,
+    BackgroundSyncRegistration registration,
+    base::OnceClosure done_closure,
+    base::TimeDelta delay) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (delay.is_max()) {
+    std::move(done_closure).Run();
+    return;
+  }
+
+  BackgroundSyncRegistration* active_registration =
+      LookupActiveRegistration(blink::mojom::BackgroundSyncRegistrationInfo(
+          service_worker_registration_id, registration.options()->tag,
+          registration.sync_type()));
+  if (!active_registration) {
+    std::move(done_closure).Run();
+    return;
+  }
+
+  active_registration->set_delay_until(clock_->Now() + delay);
+
+  StoreRegistrations(
+      service_worker_registration_id,
+      base::BindOnce(&BackgroundSyncManager::ReviveDidStoreRegistration,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     service_worker_registration_id, std::move(done_closure)));
+}
+
+void BackgroundSyncManager::ReviveDidStoreRegistration(
+    int64_t service_worker_registration_id,
+    base::OnceClosure done_closure,
+    blink::ServiceWorkerStatusCode status) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+
+  if (status == blink::ServiceWorkerStatusCode::kErrorNotFound) {
+    // The service worker registration is gone.
+    active_registrations_.erase(service_worker_registration_id);
+    std::move(done_closure).Run();
+  }
+
+  if (status != blink::ServiceWorkerStatusCode::kOk) {
+    DisableAndClearManager(std::move(done_closure));
+    return;
+  }
+
+  std::move(done_closure).Run();
+}
+
+void BackgroundSyncManager::DidReceiveDelaysForSuspendedRegistrations(
+    base::OnceClosure callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  ScheduleDelayedProcessingOfRegistrations(BackgroundSyncType::PERIODIC);
+  base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, std::move(callback));
+}
+
 void BackgroundSyncManager::ScheduleDelayedProcessingOfRegistrations(
     blink::mojom::BackgroundSyncType sync_type) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -1549,8 +1678,6 @@ void BackgroundSyncManager::FireReadyEventsImpl(
   }
 
   // Find the registrations that are ready to run.
-  // TODO(crbug.com/925297): Periodically re-evaluate suspended Periodic Sync
-  // registrations.
   std::vector<blink::mojom::BackgroundSyncRegistrationInfoPtr> to_fire;
 
   for (auto& sw_reg_id_and_registrations : active_registrations_) {

content/browser/background_sync/background_sync_manager.h

@@ -177,6 +177,9 @@ class CONTENT_EXPORT BackgroundSyncManager
   base::Time GetSoonestPeriodicSyncEventTimeForOrigin(
       const url::Origin& origin) const;
 
+  // Revive any pending periodic Background Sync registrations for |origin|.
+  void RevivePeriodicSyncRegistrations(url::Origin origin);
+
  protected:
   BackgroundSyncManager(
       scoped_refptr<ServiceWorkerContextWrapper> context,
@@ -398,6 +401,16 @@ class CONTENT_EXPORT BackgroundSyncManager
   // Whether an event should be logged for debuggability, for |sync_type|.
   bool ShouldLogToDevTools(blink::mojom::BackgroundSyncType sync_type);
 
+  void ReviveOriginImpl(url::Origin origin, base::OnceClosure callback);
+  void ReviveDidGetNextEventDelay(int64_t service_worker_registration_id,
+                                  BackgroundSyncRegistration registration,
+                                  base::OnceClosure done_closure,
+                                  base::TimeDelta delay);
+  void ReviveDidStoreRegistration(int64_t service_worker_registration_id,
+                                  base::OnceClosure done_closure,
+                                  blink::ServiceWorkerStatusCode status);
+  void DidReceiveDelaysForSuspendedRegistrations(base::OnceClosure callback);
+
   base::OnceClosure MakeEmptyCompletion();
 
   blink::ServiceWorkerStatusCode CanEmulateSyncEvent(

content/browser/background_sync/background_sync_manager_unittest.cc

@@ -257,6 +257,15 @@ class BackgroundSyncManagerTest
         sync_type, last_browser_wakeup_for_periodic_sync);
   }
 
+  void SuspendPeriodicSyncRegistrations(std::set<url::Origin> origins) {
+    GetController()->NoteSuspendedPeriodicSyncOrigins(std::move(origins));
+  }
+
+  void RevivePeriodicSyncRegistrations(url::Origin origin) {
+    GetController()->ReviveSuspendedPeriodicSyncOrigin(origin);
+    test_background_sync_manager()->RevivePeriodicSyncRegistrations(origin);
+  }
+
  protected:
   MOCK_METHOD1(OnEventReceived,
                void(const devtools::proto::BackgroundServiceEvent&));
@@ -1228,6 +1237,43 @@ TEST_F(BackgroundSyncManagerTest, PeriodicSyncFiresWhenExpected) {
   EXPECT_TRUE(GetRegistration(sync_options_2_));
 }
 
+TEST_F(BackgroundSyncManagerTest, TestSupensionAndRevival) {
+  InitPeriodicSyncEventTest();
+  auto thirteen_hours = base::TimeDelta::FromHours(13);
+  sync_options_2_.min_interval = thirteen_hours.InMilliseconds();
+  sync_options_1_.min_interval = thirteen_hours.InMilliseconds();
+
+  auto origin = url::Origin::Create(GURL(kScope1).GetOrigin());
+
+  SuspendPeriodicSyncRegistrations({origin});
+  EXPECT_TRUE(Register(sync_options_1_));
+  EXPECT_TRUE(GetRegistration(sync_options_1_));
+  EXPECT_TRUE(Register(sync_options_2_));
+  EXPECT_TRUE(GetRegistration(sync_options_2_));
+  EXPECT_EQ(0, periodic_sync_events_called_);
+
+  // Advance clock.
+  test_clock_.Advance(thirteen_hours);
+  FireReadyEvents();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(0, periodic_sync_events_called_);
+  EXPECT_TRUE(GetRegistration(sync_options_1_));
+  EXPECT_TRUE(GetRegistration(sync_options_2_));
+
+  RevivePeriodicSyncRegistrations(std::move(origin));
+  base::RunLoop().RunUntilIdle();
+
+  // Advance clock.
+  test_clock_.Advance(thirteen_hours);
+  FireReadyEvents();
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(2, periodic_sync_events_called_);
+  EXPECT_TRUE(GetRegistration(sync_options_1_));
+  EXPECT_TRUE(GetRegistration(sync_options_2_));
+  Unregister(sync_options_1_);
+  Unregister(sync_options_2_);
+}
+
 TEST_F(BackgroundSyncManagerTest, ReregisterMidSyncFirstAttemptFails) {
   InitDelayedSyncEventTest();
   RegisterAndVerifySyncEventDelayed(sync_options_1_);

content/public/browser/background_sync_context.h

@@ -9,14 +9,14 @@
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
+#include "third_party/blink/public/mojom/background_sync/background_sync.mojom.h"
+#include "url/origin.h"
 
 #if defined(OS_ANDROID)
 #include "base/android/jni_android.h"
 #include "base/android/scoped_java_ref.h"
 #endif
 
-#include "third_party/blink/public/mojom/background_sync/background_sync.mojom.h"
-
 namespace content {
 
 class BrowserContext;
@@ -63,6 +63,10 @@ class CONTENT_EXPORT BackgroundSyncContext {
       base::Time last_browser_wakeup_for_periodic_sync,
       base::OnceCallback<void(base::TimeDelta)> callback) = 0;
 
+  // Revives any suspended periodic Background Sync registrations for |origin|.
+  virtual void RevivePeriodicBackgroundSyncRegistrations(
+      url::Origin origin) = 0;
+
  protected:
   virtual ~BackgroundSyncContext() = default;
 

content/test/mock_background_sync_controller.cc

@@ -36,6 +36,9 @@ base::TimeDelta MockBackgroundSyncController::GetNextEventDelay(
     BackgroundSyncParameters* parameters) {
   DCHECK(parameters);
 
+  if (suspended_periodic_sync_origins_.count(registration.origin()))
+    return base::TimeDelta::Max();
+
   int num_attempts = registration.num_attempts();
 
   if (!num_attempts) {

content/test/mock_background_sync_controller.h

@@ -51,6 +51,9 @@ class MockBackgroundSyncController : public BackgroundSyncController {
   BackgroundSyncParameters* background_sync_parameters() {
     return &background_sync_parameters_;
   }
+  void ReviveSuspendedPeriodicSyncOrigin(url::Origin origin_to_revive) {
+    suspended_periodic_sync_origins_.erase(origin_to_revive);
+  }
 
  private:
   int registration_count_ = 0;