diff options
author | kinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-08-01 04:12:40 +0000 |
---|---|---|
committer | kinaba@chromium.org <kinaba@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-08-01 04:12:40 +0000 |
commit | 67515674866ea851ce96beb66d037e05265f6b4f (patch) | |
tree | 57f370cb0458044b6d931426247db130e07cf329 /chrome | |
parent | ae211b853f2dcb22e65a41f7442a09ba4cbd2a66 (diff) | |
download | chromium_src-67515674866ea851ce96beb66d037e05265f6b4f.zip chromium_src-67515674866ea851ce96beb66d037e05265f6b4f.tar.gz chromium_src-67515674866ea851ce96beb66d037e05265f6b4f.tar.bz2 |
Control the frequency of fileBrowserPrivate.onFileTransfersUpdated events.
The update notifications were sent at every download progress and flooded the
file manager UI. This patch inserts a check on frequency so that the event
keeps the interval of certain (currently 1000) milliseconds.
BUG=138838
TEST=manually tested copying large files.
Review URL: https://chromiumcodereview.appspot.com/10834027
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149371 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
3 files changed, 80 insertions, 21 deletions
diff --git a/chrome/browser/chromeos/gdata/gdata_operation_registry.cc b/chrome/browser/chromeos/gdata/gdata_operation_registry.cc index f1c07dd..287691f 100644 --- a/chrome/browser/chromeos/gdata/gdata_operation_registry.cc +++ b/chrome/browser/chromeos/gdata/gdata_operation_registry.cc @@ -9,6 +9,12 @@ using content::BrowserThread; +namespace { + +const int64 kNotificationFrequencyInMilliseconds = 1000; + +} // namespace + namespace gdata { // static @@ -137,7 +143,8 @@ void GDataOperationRegistry::Operation::NotifyAuthFailed() { registry_->OnOperationAuthFailed(); } -GDataOperationRegistry::GDataOperationRegistry() { +GDataOperationRegistry::GDataOperationRegistry() + : do_notification_frequency_control_(true) { in_flight_operations_.set_check_on_null_data(true); } @@ -153,6 +160,10 @@ void GDataOperationRegistry::RemoveObserver(Observer* observer) { observer_list_.RemoveObserver(observer); } +void GDataOperationRegistry::DisableNotificationFrequencyControlForTest() { + do_notification_frequency_control_ = false; +} + void GDataOperationRegistry::CancelAll() { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI)); @@ -189,10 +200,8 @@ void GDataOperationRegistry::OnOperationStart( *id = in_flight_operations_.Add(operation); DVLOG(1) << "GDataOperation[" << *id << "] started."; - if (IsFileTransferOperation(operation)) { - FOR_EACH_OBSERVER(Observer, observer_list_, - OnProgressUpdate(GetProgressStatusList())); - } + if (IsFileTransferOperation(operation)) + NotifyStatusToObservers(); } void GDataOperationRegistry::OnOperationProgress(OperationID id) { @@ -203,10 +212,8 @@ void GDataOperationRegistry::OnOperationProgress(OperationID id) { DVLOG(1) << "GDataOperation[" << id << "] " << operation->progress_status().DebugString(); - if (IsFileTransferOperation(operation)) { - FOR_EACH_OBSERVER(Observer, observer_list_, - OnProgressUpdate(GetProgressStatusList())); - } + if (IsFileTransferOperation(operation)) + NotifyStatusToObservers(); } void GDataOperationRegistry::OnOperationFinish(OperationID id) { @@ -216,10 +223,8 @@ void GDataOperationRegistry::OnOperationFinish(OperationID id) { DCHECK(operation); DVLOG(1) << "GDataOperation[" << id << "] finished."; - if (IsFileTransferOperation(operation)) { - FOR_EACH_OBSERVER(Observer, observer_list_, - OnProgressUpdate(GetProgressStatusList())); - } + if (IsFileTransferOperation(operation)) + NotifyStatusToObservers(); in_flight_operations_.Remove(id); } @@ -256,10 +261,8 @@ void GDataOperationRegistry::OnOperationResume( new_status->operation_id = in_flight_operations_.Add(operation); DVLOG(1) << "GDataOperation[" << old_id << " -> " << new_status->operation_id << "] resumed."; - if (IsFileTransferOperation(operation)) { - FOR_EACH_OBSERVER(Observer, observer_list_, - OnProgressUpdate(GetProgressStatusList())); - } + if (IsFileTransferOperation(operation)) + NotifyStatusToObservers(); } void GDataOperationRegistry::OnOperationSuspend(OperationID id) { @@ -269,10 +272,8 @@ void GDataOperationRegistry::OnOperationSuspend(OperationID id) { DCHECK(operation); DVLOG(1) << "GDataOperation[" << id << "] suspended."; - if (IsFileTransferOperation(operation)) { - FOR_EACH_OBSERVER(Observer, observer_list_, - OnProgressUpdate(GetProgressStatusList())); - } + if (IsFileTransferOperation(operation)) + NotifyStatusToObservers(); } void GDataOperationRegistry::OnOperationAuthFailed() { @@ -303,4 +304,45 @@ GDataOperationRegistry::GetProgressStatusList() { return status_list; } +bool GDataOperationRegistry::ShouldNotifyStatusNow( + const ProgressStatusList& list) { + if (!do_notification_frequency_control_) + return true; + + base::Time now = base::Time::Now(); + + // If it is a first event, or some time abnormality is detected, we should + // not skip this notification. + if (last_notification_.is_null() || now < last_notification_) { + last_notification_ = now; + return true; + } + + // If sufficiently long time has elapsed since the previous event, we should + // not skip this notification. + if ((now - last_notification_).InMilliseconds() >= + kNotificationFrequencyInMilliseconds) { + last_notification_ = now; + return true; + } + + // If important events (OPERATION_STARTED, COMPLETED, or FAILED) are there, + // we should not skip this notification. + for (size_t i = 0; i < list.size(); ++i) { + if (list[i].transfer_state != OPERATION_IN_PROGRESS) { + last_notification_ = now; + return true; + } + } + + // Otherwise we can skip it. + return false; +} + +void GDataOperationRegistry::NotifyStatusToObservers() { + ProgressStatusList list(GetProgressStatusList()); + if (ShouldNotifyStatusNow(list)) + FOR_EACH_OBSERVER(Observer, observer_list_, OnProgressUpdate(list)); +} + } // namespace gdata diff --git a/chrome/browser/chromeos/gdata/gdata_operation_registry.h b/chrome/browser/chromeos/gdata/gdata_operation_registry.h index 9749ada..7e34245 100644 --- a/chrome/browser/chromeos/gdata/gdata_operation_registry.h +++ b/chrome/browser/chromeos/gdata/gdata_operation_registry.h @@ -143,6 +143,9 @@ class GDataOperationRegistry { void AddObserver(Observer* observer); void RemoveObserver(Observer* observer); + // Disables the notification suppression for testing purpose. + void DisableNotificationFrequencyControlForTest(); + private: // Handlers for notifications from Operations. friend class Operation; @@ -157,9 +160,18 @@ class GDataOperationRegistry { bool IsFileTransferOperation(const Operation* operation) const; + // Controls the frequency of notifications, not to flood the listeners with + // too many events. + bool ShouldNotifyStatusNow(const ProgressStatusList& list); + // Sends notifications to the observers after checking that the frequency is + // not too high by ShouldNotifyStatusNow. + void NotifyStatusToObservers(); + typedef IDMap<Operation, IDMapOwnPointer> OperationIDMap; OperationIDMap in_flight_operations_; ObserverList<Observer> observer_list_; + base::Time last_notification_; + bool do_notification_frequency_control_; DISALLOW_COPY_AND_ASSIGN(GDataOperationRegistry); }; diff --git a/chrome/browser/chromeos/gdata/gdata_operation_registry_unittest.cc b/chrome/browser/chromeos/gdata/gdata_operation_registry_unittest.cc index 4d56ec3..9d3fe6d 100644 --- a/chrome/browser/chromeos/gdata/gdata_operation_registry_unittest.cc +++ b/chrome/browser/chromeos/gdata/gdata_operation_registry_unittest.cc @@ -128,6 +128,7 @@ class GDataOperationRegistryTest : public testing::Test { TEST_F(GDataOperationRegistryTest, OneSuccess) { TestObserver observer; GDataOperationRegistry registry; + registry.DisableNotificationFrequencyControlForTest(); registry.AddObserver(&observer); base::WeakPtr<MockOperation> op1 = @@ -152,6 +153,7 @@ TEST_F(GDataOperationRegistryTest, OneSuccess) { TEST_F(GDataOperationRegistryTest, OneCancel) { TestObserver observer; GDataOperationRegistry registry; + registry.DisableNotificationFrequencyControlForTest(); registry.AddObserver(&observer); base::WeakPtr<MockOperation> op1 = @@ -172,6 +174,7 @@ TEST_F(GDataOperationRegistryTest, OneCancel) { TEST_F(GDataOperationRegistryTest, TwoSuccess) { TestObserver observer; GDataOperationRegistry registry; + registry.DisableNotificationFrequencyControlForTest(); registry.AddObserver(&observer); base::WeakPtr<MockOperation> op1 = @@ -204,6 +207,7 @@ TEST_F(GDataOperationRegistryTest, TwoSuccess) { TEST_F(GDataOperationRegistryTest, ThreeCancel) { TestObserver observer; GDataOperationRegistry registry; + registry.DisableNotificationFrequencyControlForTest(); registry.AddObserver(&observer); base::WeakPtr<MockOperation> op1 = @@ -234,6 +238,7 @@ TEST_F(GDataOperationRegistryTest, ThreeCancel) { TEST_F(GDataOperationRegistryTest, RestartOperation) { TestObserver observer; GDataOperationRegistry registry; + registry.DisableNotificationFrequencyControlForTest(); registry.AddObserver(&observer); base::WeakPtr<MockOperation> op1 = |