Notification scheduler: Rewrite BackgroundTaskCoordinator.

This CL rewrites the logic to schedule background task in
BackgroundTaskCoordinator. Now it's based on deliver time window.

The old unit tests are deleted, new unit tests will be added in
following CL.

Bug: 991010
Change-Id: I8a05e8178d94b1daadf87d8abc8f9e0bac783e37
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1756439
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689105}

chrome/browser/notifications/scheduler/internal/background_task_coordinator.cc

@@ -10,9 +10,9 @@
 #include "base/command_line.h"
 #include "base/numerics/ranges.h"
 #include "base/optional.h"
-#include "base/rand_util.h"
 #include "base/time/clock.h"
 #include "chrome/browser/notifications/scheduler/internal/impression_types.h"
+#include "chrome/browser/notifications/scheduler/internal/notification_entry.h"
 #include "chrome/browser/notifications/scheduler/internal/scheduler_config.h"
 #include "chrome/browser/notifications/scheduler/internal/scheduler_utils.h"
 #include "chrome/browser/notifications/scheduler/public/features.h"
@@ -26,32 +26,30 @@ class BackgroundTaskCoordinatorHelper {
   BackgroundTaskCoordinatorHelper(
       NotificationBackgroundTaskScheduler* background_task,
       const SchedulerConfig* config,
-      BackgroundTaskCoordinator::TimeRandomizer time_randomizer,
       base::Clock* clock)
       : background_task_(background_task),
         config_(config),
-        time_randomizer_(time_randomizer),
         clock_(clock) {}
   ~BackgroundTaskCoordinatorHelper() = default;
 
   void ScheduleBackgroundTask(
       BackgroundTaskCoordinator::Notifications notifications,
-      BackgroundTaskCoordinator::ClientStates client_states,
-      SchedulerTaskTime task_start_time) {
+      BackgroundTaskCoordinator::ClientStates client_states) {
     if (notifications.empty()) {
       background_task_->Cancel();
       return;
     }
 
+    base::Time tomorrow;
+    base::Time now = clock_->Now();
+    bool success = ToLocalHour(0, now, 1 /*day_delta*/, &tomorrow);
+    DCHECK(success);
+
     std::map<SchedulerClientType, int> shown_per_type;
     int shown_total = 0;
     SchedulerClientType last_shown_type = SchedulerClientType::kUnknown;
     NotificationsShownToday(client_states, &shown_per_type, &shown_total,
                             &last_shown_type, clock_);
-    bool reach_max_today_all_type =
-        shown_total >= config_->max_daily_shown_all_type;
-    base::Time next_morning = NextMorning();
-    base::Time this_evening = ThisEvening();
 
     for (const auto& pair : notifications) {
       auto type = pair.first;
@@ -60,88 +58,58 @@ class BackgroundTaskCoordinatorHelper {
         continue;
 
       const ClientState* client_state = it->second;
-
-      // Try to schedule on the day that suppression expires.
-      if (client_state->suppression_info.has_value()) {
-        const auto& suppression = client_state->suppression_info.value();
-        base::Time suppression_expire;
-        ToLocalHour(config_->morning_task_hour, suppression.ReleaseTime(),
-                    0 /*day_delta*/, &suppression_expire);
-        MaybeUpdateBackgroundTaskTime(
-            std::max(suppression_expire, next_morning));
-        continue;
-      }
-
-      // Has met the quota for this notification type or for all types, only can
-      // send more on next day.
       bool reach_max_today =
-          shown_per_type[type] >= client_state->current_max_daily_show;
-      if (reach_max_today || reach_max_today_all_type) {
-        MaybeUpdateBackgroundTaskTime(next_morning);
-        continue;
-      }
-
-      switch (task_start_time) {
-        case SchedulerTaskTime::kMorning:
-          // Still can send more in the evening.
-          MaybeUpdateBackgroundTaskTime(this_evening);
-          break;
-        case SchedulerTaskTime::kEvening:
-          // Wait until the next calendar day.
-          MaybeUpdateBackgroundTaskTime(next_morning);
-          break;
-        case SchedulerTaskTime::kUnknown:
-          auto now = clock_->Now();
-          auto this_morning = ThisMorning();
-          if (now <= this_morning)
-            MaybeUpdateBackgroundTaskTime(this_morning);
-          else if (now <= this_evening)
-            MaybeUpdateBackgroundTaskTime(this_evening);
-          else
-            MaybeUpdateBackgroundTaskTime(next_morning);
-          // TODO(xingliu): Support arbitrary time background task.
-          break;
+          shown_per_type[type] >= client_state->current_max_daily_show ||
+          shown_total >= config_->max_daily_shown_all_type;
+
+      // Find the eariliest notification to launch the background task.
+      for (const auto* entry : pair.second) {
+        // Currently only support deliver time window.
+        if (!entry->schedule_params.deliver_time_start.has_value()) {
+          continue;
+        }
+
+        base::Time deliver_time_start =
+            entry->schedule_params.deliver_time_start.value();
+
+        // Each background task has a minimum interval.
+        if (deliver_time_start < now + config_->background_task_min_interval)
+          deliver_time_start = now + config_->background_task_min_interval;
+
+        // Consider suppression time.
+        if (client_state->suppression_info.has_value() &&
+            deliver_time_start <
+                client_state->suppression_info->ReleaseTime()) {
+          deliver_time_start = client_state->suppression_info->ReleaseTime();
+        }
+
+        // Consider daily limit throttling.
+        if (reach_max_today && deliver_time_start < tomorrow)
+          deliver_time_start = tomorrow;
+
+        // Deliver time window has passed.
+        DCHECK(entry->schedule_params.deliver_time_end.has_value());
+        if (!entry->schedule_params.deliver_time_end.has_value() ||
+            deliver_time_start >
+                entry->schedule_params.deliver_time_end.value()) {
+          continue;
+        }
+
+        MaybeUpdateBackgroundTaskTime(deliver_time_start);
       }
     }
 
-    ScheduleBackgroundTaskInternal(task_start_time);
+    ScheduleBackgroundTaskInternal();
   }
 
  private:
-  // Returns the morning background task time on the next day.
-  base::Time NextMorning() {
-    base::Time next_morning;
-    bool success = ToLocalHour(config_->morning_task_hour, clock_->Now(),
-                               1 /*day_delta*/, &next_morning);
-    DCHECK(success);
-    return next_morning;
-  }
-
-  // Returns the morning background task time today.
-  base::Time ThisMorning() {
-    base::Time this_morning;
-    bool success = ToLocalHour(config_->morning_task_hour, clock_->Now(),
-                               0 /*day_delta*/, &this_morning);
-    DCHECK(success);
-    return this_morning;
-  }
-
-  // Returns the evening background task time on today.
-  base::Time ThisEvening() {
-    base::Time this_evening;
-    bool success = ToLocalHour(config_->evening_task_hour, clock_->Now(),
-                               0 /*day_delta*/, &this_evening);
-    DCHECK(success);
-    return this_evening;
-  }
-
   void MaybeUpdateBackgroundTaskTime(const base::Time& time) {
     if (!background_task_time_.has_value() ||
         time < background_task_time_.value())
       background_task_time_ = time;
   }
 
-  void ScheduleBackgroundTaskInternal(SchedulerTaskTime task_start_time) {
+  void ScheduleBackgroundTaskInternal() {
     if (!background_task_time_.has_value())
       return;
 
@@ -150,30 +118,22 @@ class BackgroundTaskCoordinatorHelper {
     window_start_time = base::ClampToRange(window_start_time, base::TimeDelta(),
                                            base::TimeDelta::Max());
 
+    // TODO(xingliu): Remove SchedulerTaskTime.
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
             switches::kNotificationSchedulerImmediateBackgroundTask)) {
       background_task_->Schedule(
-          task_start_time, base::TimeDelta(),
+          SchedulerTaskTime::kMorning, base::TimeDelta(),
           base::TimeDelta() + base::TimeDelta::FromMinutes(1));
       return;
     }
 
-    // Adds a randomized time delta to distribute the click loads.
-    // TODO(xingliu): Maybe show notifications one by one and spread into a time
-    // window.  See https://crbug.com/986614
-    base::TimeDelta random_interval;
-    if (task_start_time != SchedulerTaskTime::kUnknown)
-      random_interval = time_randomizer_.Run();
-
     background_task_->Schedule(
-        task_start_time, window_start_time + random_interval,
-        window_start_time + config_->background_task_window_duration +
-            random_interval);
+        SchedulerTaskTime::kMorning, window_start_time,
+        window_start_time + config_->background_task_window_duration);
   }
 
   NotificationBackgroundTaskScheduler* background_task_;
   const SchedulerConfig* config_;
-  BackgroundTaskCoordinator::TimeRandomizer time_randomizer_;
   base::Clock* clock_;
   base::Optional<base::Time> background_task_time_;
 
@@ -182,22 +142,14 @@ class BackgroundTaskCoordinatorHelper {
 
 }  // namespace
 
-// static
-base::TimeDelta BackgroundTaskCoordinator::DefaultTimeRandomizer(
-    const base::TimeDelta& time_window) {
-  return base::RandDouble() * time_window;
-}
-
 class BackgroundTaskCoordinatorImpl : public BackgroundTaskCoordinator {
  public:
   BackgroundTaskCoordinatorImpl(
       std::unique_ptr<NotificationBackgroundTaskScheduler> background_task,
       const SchedulerConfig* config,
-      TimeRandomizer time_randomizer,
       base::Clock* clock)
       : background_task_(std::move(background_task)),
         config_(config),
-        time_randomizer_(time_randomizer),
         clock_(clock) {}
 
   ~BackgroundTaskCoordinatorImpl() override = default;
@@ -205,12 +157,11 @@ class BackgroundTaskCoordinatorImpl : public BackgroundTaskCoordinator {
  private:
   // BackgroundTaskCoordinator implementation.
   void ScheduleBackgroundTask(Notifications notifications,
-                              ClientStates client_states,
-                              SchedulerTaskTime task_start_time) override {
+                              ClientStates client_states) override {
     auto helper = std::make_unique<BackgroundTaskCoordinatorHelper>(
-        background_task_.get(), config_, time_randomizer_, clock_);
+        background_task_.get(), config_, clock_);
     helper->ScheduleBackgroundTask(std::move(notifications),
-                                   std::move(client_states), task_start_time);
+                                   std::move(client_states));
   }
 
   // The class that actually schedules platform background task.
@@ -219,10 +170,6 @@ class BackgroundTaskCoordinatorImpl : public BackgroundTaskCoordinator {
   // System configuration.
   const SchedulerConfig* config_;
 
-  // Randomize the time to show the notification, to avoid large number of users
-  // to perform actions at the same time.
-  TimeRandomizer time_randomizer_;
-
   // Clock to query the current timestamp.
   base::Clock* clock_;
 
@@ -233,10 +180,9 @@ class BackgroundTaskCoordinatorImpl : public BackgroundTaskCoordinator {
 std::unique_ptr<BackgroundTaskCoordinator> BackgroundTaskCoordinator::Create(
     std::unique_ptr<NotificationBackgroundTaskScheduler> background_task,
     const SchedulerConfig* config,
-    TimeRandomizer time_randomizer,
     base::Clock* clock) {
   return std::make_unique<BackgroundTaskCoordinatorImpl>(
-      std::move(background_task), config, time_randomizer, clock);
+      std::move(background_task), config, clock);
 }
 
 BackgroundTaskCoordinator::~BackgroundTaskCoordinator() = default;

chrome/browser/notifications/scheduler/internal/background_task_coordinator.h

@@ -39,15 +39,13 @@ class BackgroundTaskCoordinator {
   static std::unique_ptr<BackgroundTaskCoordinator> Create(
       std::unique_ptr<NotificationBackgroundTaskScheduler> background_task,
       const SchedulerConfig* config,
-      TimeRandomizer time_randomizer,
       base::Clock* clock);
 
   virtual ~BackgroundTaskCoordinator();
 
   // Schedule background task based on current notification in the storage.
   virtual void ScheduleBackgroundTask(Notifications notifications,
-                                      ClientStates client_states,
-                                      SchedulerTaskTime task_start_time) = 0;
+                                      ClientStates client_states) = 0;
 };
 
 }  // namespace notifications

chrome/browser/notifications/scheduler/internal/background_task_coordinator_unittest.cc

@@ -44,19 +44,12 @@ const std::vector<test::ImpressionTestData> kClientsImpressionTestData = {
      {},
      base::nullopt /* suppression_info */}};
 
