diff options
| author | jkarlin <jkarlin@chromium.org> | 2015-11-23 13:50:10 -0800 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2015-11-23 21:51:51 +0000 |
| commit | 7736b6981249800392e4fdde226cd4cab2c1835e (patch) | |
| tree | 211a95ac46825138595f6d67bc7026123acfefbd /content/browser/background_sync | |
| parent | 881ef3f3d4e0738c30af2ef47a728dba5dfa94a0 (diff) | |
| download | chromium_src-7736b6981249800392e4fdde226cd4cab2c1835e.zip chromium_src-7736b6981249800392e4fdde226cd4cab2c1835e.tar.gz chromium_src-7736b6981249800392e4fdde226cd4cab2c1835e.tar.bz2 | |
[BackgroundSync] Split failed and succeeded cases of BackgroundSyncMetrics::CountRegister
Doing this prevents us from logging bogus values (for duplicate and could_fire) in case of failure.
BUG=559143
Review URL: https://codereview.chromium.org/1465533006
Cr-Commit-Position: refs/heads/master@{#361194}
Diffstat (limited to 'content/browser/background_sync')
3 files changed, 64 insertions, 60 deletions
diff --git a/content/browser/background_sync/background_sync_manager.cc b/content/browser/background_sync/background_sync_manager.cc index 97b637f..071b97a 100644 --- a/content/browser/background_sync/background_sync_manager.cc +++ b/content/browser/background_sync/background_sync_manager.cc @@ -174,17 +174,9 @@ void BackgroundSyncManager::Register( const StatusAndRegistrationCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - // For UMA, determine here whether the sync could fire immediately - BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire = - AreOptionConditionsMet(options) - ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE - : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE; - if (disabled_) { - BackgroundSyncMetrics::CountRegister( - options.periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, - BACKGROUND_SYNC_STATUS_STORAGE_ERROR); + BackgroundSyncMetrics::CountRegisterFailure( + options.periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR); PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback); return; } @@ -407,17 +399,9 @@ void BackgroundSyncManager::RegisterImpl( const StatusAndRegistrationCallback& callback) { DCHECK_CURRENTLY_ON(BrowserThread::IO); - // For UMA, determine here whether the sync could fire immediately - BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire = - AreOptionConditionsMet(options) - ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE - : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE; - if (disabled_) { - BackgroundSyncMetrics::CountRegister( - options.periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, - BACKGROUND_SYNC_STATUS_STORAGE_ERROR); + BackgroundSyncMetrics::CountRegisterFailure( + options.periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR); PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback); return; } @@ -430,10 +414,8 @@ void BackgroundSyncManager::RegisterImpl( } if (options.tag.length() > kMaxTagLength) { - BackgroundSyncMetrics::CountRegister( - options.periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, - BACKGROUND_SYNC_STATUS_NOT_ALLOWED); + BackgroundSyncMetrics::CountRegisterFailure( + options.periodicity, BACKGROUND_SYNC_STATUS_NOT_ALLOWED); PostErrorResponse(BACKGROUND_SYNC_STATUS_NOT_ALLOWED, callback); return; } @@ -441,10 +423,8 @@ void BackgroundSyncManager::RegisterImpl( ServiceWorkerRegistration* sw_registration = service_worker_context_->GetLiveRegistration(sw_registration_id); if (!sw_registration || !sw_registration->active_version()) { - BackgroundSyncMetrics::CountRegister( - options.periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, - BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER); + BackgroundSyncMetrics::CountRegisterFailure( + options.periodicity, BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER); PostErrorResponse(BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER, callback); return; } @@ -468,11 +448,14 @@ void BackgroundSyncManager::RegisterImpl( BackgroundSyncRegistration* existing_registration = existing_registration_ref->value(); - // Record the duplicated registration - BackgroundSyncMetrics::CountRegister( - existing_registration->options()->periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_DUPLICATE, - BACKGROUND_SYNC_STATUS_OK); + BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire = + AreOptionConditionsMet(options) + ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE + : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE; + BackgroundSyncMetrics::CountRegisterSuccess( + existing_registration->options()->periodicity, + registration_could_fire, + BackgroundSyncMetrics::REGISTRATION_IS_DUPLICATE); if (existing_registration->IsFiring()) { existing_registration->set_sync_state( @@ -646,17 +629,10 @@ void BackgroundSyncManager::RegisterDidStore( const BackgroundSyncRegistration* new_registration = new_registration_ref->value(); - // For UMA, determine here whether the sync could fire immediately - BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire = - AreOptionConditionsMet(*new_registration->options()) - ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE - : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE; - if (status == SERVICE_WORKER_ERROR_NOT_FOUND) { // The service worker registration is gone. - BackgroundSyncMetrics::CountRegister( - new_registration->options()->periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, + BackgroundSyncMetrics::CountRegisterFailure( + new_registration->options()->periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR); active_registrations_.erase(sw_registration_id); PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback); @@ -666,9 +642,8 @@ void BackgroundSyncManager::RegisterDidStore( if (status != SERVICE_WORKER_OK) { LOG(ERROR) << "BackgroundSync failed to store registration due to backend " "failure."; - BackgroundSyncMetrics::CountRegister( - new_registration->options()->periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, + BackgroundSyncMetrics::CountRegisterFailure( + new_registration->options()->periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR); DisableAndClearManager(base::Bind( callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR, @@ -676,10 +651,14 @@ void BackgroundSyncManager::RegisterDidStore( return; } - BackgroundSyncMetrics::CountRegister( + BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire = + AreOptionConditionsMet(*new_registration->options()) + ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE + : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE; + BackgroundSyncMetrics::CountRegisterSuccess( new_registration->options()->periodicity, registration_could_fire, - BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, - BACKGROUND_SYNC_STATUS_OK); + BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE); + FireReadyEvents(); base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, diff --git a/content/browser/background_sync/background_sync_metrics.cc b/content/browser/background_sync/background_sync_metrics.cc index 4bfef5c..dddec9b 100644 --- a/content/browser/background_sync/background_sync_metrics.cc +++ b/content/browser/background_sync/background_sync_metrics.cc @@ -32,6 +32,7 @@ ResultPattern EventResultToResultPattern(bool success, namespace content { +// static void BackgroundSyncMetrics::RecordEventResult(SyncPeriodicity periodicity, bool success, bool finished_in_foreground) { @@ -52,6 +53,7 @@ void BackgroundSyncMetrics::RecordEventResult(SyncPeriodicity periodicity, NOTREACHED(); } +// static void BackgroundSyncMetrics::RecordBatchSyncEventComplete( const base::TimeDelta& time, int number_of_batched_sync_events) { @@ -64,14 +66,15 @@ void BackgroundSyncMetrics::RecordBatchSyncEventComplete( number_of_batched_sync_events); } -void BackgroundSyncMetrics::CountRegister( +// static +void BackgroundSyncMetrics::CountRegisterSuccess( SyncPeriodicity periodicity, RegistrationCouldFire registration_could_fire, - RegistrationIsDuplicate registration_is_duplicate, - BackgroundSyncStatus result) { + RegistrationIsDuplicate registration_is_duplicate) { switch (periodicity) { case SYNC_ONE_SHOT: - UMA_HISTOGRAM_ENUMERATION("BackgroundSync.Registration.OneShot", result, + UMA_HISTOGRAM_ENUMERATION("BackgroundSync.Registration.OneShot", + BACKGROUND_SYNC_STATUS_OK, BACKGROUND_SYNC_STATUS_MAX + 1); UMA_HISTOGRAM_BOOLEAN("BackgroundSync.Registration.OneShot.CouldFire", registration_could_fire == REGISTRATION_COULD_FIRE); @@ -80,7 +83,8 @@ void BackgroundSyncMetrics::CountRegister( registration_is_duplicate == REGISTRATION_IS_DUPLICATE); return; case SYNC_PERIODIC: - UMA_HISTOGRAM_ENUMERATION("BackgroundSync.Registration.Periodic", result, + UMA_HISTOGRAM_ENUMERATION("BackgroundSync.Registration.Periodic", + BACKGROUND_SYNC_STATUS_OK, BACKGROUND_SYNC_STATUS_MAX + 1); UMA_HISTOGRAM_BOOLEAN( "BackgroundSync.Registration.Periodic.IsDuplicate", @@ -90,6 +94,23 @@ void BackgroundSyncMetrics::CountRegister( NOTREACHED(); } +// static +void BackgroundSyncMetrics::CountRegisterFailure(SyncPeriodicity periodicity, + BackgroundSyncStatus result) { + switch (periodicity) { + case SYNC_ONE_SHOT: + UMA_HISTOGRAM_ENUMERATION("BackgroundSync.Registration.OneShot", result, + BACKGROUND_SYNC_STATUS_MAX + 1); + return; + case SYNC_PERIODIC: + UMA_HISTOGRAM_ENUMERATION("BackgroundSync.Registration.Periodic", result, + BACKGROUND_SYNC_STATUS_MAX + 1); + return; + } + NOTREACHED(); +} + +// static void BackgroundSyncMetrics::CountUnregister(SyncPeriodicity periodicity, BackgroundSyncStatus result) { switch (periodicity) { diff --git a/content/browser/background_sync/background_sync_metrics.h b/content/browser/background_sync/background_sync_metrics.h index 440ce3f..2c958b3 100644 --- a/content/browser/background_sync/background_sync_metrics.h +++ b/content/browser/background_sync/background_sync_metrics.h @@ -38,13 +38,17 @@ class BackgroundSyncMetrics { static void RecordBatchSyncEventComplete(const base::TimeDelta& time, int number_of_batched_sync_events); - // Records the result of trying to register a sync. |could_fire| indicates - // whether the conditions were sufficient for the sync to fire immediately at - // the time it was registered. - static void CountRegister(SyncPeriodicity periodicity, - RegistrationCouldFire could_fire, - RegistrationIsDuplicate registration_is_duplicate, - BackgroundSyncStatus result); + // Records the result of successfully registering a sync. |could_fire| + // indicates whether the conditions were sufficient for the sync to fire + // immediately at the time it was registered. + static void CountRegisterSuccess( + SyncPeriodicity periodicity, + RegistrationCouldFire could_fire, + RegistrationIsDuplicate registration_is_duplicate); + + // Records the status of a failed sync registration. + static void CountRegisterFailure(SyncPeriodicity periodicity, + BackgroundSyncStatus status); // Records the result of trying to unregister a sync. static void CountUnregister(SyncPeriodicity periodicity, |