diff --git a/components/drive/drive_notification_manager.cc b/components/drive/drive_notification_manager.cc index 3f789e5d5f10f2..175d304ec2d2e6 100644 --- a/components/drive/drive_notification_manager.cc +++ b/components/drive/drive_notification_manager.cc @@ -8,6 +8,7 @@ #include #include "base/metrics/histogram_macros.h" +#include "base/rand_util.h" #include "base/strings/char_traits.h" #include "base/strings/strcat.h" #include "base/strings/string_util.h" @@ -25,11 +26,10 @@ const int kFastPollingIntervalInSecs = 60; // The polling interval time is used when XMPP is enabled. Theoretically // polling should be unnecessary if XMPP is enabled, but just in case. -const int kSlowPollingIntervalInSecs = 300; +const int kSlowPollingIntervalInSecs = 3600; // The period to batch together invalidations before passing them to observers. -constexpr base::TimeDelta kInvalidationBatchInterval = - base::TimeDelta::FromSeconds(5); +constexpr int kInvalidationBatchIntervalSecs = 15; // The sync invalidation object ID for Google Drive. const char kDriveInvalidationObjectId[] = "CHANGELOG"; @@ -49,16 +49,11 @@ DriveNotificationManager::DriveNotificationManager( push_notification_registered_(false), push_notification_enabled_(false), observers_notified_(false), + batch_timer_(clock), weak_ptr_factory_(this) { DCHECK(invalidation_service_); RegisterDriveNotifications(); RestartPollingTimer(); - - batch_timer_ = std::make_unique( - FROM_HERE, kInvalidationBatchInterval, - base::BindRepeating(&DriveNotificationManager::OnBatchTimerExpired, - weak_ptr_factory_.GetWeakPtr()), - clock); } DriveNotificationManager::~DriveNotificationManager() { @@ -122,12 +117,12 @@ void DriveNotificationManager::OnIncomingInvalidation( // See crbug.com/320878. invalidation_map.AcknowledgeAll(); - if (!batch_timer_->IsRunning() && !invalidated_change_ids_.empty()) { + if (!batch_timer_.IsRunning() && !invalidated_change_ids_.empty()) { // Stop the polling timer as we'll be sending a batch soon. polling_timer_.Stop(); // Restart the timer to send the batch when the timer fires. - batch_timer_->Reset(); + RestartBatchTimer(); } } @@ -175,14 +170,30 @@ void DriveNotificationManager::RestartPollingTimer() { const int interval_secs = (push_notification_enabled_ ? kSlowPollingIntervalInSecs : kFastPollingIntervalInSecs); + + int jitter = base::RandInt(0, interval_secs); + polling_timer_.Stop(); polling_timer_.Start( - FROM_HERE, base::TimeDelta::FromSeconds(interval_secs), + FROM_HERE, base::TimeDelta::FromSeconds(interval_secs + jitter), base::BindOnce(&DriveNotificationManager::NotifyObserversToUpdate, weak_ptr_factory_.GetWeakPtr(), NOTIFICATION_POLLING, std::map())); } +void DriveNotificationManager::RestartBatchTimer() { + DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); + + int jitter = base::RandInt(0, kInvalidationBatchIntervalSecs); + + batch_timer_.Stop(); + batch_timer_.Start( + FROM_HERE, + base::TimeDelta::FromSeconds(kInvalidationBatchIntervalSecs + jitter), + base::BindOnce(&DriveNotificationManager::OnBatchTimerExpired, + weak_ptr_factory_.GetWeakPtr())); +} + void DriveNotificationManager::NotifyObserversToUpdate( NotificationSource source, std::map invalidations) { diff --git a/components/drive/drive_notification_manager.h b/components/drive/drive_notification_manager.h index 38febf49e671df..ba6981b3642c53 100644 --- a/components/drive/drive_notification_manager.h +++ b/components/drive/drive_notification_manager.h @@ -86,6 +86,10 @@ class DriveNotificationManager : public KeyedService, // Restarts the polling timer. Used for polling-based notification. void RestartPollingTimer(); + // Restarts the batch notification timer. Used for batching together XMPP + // notifications so we can smooth out the traffic on the drive backends. + void RestartBatchTimer(); + // Notifies the observers that it's time to check for updates. // |source| indicates where the notification comes from. void NotifyObserversToUpdate(NotificationSource source, @@ -123,7 +127,7 @@ class DriveNotificationManager : public KeyedService, // This timer is used to batch together invalidations. The invalidation // service can send many invalidations for the same id in rapid succession, // batching them together and removing duplicates is an optimzation. - std::unique_ptr batch_timer_; + base::OneShotTimer batch_timer_; // The batch of invalidation id's that we've seen from the invaliation // service, will be reset when when send the invalidations to the observers. diff --git a/components/drive/drive_notification_manager_unittest.cc b/components/drive/drive_notification_manager_unittest.cc index c34a7d1e06064c..0a9543bbcb6952 100644 --- a/components/drive/drive_notification_manager_unittest.cc +++ b/components/drive/drive_notification_manager_unittest.cc @@ -162,7 +162,7 @@ TEST_F(DriveNotificationManagerTest, TestBatchInvalidation) { syncer::Invalidation::InitUnknownVersion(kDefaultCorpusObjectId)); EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty()); - task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5)); + task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(30)); // Default corpus is has the id "" when sent to observers. std::map expected_ids = {{"", -1}}; @@ -181,7 +181,7 @@ TEST_F(DriveNotificationManagerTest, TestBatchInvalidation) { syncer::Invalidation::Init(kDefaultCorpusObjectId, 1, "")); EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty()); - task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5)); + task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(30)); // Default corpus is has the id "" when sent to observers. expected_ids = {{"", 2}}; @@ -193,7 +193,7 @@ TEST_F(DriveNotificationManagerTest, TestBatchInvalidation) { syncer::Invalidation::Init(team_drive_1_object_id, 2, "")); EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty()); - task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5)); + task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(30)); expected_ids = {{team_drive_id_1, 2}}; EXPECT_EQ(expected_ids, drive_notification_observer_->GetNotificationIds()); drive_notification_observer_->ClearNotificationIds(); @@ -214,7 +214,7 @@ TEST_F(DriveNotificationManagerTest, TestBatchInvalidation) { EXPECT_TRUE(drive_notification_observer_->GetNotificationIds().empty()); - task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(5)); + task_runner_->FastForwardBy(base::TimeDelta::FromSeconds(30)); expected_ids = {{"", 2}, {team_drive_id_1, 2}}; EXPECT_EQ(expected_ids, drive_notification_observer_->GetNotificationIds()); }