-base::TimeDelta NoopTimeRandomizer(const base::TimeDelta& time_window) {
-  return base::TimeDelta();
-}
-
 struct TestData {
   // Impression data as the input.
   std::vector<test::ImpressionTestData> impression_test_data;
 
   // Notification entries as the input.
   std::vector<NotificationEntry> notification_entries;
-
-  // The type of current background task.
-  SchedulerTaskTime task_start_time = SchedulerTaskTime::kMorning;
 };
 
 class BackgroundTaskCoordinatorTest : public testing::Test {
@@ -67,8 +60,6 @@ class BackgroundTaskCoordinatorTest : public testing::Test {
  protected:
   void SetUp() override {
     // Setup configuration used by this test.
-    config_.morning_task_hour = 6;
-    config_.evening_task_hour = 18;
     config_.max_daily_shown_all_type = 3;
     config_.max_daily_shown_per_type = 2;
     config_.suppression_duration = base::TimeDelta::FromDays(3);
@@ -76,9 +67,8 @@ class BackgroundTaskCoordinatorTest : public testing::Test {
     auto background_task =
         std::make_unique<test::MockNotificationBackgroundTaskScheduler>();
     background_task_ = background_task.get();
-    coordinator_ = BackgroundTaskCoordinator::Create(
-        std::move(background_task), &config_,
-        base::BindRepeating(&NoopTimeRandomizer, base::TimeDelta()), &clock_);
+    coordinator_ = BackgroundTaskCoordinator::Create(std::move(background_task),
+                                                     &config_, &clock_);
   }
 
   test::MockNotificationBackgroundTaskScheduler* background_task() {
@@ -107,8 +97,7 @@ class BackgroundTaskCoordinatorTest : public testing::Test {
       notifications[entry.type].emplace_back(&entry);
     }
     coordinator_->ScheduleBackgroundTask(std::move(notifications),
-                                         std::move(client_states),
-                                         test_data_.task_start_time);
+                                         std::move(client_states));
   }
 
   void TestScheduleNewNotification(const char* now,
@@ -123,8 +112,7 @@ class BackgroundTaskCoordinatorTest : public testing::Test {
                              config()->background_task_window_duration));
 
     NotificationEntry entry(SchedulerClientType::kTest1, kGuid);
-    TestData test_data{
-        kSingleClientImpressionTestData, {entry}, SchedulerTaskTime::kUnknown};
+    TestData test_data{kSingleClientImpressionTestData, {entry}};
     ScheduleTask(test_data);
   }
 
@@ -150,192 +138,5 @@ TEST_F(BackgroundTaskCoordinatorTest, NoNotification) {
   ScheduleTask(test_data);
 }
 
-// In a morning task, find one notification and schedule an evening task.
-TEST_F(BackgroundTaskCoordinatorTest, InMorningScheduleEvening) {
-  const char kNow[] = "04/25/20 01:00:00 AM";
-  SetNow(kNow);
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // Expected to run task this evening.
-  auto expected_window_start = GetTime("04/25/20 18:00:00 PM") - GetTime(kNow);
-  EXPECT_CALL(*background_task(),
-              Schedule(_, expected_window_start,
-                       expected_window_start +
-                           config()->background_task_window_duration));
-
-  NotificationEntry entry(SchedulerClientType::kTest1, kGuid);
-  TestData test_data{
-      kSingleClientImpressionTestData, {entry}, SchedulerTaskTime::kMorning};
-  ScheduleTask(test_data);
-}
-
-// In morning task, schedule evening task but throttled, schedule to next
-// morning.
-TEST_F(BackgroundTaskCoordinatorTest, InMorningScheduleEveningThrottled) {
-  const char kNow[] = "04/25/20 02:00:00 PM";
-  SetNow(kNow);
-
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // Expected to run task next morning.
-  EXPECT_CALL(*background_task(),
-              Schedule(_, GetTime("04/26/20 06:00:00 AM") - GetTime(kNow), _));
-
-  auto impression_data = kSingleClientImpressionTestData;
-  Impression impression;
-  impression.create_time = GetTime("04/25/20 01:00:00 AM");
-  impression_data.back().impressions.emplace_back(impression);
-
-  NotificationEntry entry(SchedulerClientType::kTest1, kGuid);
-  TestData test_data{impression_data, {entry}, SchedulerTaskTime::kMorning};
-  ScheduleTask(test_data);
-}
-
-// In an evening task, schedule background task to run next morning.
-TEST_F(BackgroundTaskCoordinatorTest, InEveningScheduleNextMorning) {
-  const char kNow[] = "04/25/20 18:00:00 PM";
-  SetNow(kNow);
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // Expected to run task next morning.
-  auto expected_window_start = GetTime("04/26/20 06:00:00 AM") - GetTime(kNow);
-  EXPECT_CALL(*background_task(), Schedule(_, expected_window_start, _));
-
-  NotificationEntry entry(SchedulerClientType::kTest1, kGuid);
-  TestData test_data{
-      kSingleClientImpressionTestData, {entry}, SchedulerTaskTime::kEvening};
-  ScheduleTask(test_data);
-}
-
-// In an evening task, schedule background task to run next morning, even if we
-// reached the daily max.
-TEST_F(BackgroundTaskCoordinatorTest, InEveningScheduleNextMorningThrottled) {
-  const char kNow[] = "04/25/20 18:00:00 PM";
-  SetNow(kNow);
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // Expected to run task next morning.
-  auto expected_window_start = GetTime("04/26/20 06:00:00 AM") - GetTime(kNow);
-  EXPECT_CALL(*background_task(), Schedule(_, expected_window_start, _));
-
-  // We have reached daily max.
-  auto impression_data = kSingleClientImpressionTestData;
-  Impression impression;
-  impression.create_time = GetTime("04/25/20 01:00:00 AM");
-  impression_data.back().impressions.emplace_back(impression);
-
-  NotificationEntry entry(SchedulerClientType::kTest1, kGuid);
-  TestData test_data{
-      kSingleClientImpressionTestData, {entry}, SchedulerTaskTime::kEvening};
-  ScheduleTask(test_data);
-}
-
-// Suppression will result in background task scheduled after suppression
-// expired.
-TEST_F(BackgroundTaskCoordinatorTest, Suppression) {
-  const char kNow[] = "04/25/20 06:00:00 AM";
-  SetNow(kNow);
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // Expected to run task in the morning after suppression expired.
-  auto expected_window_start = GetTime("04/28/20 06:00:00 AM") - GetTime(kNow);
-  EXPECT_CALL(*background_task(), Schedule(_, expected_window_start, _));
-
-  auto impression_data = kSingleClientImpressionTestData;
-  impression_data.back().suppression_info = SuppressionInfo(
-      GetTime("04/25/20 00:00:00 AM"), base::TimeDelta::FromDays(3));
-
-  NotificationEntry entry(SchedulerClientType::kTest1, kGuid);
-  TestData test_data{impression_data, {entry}, SchedulerTaskTime::kMorning};
-  ScheduleTask(test_data);
-}
-
-// If two different types want to schedule at different times, pick the earilier
-// one.
-TEST_F(BackgroundTaskCoordinatorTest, ScheduleEarlierTime) {
-  const char kNow[] = "04/25/20 01:00:00 AM";
-  SetNow(kNow);
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // kTest1 type will run this evening, kTest2 will run task 3 days later.
-  // Expected to run the earilier task.
-  auto expected_window_start = GetTime("04/25/20 18:00:00 PM") - GetTime(kNow);
-  EXPECT_CALL(*background_task(), Schedule(_, expected_window_start, _));
-
-  NotificationEntry entry1(SchedulerClientType::kTest1, kGuid);
-  NotificationEntry entry2(SchedulerClientType::kTest2, "guid_entry2");
-  auto impression_data = kClientsImpressionTestData;
-  impression_data[0].suppression_info = SuppressionInfo(
-      GetTime("04/25/20 00:00:00 AM"), base::TimeDelta::FromDays(3));
-  TestData test_data{
-      impression_data, {entry1, entry2}, SchedulerTaskTime::kMorning};
-  ScheduleTask(test_data);
-}
-
-// If reached |max_daily_shown_all_type|, background task should run tomorrow.
-TEST_F(BackgroundTaskCoordinatorTest, InMorningThrottledAllTypes) {
-  const char kNow[] = "04/25/20 05:00:00 AM";
-  SetNow(kNow);
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // Expected to run task next morning.
-  auto expected_window_start = GetTime("04/26/20 06:00:00 AM") - GetTime(kNow);
-  EXPECT_CALL(*background_task(), Schedule(_, expected_window_start, _));
-
-  auto impression_data = kClientsImpressionTestData;
-  Impression impression;
-  impression.create_time = GetTime("04/25/20 01:00:00 AM");
-
-  // Make sure we reach daily max for all types.
-  for (int i = 0; i < config()->max_daily_shown_all_type; i++)
-    impression_data.back().impressions.emplace_back(impression);
-
-  NotificationEntry entry(SchedulerClientType::kTest1, kGuid);
-  TestData test_data{impression_data, {entry}, SchedulerTaskTime::kMorning};
-  ScheduleTask(test_data);
-}
-
-// If reached |max_daily_shown_all_type| and all types have suppression,
-// background task should run after one suppression expired.
-TEST_F(BackgroundTaskCoordinatorTest, ThrottledAllTypesAndSuppression) {
-  const char kNow[] = "04/25/20 05:00:00 AM";
-  SetNow(kNow);
-  EXPECT_CALL(*background_task(), Cancel()).Times(0);
-  // Expected to run after 3 days suppression ends.
-  auto expected_window_start = GetTime("04/28/20 06:00:00 AM") - GetTime(kNow);
-  EXPECT_CALL(*background_task(), Schedule(_, expected_window_start, _));
-
-  auto impression_data = kClientsImpressionTestData;
-  Impression impression;
-  impression.create_time = GetTime("04/25/20 01:00:00 AM");
-
-  // Make sure we reach daily max for all types.
-  for (int i = 0; i < config()->max_daily_shown_all_type; i++)
-    impression_data[1].impressions.emplace_back(impression);
-
-  // Suppression for both types.
-  impression_data[0].suppression_info = SuppressionInfo(
-      GetTime("04/25/20 00:00:00 AM"), base::TimeDelta::FromDays(3));
-  impression_data[1].suppression_info = SuppressionInfo(
-      GetTime("04/25/20 00:00:00 AM"), base::TimeDelta::FromDays(4));
-
-  NotificationEntry entry1(SchedulerClientType::kTest1, "test_guid_1");
-  NotificationEntry entry2(SchedulerClientType::kTest2, "test_guid_2");
-  TestData test_data{
-      impression_data, {entry1, entry2}, SchedulerTaskTime::kMorning};
-  ScheduleTask(test_data);
-}
-
-// Schedules a new notification when Chrome is not running in a background task
-// at different time of a day.
-TEST_F(BackgroundTaskCoordinatorTest, ScheduleNewNotification) {
-  TestScheduleNewNotification("04/25/20 01:00:00 AM", "04/25/20 06:00:00 AM");
-  TestScheduleNewNotification("04/25/20 07:00:00 AM", "04/25/20 18:00:00 PM");
-  TestScheduleNewNotification("04/25/20 18:30:00 PM", "04/26/20 06:00:00 AM");
-}
-
-// Test to verify the default time randomizer.
-TEST_F(BackgroundTaskCoordinatorTest, DefaultTimeRandomizer) {
-  EXPECT_EQ(BackgroundTaskCoordinator::DefaultTimeRandomizer(base::TimeDelta()),
-            base::TimeDelta());
-  auto time_window = base::TimeDelta::FromHours(1);
-  auto delta = BackgroundTaskCoordinator::DefaultTimeRandomizer(time_window);
-  EXPECT_LT(delta, time_window);
-  EXPECT_GE(delta, base::TimeDelta());
-}
-
 }  // namespace
 }  // namespace notifications

