Commit bc9b545c authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Notification scheduler: Add time window randomizer.

This CL adds a time randomizer for notification scheduler background
task.

Bug: 963283
Change-Id: I79f255733b01f6257fb39e603270345c2f08d706
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1708014
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#679824}
parent 95d8123a
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/numerics/ranges.h" #include "base/numerics/ranges.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/rand_util.h"
#include "base/time/clock.h" #include "base/time/clock.h"
#include "chrome/browser/notifications/scheduler/internal/impression_types.h" #include "chrome/browser/notifications/scheduler/internal/impression_types.h"
#include "chrome/browser/notifications/scheduler/internal/scheduler_config.h" #include "chrome/browser/notifications/scheduler/internal/scheduler_config.h"
...@@ -25,8 +26,12 @@ class BackgroundTaskCoordinatorHelper { ...@@ -25,8 +26,12 @@ class BackgroundTaskCoordinatorHelper {
BackgroundTaskCoordinatorHelper( BackgroundTaskCoordinatorHelper(
NotificationBackgroundTaskScheduler* background_task, NotificationBackgroundTaskScheduler* background_task,
const SchedulerConfig* config, const SchedulerConfig* config,
BackgroundTaskCoordinator::TimeRandomizer time_randomizer,
base::Clock* clock) base::Clock* clock)
: background_task_(background_task), config_(config), clock_(clock) {} : background_task_(background_task),
config_(config),
time_randomizer_(time_randomizer),
clock_(clock) {}
~BackgroundTaskCoordinatorHelper() = default; ~BackgroundTaskCoordinatorHelper() = default;
void ScheduleBackgroundTask( void ScheduleBackgroundTask(
...@@ -153,13 +158,22 @@ class BackgroundTaskCoordinatorHelper { ...@@ -153,13 +158,22 @@ class BackgroundTaskCoordinatorHelper {
return; 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( background_task_->Schedule(
task_start_time, window_start_time, task_start_time, window_start_time + random_interval,
window_start_time + config_->background_task_window_duration); window_start_time + config_->background_task_window_duration +
random_interval);
} }
NotificationBackgroundTaskScheduler* background_task_; NotificationBackgroundTaskScheduler* background_task_;
const SchedulerConfig* config_; const SchedulerConfig* config_;
BackgroundTaskCoordinator::TimeRandomizer time_randomizer_;
base::Clock* clock_; base::Clock* clock_;
base::Optional<base::Time> background_task_time_; base::Optional<base::Time> background_task_time_;
...@@ -168,12 +182,20 @@ class BackgroundTaskCoordinatorHelper { ...@@ -168,12 +182,20 @@ class BackgroundTaskCoordinatorHelper {
} // namespace } // namespace
// static
base::TimeDelta BackgroundTaskCoordinator::DefaultTimeRandomizer(
const base::TimeDelta& time_window) {
return base::RandDouble() * time_window;
}
BackgroundTaskCoordinator::BackgroundTaskCoordinator( BackgroundTaskCoordinator::BackgroundTaskCoordinator(
std::unique_ptr<NotificationBackgroundTaskScheduler> background_task, std::unique_ptr<NotificationBackgroundTaskScheduler> background_task,
const SchedulerConfig* config, const SchedulerConfig* config,
TimeRandomizer time_randomizer,
base::Clock* clock) base::Clock* clock)
: background_task_(std::move(background_task)), : background_task_(std::move(background_task)),
config_(config), config_(config),
time_randomizer_(time_randomizer),
clock_(clock) {} clock_(clock) {}
BackgroundTaskCoordinator::~BackgroundTaskCoordinator() = default; BackgroundTaskCoordinator::~BackgroundTaskCoordinator() = default;
...@@ -183,7 +205,7 @@ void BackgroundTaskCoordinator::ScheduleBackgroundTask( ...@@ -183,7 +205,7 @@ void BackgroundTaskCoordinator::ScheduleBackgroundTask(
ClientStates client_states, ClientStates client_states,
SchedulerTaskTime task_start_time) { SchedulerTaskTime task_start_time) {
auto helper = std::make_unique<BackgroundTaskCoordinatorHelper>( auto helper = std::make_unique<BackgroundTaskCoordinatorHelper>(
background_task_.get(), config_, clock_); background_task_.get(), config_, time_randomizer_, clock_);
helper->ScheduleBackgroundTask(std::move(notifications), helper->ScheduleBackgroundTask(std::move(notifications),
std::move(client_states), task_start_time); std::move(client_states), task_start_time);
} }
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <memory> #include <memory>
#include <vector> #include <vector>
#include "base/callback.h"
#include "base/macros.h" #include "base/macros.h"
#include "chrome/browser/notifications/scheduler/public/notification_scheduler_types.h" #include "chrome/browser/notifications/scheduler/public/notification_scheduler_types.h"
...@@ -30,9 +31,13 @@ class BackgroundTaskCoordinator { ...@@ -30,9 +31,13 @@ class BackgroundTaskCoordinator {
using Notifications = using Notifications =
std::map<SchedulerClientType, std::vector<const NotificationEntry*>>; std::map<SchedulerClientType, std::vector<const NotificationEntry*>>;
using ClientStates = std::map<SchedulerClientType, const ClientState*>; using ClientStates = std::map<SchedulerClientType, const ClientState*>;
using TimeRandomizer = base::RepeatingCallback<base::TimeDelta()>;
static base::TimeDelta DefaultTimeRandomizer(
const base::TimeDelta& time_window);
BackgroundTaskCoordinator( BackgroundTaskCoordinator(
std::unique_ptr<NotificationBackgroundTaskScheduler> background_task, std::unique_ptr<NotificationBackgroundTaskScheduler> background_task,
const SchedulerConfig* config, const SchedulerConfig* config,
TimeRandomizer time_randomizer,
base::Clock* clock); base::Clock* clock);
virtual ~BackgroundTaskCoordinator(); virtual ~BackgroundTaskCoordinator();
...@@ -48,6 +53,10 @@ class BackgroundTaskCoordinator { ...@@ -48,6 +53,10 @@ class BackgroundTaskCoordinator {
// System configuration. // System configuration.
const SchedulerConfig* config_; 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. // Clock to query the current timestamp.
base::Clock* clock_; base::Clock* clock_;
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include <utility> #include <utility>
#include <vector> #include <vector>
#include "base/bind.h"
#include "base/test/scoped_task_environment.h" #include "base/test/scoped_task_environment.h"
#include "chrome/browser/notifications/scheduler/internal/notification_entry.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_config.h"
...@@ -58,6 +59,10 @@ class MockNotificationBackgroundTaskScheduler ...@@ -58,6 +59,10 @@ class MockNotificationBackgroundTaskScheduler
DISALLOW_COPY_AND_ASSIGN(MockNotificationBackgroundTaskScheduler); DISALLOW_COPY_AND_ASSIGN(MockNotificationBackgroundTaskScheduler);
}; };
base::TimeDelta NoopTimeRandomizer(const base::TimeDelta& time_window) {
return base::TimeDelta();
}
struct TestData { struct TestData {
// Impression data as the input. // Impression data as the input.
std::vector<test::ImpressionTestData> impression_test_data; std::vector<test::ImpressionTestData> impression_test_data;
...@@ -87,7 +92,8 @@ class BackgroundTaskCoordinatorTest : public testing::Test { ...@@ -87,7 +92,8 @@ class BackgroundTaskCoordinatorTest : public testing::Test {
std::make_unique<MockNotificationBackgroundTaskScheduler>(); std::make_unique<MockNotificationBackgroundTaskScheduler>();
background_task_ = background_task.get(); background_task_ = background_task.get();
coordinator_ = std::make_unique<BackgroundTaskCoordinator>( coordinator_ = std::make_unique<BackgroundTaskCoordinator>(
std::move(background_task), &config_, &clock_); std::move(background_task), &config_,
base::BindRepeating(&NoopTimeRandomizer, base::TimeDelta()), &clock_);
} }
MockNotificationBackgroundTaskScheduler* background_task() { MockNotificationBackgroundTaskScheduler* background_task() {
...@@ -336,5 +342,15 @@ TEST_F(BackgroundTaskCoordinatorTest, ScheduleNewNotification) { ...@@ -336,5 +342,15 @@ TEST_F(BackgroundTaskCoordinatorTest, ScheduleNewNotification) {
TestScheduleNewNotification("04/25/20 18:30:00 PM", "04/26/20 06:00:00 AM"); 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
} // namespace notifications } // namespace notifications
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <memory> #include <memory>
#include <utility> #include <utility>
#include "base/bind.h"
#include "base/time/default_clock.h" #include "base/time/default_clock.h"
#include "chrome/browser/notifications/scheduler/internal/background_task_coordinator.h" #include "chrome/browser/notifications/scheduler/internal/background_task_coordinator.h"
#include "chrome/browser/notifications/scheduler/internal/display_decider.h" #include "chrome/browser/notifications/scheduler/internal/display_decider.h"
...@@ -37,6 +38,8 @@ NotificationSchedulerContext::NotificationSchedulerContext( ...@@ -37,6 +38,8 @@ NotificationSchedulerContext::NotificationSchedulerContext(
background_task_coordinator_(std::make_unique<BackgroundTaskCoordinator>( background_task_coordinator_(std::make_unique<BackgroundTaskCoordinator>(
std::move(background_task), std::move(background_task),
config_.get(), config_.get(),
base::BindRepeating(&BackgroundTaskCoordinator::DefaultTimeRandomizer,
config_->background_task_random_time_window),
base::DefaultClock::GetInstance())) {} base::DefaultClock::GetInstance())) {}
NotificationSchedulerContext::~NotificationSchedulerContext() = default; NotificationSchedulerContext::~NotificationSchedulerContext() = default;
......
...@@ -32,6 +32,10 @@ constexpr base::TimeDelta kDefaultDimissDuration = base::TimeDelta::FromDays(7); ...@@ -32,6 +32,10 @@ constexpr base::TimeDelta kDefaultDimissDuration = base::TimeDelta::FromDays(7);
constexpr base::TimeDelta kDefaultBackgroundTaskWindowDuration = constexpr base::TimeDelta kDefaultBackgroundTaskWindowDuration =
base::TimeDelta::FromHours(1); base::TimeDelta::FromHours(1);
// Default randomized time window to distribute load from user actions.
constexpr base::TimeDelta kDefaultBackgroundTaskRandomTimeWindow =
base::TimeDelta::FromHours(1);
// static // static
std::unique_ptr<SchedulerConfig> SchedulerConfig::Create() { std::unique_ptr<SchedulerConfig> SchedulerConfig::Create() {
return std::make_unique<SchedulerConfig>(); return std::make_unique<SchedulerConfig>();
...@@ -48,7 +52,9 @@ SchedulerConfig::SchedulerConfig() ...@@ -48,7 +52,9 @@ SchedulerConfig::SchedulerConfig()
dismiss_duration(kDefaultDimissDuration), dismiss_duration(kDefaultDimissDuration),
morning_task_hour(kDefaultMorningTaskHour), morning_task_hour(kDefaultMorningTaskHour),
evening_task_hour(kDefaultEveningTaskHour), evening_task_hour(kDefaultEveningTaskHour),
background_task_window_duration(kDefaultBackgroundTaskWindowDuration) { background_task_window_duration(kDefaultBackgroundTaskWindowDuration),
background_task_random_time_window(
kDefaultBackgroundTaskRandomTimeWindow) {
// TODO(xingliu): Add constructor using finch data. // TODO(xingliu): Add constructor using finch data.
} }
......
...@@ -60,6 +60,9 @@ struct SchedulerConfig { ...@@ -60,6 +60,9 @@ struct SchedulerConfig {
// The time window to launch the background task. // The time window to launch the background task.
base::TimeDelta background_task_window_duration; 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;
private: private:
DISALLOW_COPY_AND_ASSIGN(SchedulerConfig); DISALLOW_COPY_AND_ASSIGN(SchedulerConfig);
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment