[Notification scheduler]: NoThrottle priority support.

- This CL implements support of Priority::NoThrottle notifications in
display_decider flow and background_task_coordinator flow.
- Added unittest.

Bug: 1005008
Change-Id: Iab85bc9dded29729ed62178d807a5153df1acbe6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810079
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Auto-Submit: Hesen Zhang <hesen@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#701338}

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

@@ -27,9 +27,7 @@ class BackgroundTaskCoordinatorHelper {
       NotificationBackgroundTaskScheduler* background_task,
       const SchedulerConfig* config,
       base::Clock* clock)
-      : background_task_(background_task),
-        config_(config),
-        clock_(clock) {}
+      : background_task_(background_task), config_(config), clock_(clock) {}
   ~BackgroundTaskCoordinatorHelper() = default;
 
   void ScheduleBackgroundTask(
@@ -40,6 +38,45 @@ class BackgroundTaskCoordinatorHelper {
       return;
     }
 
+    BackgroundTaskCoordinator::Notifications unthrottled_notifications;
+    BackgroundTaskCoordinator::Notifications throttled_notifications;
+    for (auto& pair : notifications) {
+      for (auto* notification : pair.second) {
+        auto type = pair.first;
+        if (notification->schedule_params.priority ==
+            ScheduleParams::Priority::kNoThrottle) {
+          unthrottled_notifications[type].emplace_back(std::move(notification));
+        } else {
+          throttled_notifications[type].emplace_back(std::move(notification));
+        }
+      }
+    }
+    ProcessUnthrottledNotifications(std::move(unthrottled_notifications));
+    ProcessThrottledNotifications(std::move(throttled_notifications),
+                                  std::move(client_states));
+    ScheduleBackgroundTaskInternal();
+  }
+
+ private:
+  void ProcessUnthrottledNotifications(
+      BackgroundTaskCoordinator::Notifications notifications) {
+    for (const auto& pair : notifications) {
+      for (const auto* entry : pair.second) {
+        DCHECK_EQ(entry->schedule_params.priority,
+                  ScheduleParams::Priority::kNoThrottle);
+        if (!entry->schedule_params.deliver_time_start.has_value()) {
+          continue;
+        }
+        base::Time deliver_time_start =
+            entry->schedule_params.deliver_time_start.value();
+        MaybeUpdateBackgroundTaskTime(deliver_time_start);
+      }
+    }
+  }
+
+  void ProcessThrottledNotifications(
+      BackgroundTaskCoordinator::Notifications notifications,
+      BackgroundTaskCoordinator::ClientStates client_states) {
     base::Time tomorrow;
     base::Time now = clock_->Now();
     bool success = ToLocalHour(0, now, 1 /*day_delta*/, &tomorrow);
@@ -64,6 +101,8 @@ class BackgroundTaskCoordinatorHelper {
 
       // Find the eariliest notification to launch the background task.
       for (const auto* entry : pair.second) {
+        DCHECK_NE(entry->schedule_params.priority,
+                  ScheduleParams::Priority::kNoThrottle);
         // Currently only support deliver time window.
         if (!entry->schedule_params.deliver_time_start.has_value()) {
           continue;
@@ -94,11 +133,8 @@ class BackgroundTaskCoordinatorHelper {
         MaybeUpdateBackgroundTaskTime(deliver_time_start);
       }
     }
-
-    ScheduleBackgroundTaskInternal();
   }
 
- private:
   void MaybeUpdateBackgroundTaskTime(const base::Time& time) {
     if (!background_task_time_.has_value() ||
         time < background_task_time_.value())

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

@@ -6,6 +6,7 @@
 
 #include <map>
 #include <memory>
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -262,5 +263,23 @@ TEST_F(BackgroundTaskCoordinatorTest, MutipleNotifications) {
   ScheduleTask(test_data);
 }
 
+// Verifies that the notification with NoThrottle priority will always trigger
+// background task.
+TEST_F(BackgroundTaskCoordinatorTest, NoThrottleNotifications) {
+  TestData test_data;
+  test_data.impression_test_data = kSingleClientImpressionTestData;
+  test_data.impression_test_data.front().current_max_daily_show = 1;
+  config()->max_daily_shown_all_type = 0;
+  auto entry =
+      CreateNotification(SchedulerClientType::kTest1, kGuid,
+                         kDeliverTimeWindowStart, kDeliverTimeWindowEnd);
+  entry.schedule_params.priority = ScheduleParams::Priority::kNoThrottle;
+  test_data.notification_entries = {entry};
+  EXPECT_CALL(*background_task(),
+              Schedule(_, GetTime(kDeliverTimeWindowStart) - GetTime(kNow), _));
+  EXPECT_CALL(*background_task(), Cancel()).Times(0);
+  ScheduleTask(test_data);
+}
+
 }  // namespace
 }  // namespace notifications

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

@@ -60,10 +60,10 @@ class DecisionHelper {
 
       for (const auto* notification : pair.second) {
         DCHECK(notification);
+        DCHECK_NE(notification->schedule_params.priority,
+                  ScheduleParams::Priority::kNoThrottle);
         if (!ShouldFilterOut(notification))
           filtered_notifications[notification->type].emplace_back(notification);
-        // TODO(xingliu): Filter with priority. Consider to delete notification
-        // with expired scheduling time stamp.
       }
     }
 
@@ -109,7 +109,6 @@ class DecisionHelper {
     size_t steps = 0u;
 
     // Circling around all clients to find new notification to show.
-    // TODO(xingliu): Apply scheduling parameters here.
     do {
       // Move the iterator to next client type.
       DCHECK(it != clients_.end());
@@ -179,12 +178,25 @@ class DisplayDeciderImpl : public DisplayDecider {
 
  private:
   // DisplayDecider implementation.
-  void FindNotificationsToShow(
-      Notifications notifications,
+  void FindNotificationsToShow(Notifications notifications,
                                ClientStates client_states,
                                Results* results) override {
-    auto helper = std::make_unique<DecisionHelper>(config_, clients_, clock_,
-                                                   std::move(notifications),
+    Notifications throttled_notifications;
+    for (const auto& pair : notifications) {
+      auto type = pair.first;
+      for (auto* notification : pair.second) {
+        // Move unthrottled notifications to results directly.
+        if (notification->schedule_params.priority ==
+            ScheduleParams::Priority::kNoThrottle) {
+          results->emplace(notification->guid);
+        } else {
+          throttled_notifications[type].emplace_back(std::move(notification));
+        }
+      }
+    }
+    // Handle throttled notifications.
+    auto helper = std::make_unique<DecisionHelper>(
+        config_, clients_, clock_, std::move(throttled_notifications),
         std::move(client_states));
     helper->DecideNotificationToShow(results);
   }

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

@@ -153,9 +153,7 @@ class DisplayDeciderTest : public testing::Test {
 };
 
 TEST_F(DisplayDeciderTest, NoNotification) {
-  TestData data{kClientsImpressionTestData,
-                {},
-                DisplayDecider::Results()};
+  TestData data{kClientsImpressionTestData, {}, DisplayDecider::Results()};
   RunTestCase(data);
 }
 
@@ -255,5 +253,17 @@ TEST_F(DisplayDeciderTest, ThrottleSuppressedClient) {
   RunTestCase(data);
 }
 
+// Notifitions with NoThrottle Priority should always show.
+TEST_F(DisplayDeciderTest, UnthrottlePriority) {
+  auto impression_test_data = kClientsImpressionTestData;
+  auto entry1 = CreateNotification(SchedulerClientType::kTest1, "guid1");
+  entry1.schedule_params.priority = ScheduleParams::Priority::kNoThrottle;
+  auto entry2 = CreateNotification(SchedulerClientType::kTest1, "guid2");
+  entry2.schedule_params.priority = ScheduleParams::Priority::kNoThrottle;
+  config()->max_daily_shown_all_type = 0;
+  TestData data{impression_test_data, {entry1, entry2}, {"guid1", "guid2"}};
+  RunTestCase(data);
+}
+
 }  // namespace
 }  // namespace notifications