chrome/browser/notifications/scheduler/internal/display_decider_unittest.cc

@@ -6,6 +6,7 @@
 
 #include <map>
 #include <memory>
+#include <string>
 #include <vector>
 
 #include "base/strings/stringprintf.h"
@@ -75,8 +76,6 @@ class DisplayDeciderTest : public testing::Test {
 
   void SetUp() override {
     // Setup configuration used by this test.
-    config_.morning_task_hour = 7;
-    config_.evening_task_hour = 18;
     config_.max_daily_shown_all_type = 3;
   }
 

chrome/browser/notifications/scheduler/internal/notification_scheduler.cc

@@ -267,7 +267,7 @@ class NotificationSchedulerImpl : public NotificationScheduler,
     context_->impression_tracker()->GetClientStates(&client_states);
 
     context_->background_task_coordinator()->ScheduleBackgroundTask(
-        std::move(notifications), std::move(client_states), task_start_time_);
+        std::move(notifications), std::move(client_states));
   }
 
   void OnUserAction(const UserActionData& action_data) override {

chrome/browser/notifications/scheduler/internal/notification_scheduler_unittest.cc

@@ -207,7 +207,7 @@ TEST_F(NotificationSchedulerTest, Schedule) {
 
   auto param = std::unique_ptr<NotificationParams>();
   EXPECT_CALL(*notification_manager(), ScheduleNotification(_));
-  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _, _));
+  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
   scheduler()->Schedule(std::move(param));
 }
 
