Skip to content

GitLab

T
tangled
Commit 7736b698 authored by jkarlin's avatar jkarlin Committed by Commit bot
Browse files
Options

[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}
parent 881ef3f3
Hide whitespace changes
Inline Side-by-side
Showing with 64 additions and 60 deletions
content/browser/background_sync/background_sync_manager.cc
View file @ 7736b698
...@@ -174,17 +174,9 @@ void BackgroundSyncManager::Register( ...@@ -174,17 +174,9 @@ void BackgroundSyncManager::Register(
const StatusAndRegistrationCallback& callback) { const StatusAndRegistrationCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); 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_) { if (disabled_) {
BackgroundSyncMetrics::CountRegister( BackgroundSyncMetrics::CountRegisterFailure(
options.periodicity, registration_could_fire, options.periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback); PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
return; return;
} }
...@@ -407,17 +399,9 @@ void BackgroundSyncManager::RegisterImpl( ...@@ -407,17 +399,9 @@ void BackgroundSyncManager::RegisterImpl(
const StatusAndRegistrationCallback& callback) { const StatusAndRegistrationCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); 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_) { if (disabled_) {
BackgroundSyncMetrics::CountRegister( BackgroundSyncMetrics::CountRegisterFailure(
options.periodicity, registration_could_fire, options.periodicity, BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback); PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
return; return;
} }
...@@ -430,10 +414,8 @@ void BackgroundSyncManager::RegisterImpl( ...@@ -430,10 +414,8 @@ void BackgroundSyncManager::RegisterImpl(
} }
if (options.tag.length() > kMaxTagLength) { if (options.tag.length() > kMaxTagLength) {
BackgroundSyncMetrics::CountRegister( BackgroundSyncMetrics::CountRegisterFailure(
options.periodicity, registration_could_fire, options.periodicity, BACKGROUND_SYNC_STATUS_NOT_ALLOWED);
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
BACKGROUND_SYNC_STATUS_NOT_ALLOWED);
PostErrorResponse(BACKGROUND_SYNC_STATUS_NOT_ALLOWED, callback); PostErrorResponse(BACKGROUND_SYNC_STATUS_NOT_ALLOWED, callback);
return; return;
} }
...@@ -441,10 +423,8 @@ void BackgroundSyncManager::RegisterImpl( ...@@ -441,10 +423,8 @@ void BackgroundSyncManager::RegisterImpl(
ServiceWorkerRegistration* sw_registration = ServiceWorkerRegistration* sw_registration =
service_worker_context_->GetLiveRegistration(sw_registration_id); service_worker_context_->GetLiveRegistration(sw_registration_id);
if (!sw_registration || !sw_registration->active_version()) { if (!sw_registration || !sw_registration->active_version()) {
BackgroundSyncMetrics::CountRegister( BackgroundSyncMetrics::CountRegisterFailure(
options.periodicity, registration_could_fire, options.periodicity, BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER);
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER);
PostErrorResponse(BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER, callback); PostErrorResponse(BACKGROUND_SYNC_STATUS_NO_SERVICE_WORKER, callback);
return; return;
} }
...@@ -468,11 +448,14 @@ void BackgroundSyncManager::RegisterImpl( ...@@ -468,11 +448,14 @@ void BackgroundSyncManager::RegisterImpl(
BackgroundSyncRegistration* existing_registration = BackgroundSyncRegistration* existing_registration =
existing_registration_ref->value(); existing_registration_ref->value();
// Record the duplicated registration BackgroundSyncMetrics::RegistrationCouldFire registration_could_fire =
BackgroundSyncMetrics::CountRegister( AreOptionConditionsMet(options)
existing_registration->options()->periodicity, registration_could_fire, ? BackgroundSyncMetrics::REGISTRATION_COULD_FIRE
BackgroundSyncMetrics::REGISTRATION_IS_DUPLICATE, : BackgroundSyncMetrics::REGISTRATION_COULD_NOT_FIRE;
BACKGROUND_SYNC_STATUS_OK); BackgroundSyncMetrics::CountRegisterSuccess(
existing_registration->options()->periodicity,
registration_could_fire,
BackgroundSyncMetrics::REGISTRATION_IS_DUPLICATE);
if (existing_registration->IsFiring()) { if (existing_registration->IsFiring()) {
existing_registration->set_sync_state( existing_registration->set_sync_state(
...@@ -646,17 +629,10 @@ void BackgroundSyncManager::RegisterDidStore( ...@@ -646,17 +629,10 @@ void BackgroundSyncManager::RegisterDidStore(
const BackgroundSyncRegistration* new_registration = const BackgroundSyncRegistration* new_registration =
new_registration_ref->value(); 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) { if (status == SERVICE_WORKER_ERROR_NOT_FOUND) {
// The service worker registration is gone. // The service worker registration is gone.
BackgroundSyncMetrics::CountRegister( BackgroundSyncMetrics::CountRegisterFailure(
new_registration->options()->periodicity, registration_could_fire, new_registration->options()->periodicity,
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
BACKGROUND_SYNC_STATUS_STORAGE_ERROR); BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
active_registrations_.erase(sw_registration_id); active_registrations_.erase(sw_registration_id);
PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback); PostErrorResponse(BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback);
...@@ -666,9 +642,8 @@ void BackgroundSyncManager::RegisterDidStore( ...@@ -666,9 +642,8 @@ void BackgroundSyncManager::RegisterDidStore(
if (status != SERVICE_WORKER_OK) { if (status != SERVICE_WORKER_OK) {
LOG(ERROR) << "BackgroundSync failed to store registration due to backend " LOG(ERROR) << "BackgroundSync failed to store registration due to backend "
"failure."; "failure.";
BackgroundSyncMetrics::CountRegister( BackgroundSyncMetrics::CountRegisterFailure(
new_registration->options()->periodicity, registration_could_fire, new_registration->options()->periodicity,
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE,
BACKGROUND_SYNC_STATUS_STORAGE_ERROR); BACKGROUND_SYNC_STATUS_STORAGE_ERROR);
DisableAndClearManager(base::Bind( DisableAndClearManager(base::Bind(
callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR, callback, BACKGROUND_SYNC_STATUS_STORAGE_ERROR,
...@@ -676,10 +651,14 @@ void BackgroundSyncManager::RegisterDidStore( ...@@ -676,10 +651,14 @@ void BackgroundSyncManager::RegisterDidStore(
return; 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, new_registration->options()->periodicity, registration_could_fire,
BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE, BackgroundSyncMetrics::REGISTRATION_IS_NOT_DUPLICATE);
BACKGROUND_SYNC_STATUS_OK);
FireReadyEvents(); FireReadyEvents();
base::ThreadTaskRunnerHandle::Get()->PostTask( base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, FROM_HERE,
......
content/browser/background_sync/background_sync_metrics.cc
View file @ 7736b698
...@@ -32,6 +32,7 @@ ResultPattern EventResultToResultPattern(bool success, ...@@ -32,6 +32,7 @@ ResultPattern EventResultToResultPattern(bool success,
namespace content { namespace content {
// static
void BackgroundSyncMetrics::RecordEventResult(SyncPeriodicity periodicity, void BackgroundSyncMetrics::RecordEventResult(SyncPeriodicity periodicity,
bool success, bool success,
bool finished_in_foreground) { bool finished_in_foreground) {
...@@ -52,6 +53,7 @@ void BackgroundSyncMetrics::RecordEventResult(SyncPeriodicity periodicity, ...@@ -52,6 +53,7 @@ void BackgroundSyncMetrics::RecordEventResult(SyncPeriodicity periodicity,
NOTREACHED(); NOTREACHED();
} }
// static
void BackgroundSyncMetrics::RecordBatchSyncEventComplete( void BackgroundSyncMetrics::RecordBatchSyncEventComplete(
const base::TimeDelta& time, const base::TimeDelta& time,
int number_of_batched_sync_events) { int number_of_batched_sync_events) {
...@@ -64,14 +66,15 @@ void BackgroundSyncMetrics::RecordBatchSyncEventComplete( ...@@ -64,14 +66,15 @@ void BackgroundSyncMetrics::RecordBatchSyncEventComplete(
number_of_batched_sync_events); number_of_batched_sync_events);
} }
void BackgroundSyncMetrics::CountRegister( // static
void BackgroundSyncMetrics::CountRegisterSuccess(
SyncPeriodicity periodicity, SyncPeriodicity periodicity,
RegistrationCouldFire registration_could_fire, RegistrationCouldFire registration_could_fire,
RegistrationIsDuplicate registration_is_duplicate, RegistrationIsDuplicate registration_is_duplicate) {
BackgroundSyncStatus result) {
switch (periodicity) { switch (periodicity) {
case SYNC_ONE_SHOT: 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); BACKGROUND_SYNC_STATUS_MAX + 1);
UMA_HISTOGRAM_BOOLEAN("BackgroundSync.Registration.OneShot.CouldFire", UMA_HISTOGRAM_BOOLEAN("BackgroundSync.Registration.OneShot.CouldFire",
registration_could_fire == REGISTRATION_COULD_FIRE); registration_could_fire == REGISTRATION_COULD_FIRE);
...@@ -80,7 +83,8 @@ void BackgroundSyncMetrics::CountRegister( ...@@ -80,7 +83,8 @@ void BackgroundSyncMetrics::CountRegister(
registration_is_duplicate == REGISTRATION_IS_DUPLICATE); registration_is_duplicate == REGISTRATION_IS_DUPLICATE);
return; return;
case SYNC_PERIODIC: 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); BACKGROUND_SYNC_STATUS_MAX + 1);
UMA_HISTOGRAM_BOOLEAN( UMA_HISTOGRAM_BOOLEAN(
"BackgroundSync.Registration.Periodic.IsDuplicate", "BackgroundSync.Registration.Periodic.IsDuplicate",
...@@ -90,6 +94,23 @@ void BackgroundSyncMetrics::CountRegister( ...@@ -90,6 +94,23 @@ void BackgroundSyncMetrics::CountRegister(
NOTREACHED(); 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, void BackgroundSyncMetrics::CountUnregister(SyncPeriodicity periodicity,
BackgroundSyncStatus result) { BackgroundSyncStatus result) {
switch (periodicity) { switch (periodicity) {
......
content/browser/background_sync/background_sync_metrics.h
View file @ 7736b698
...@@ -38,13 +38,17 @@ class BackgroundSyncMetrics { ...@@ -38,13 +38,17 @@ class BackgroundSyncMetrics {
static void RecordBatchSyncEventComplete(const base::TimeDelta& time, static void RecordBatchSyncEventComplete(const base::TimeDelta& time,
int number_of_batched_sync_events); int number_of_batched_sync_events);
// Records the result of trying to register a sync. |could_fire| indicates // Records the result of successfully registering a sync. |could_fire|
// whether the conditions were sufficient for the sync to fire immediately at // indicates whether the conditions were sufficient for the sync to fire
// the time it was registered. // immediately at the time it was registered.
static void CountRegister(SyncPeriodicity periodicity, static void CountRegisterSuccess(
RegistrationCouldFire could_fire, SyncPeriodicity periodicity,
RegistrationIsDuplicate registration_is_duplicate, RegistrationCouldFire could_fire,
BackgroundSyncStatus result); 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. // Records the result of trying to unregister a sync.
static void CountUnregister(SyncPeriodicity periodicity, static void CountUnregister(SyncPeriodicity periodicity,
......
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