@@ -217,7 +217,7 @@ TEST_F(NotificationSchedulerTest, DeleteAllNotifications) {
 
   // Currently we don't reschedule background task even if all the notifications
   // are deleted.
-  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _, _)).Times(0);
+  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _)).Times(0);
   EXPECT_CALL(*notification_manager(),
               DeleteNotifications(SchedulerClientType::kTest1));
   scheduler()->DeleteAllNotifications(SchedulerClientType::kTest1);
@@ -260,7 +260,7 @@ TEST_F(NotificationSchedulerTest, BackgroundTaskStartShowNothing) {
 
   EXPECT_CALL(*display_agent(), ShowNotification(_, _)).Times(0);
   EXPECT_CALL(*notification_manager(), DisplayNotification(_)).Times(0);
-  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _, _));
+  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
 
   scheduler()->OnStartTask(SchedulerTaskTime::kMorning, base::DoNothing());
 }
@@ -308,7 +308,7 @@ TEST_F(NotificationSchedulerTest, BackgroundTaskStartShowNotification) {
         notification_manager_delegate()->DisplayNotification(std::move(entry));
       }));
 
-  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _, _));
+  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
   scheduler()->OnStartTask(SchedulerTaskTime::kMorning, base::DoNothing());
 
   loop.Run();
@@ -317,7 +317,7 @@ TEST_F(NotificationSchedulerTest, BackgroundTaskStartShowNotification) {
 // Test to simulate a background task stopped by the OS.
 TEST_F(NotificationSchedulerTest, BackgroundTaskStop) {
   Init();
-  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _, _));
+  EXPECT_CALL(*task_coordinator(), ScheduleBackgroundTask(_, _));
   scheduler()->OnStopTask(SchedulerTaskTime::kMorning);
 }
 

chrome/browser/notifications/scheduler/internal/scheduler_config.cc

@@ -18,12 +18,6 @@ constexpr base::TimeDelta kDefaultImpressionExpiration =
 constexpr base::TimeDelta kDefaultSuppressionDuration =
     base::TimeDelta::FromDays(56);
 
-// The morning task by default will run at 7am.
-constexpr int kDefaultMorningTaskHour = 7;
-
-// The evening task by default will run at 6pm.
-constexpr int kDefaultEveningTaskHour = 18;
-
 // Check consecutive notification dismisses in this duration to generate a
 // dismiss event.
 constexpr base::TimeDelta kDefaultDimissDuration = base::TimeDelta::FromDays(7);
@@ -33,8 +27,8 @@ constexpr base::TimeDelta kDefaultBackgroundTaskWindowDuration =
     base::TimeDelta::FromHours(1);
 
 // Default randomized time window to distribute load from user actions.
-constexpr base::TimeDelta kDefaultBackgroundTaskRandomTimeWindow =
-    base::TimeDelta::FromHours(1);
+constexpr base::TimeDelta kDefaultBackgroundTaskMinInterval =
+    base::TimeDelta::FromMinutes(10);
 
 // static
 std::unique_ptr<SchedulerConfig> SchedulerConfig::Create() {
@@ -50,11 +44,8 @@ SchedulerConfig::SchedulerConfig()
       suppression_duration(kDefaultSuppressionDuration),
       dismiss_count(3),
       dismiss_duration(kDefaultDimissDuration),
-      morning_task_hour(kDefaultMorningTaskHour),
-      evening_task_hour(kDefaultEveningTaskHour),
       background_task_window_duration(kDefaultBackgroundTaskWindowDuration),
-      background_task_random_time_window(
-          kDefaultBackgroundTaskRandomTimeWindow) {
+      background_task_min_interval(kDefaultBackgroundTaskMinInterval) {
   // TODO(xingliu): Add constructor using finch data.
 }
 

chrome/browser/notifications/scheduler/internal/scheduler_config.h

@@ -49,19 +49,11 @@ struct SchedulerConfig {
   // in this duration, to generate a dismiss event.
   base::TimeDelta dismiss_duration;
 
-  // The hour (from 0 to 23) to run the morning background task for notification
-  // scheduler.
-  int morning_task_hour;
-
-  // The hour (from 0 to 23) to run the evening background task for notification
-  // scheduler.
-  int evening_task_hour;
-
   // The time window to launch the background task.
   base::TimeDelta background_task_window_duration;
 
-  // A random time delta to distribute the user clicks to a time window.
-  base::TimeDelta background_task_random_time_window;
+  // The minimum interval between background tasks.
+  base::TimeDelta background_task_min_interval;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SchedulerConfig);

chrome/browser/notifications/scheduler/schedule_service_factory_helper.cc

@@ -97,8 +97,6 @@ KeyedService* CreateNotificationScheduleService(
 
   auto background_task_coordinator = BackgroundTaskCoordinator::Create(
       std::move(background_task_scheduler), config.get(),
-      base::BindRepeating(&BackgroundTaskCoordinator::DefaultTimeRandomizer,
-                          config->background_task_random_time_window),
       base::DefaultClock::GetInstance());
 
   auto display_decider = DisplayDecider::Create(

chrome/browser/notifications/scheduler/test/mock_background_task_coordinator.h

@@ -15,10 +15,9 @@ class MockBackgroundTaskCoordinator : public BackgroundTaskCoordinator {
  public:
   MockBackgroundTaskCoordinator();
   ~MockBackgroundTaskCoordinator() override;
-  MOCK_METHOD3(ScheduleBackgroundTask,
+  MOCK_METHOD2(ScheduleBackgroundTask,
                void(BackgroundTaskCoordinator::Notifications notifications,
-                    BackgroundTaskCoordinator::ClientStates client_states,
-                    SchedulerTaskTime task_start_time));
+                    BackgroundTaskCoordinator::ClientStates client_states));
 };
 
 }  // namespace test