diff options
author | gangwu <gangwu@chromium.org> | 2015-05-15 17:52:32 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-16 00:52:39 +0000 |
commit | 68208a4cb9def359c3054f0769853f31602968bf (patch) | |
tree | 92d62a2065822d553cdce98ebb7131802951ca8d | |
parent | c0ca01a78f0cce5dff227f7aab0b6f0c1755724d (diff) | |
download | chromium_src-68208a4cb9def359c3054f0769853f31602968bf.zip chromium_src-68208a4cb9def359c3054f0769853f31602968bf.tar.gz chromium_src-68208a4cb9def359c3054f0769853f31602968bf.tar.bz2 |
Adding UMA for sync service, to see how many MEMORY_PRESSURE_LEVEL_CRITICAL received.
BUG=488290
Review URL: https://codereview.chromium.org/1136763004
Cr-Commit-Position: refs/heads/master@{#330253}
-rw-r--r-- | chrome/browser/sync/profile_sync_service.cc | 35 | ||||
-rw-r--r-- | chrome/browser/sync/profile_sync_service.h | 11 | ||||
-rw-r--r-- | chrome/browser/sync/profile_sync_service_unittest.cc | 45 | ||||
-rw-r--r-- | components/sync_driver/pref_names.cc | 6 | ||||
-rw-r--r-- | components/sync_driver/pref_names.h | 3 | ||||
-rw-r--r-- | components/sync_driver/sync_prefs.cc | 22 | ||||
-rw-r--r-- | components/sync_driver/sync_prefs.h | 10 | ||||
-rw-r--r-- | tools/metrics/histograms/histograms.xml | 30 |
8 files changed, 162 insertions, 0 deletions
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc index 2ac3dee..5f0bffd 100644 --- a/chrome/browser/sync/profile_sync_service.cc +++ b/chrome/browser/sync/profile_sync_service.cc @@ -365,6 +365,8 @@ void ProfileSyncService::Initialize() { DCHECK(!running_rollback); #endif + memory_pressure_listener_.reset(new base::MemoryPressureListener(base::Bind( + &ProfileSyncService::OnMemoryPressure, weak_factory_.GetWeakPtr()))); startup_controller_->Reset(GetRegisteredDataTypes()); startup_controller_->TryStart(); } @@ -702,6 +704,8 @@ void ProfileSyncService::StartUpSlowBackendComponents( InitializeBackend(ShouldDeleteSyncFolder()); UpdateFirstSyncTimePref(); + + ReportPreviousSessionMemoryWarningCount(); } void ProfileSyncService::OnGetTokenSuccess( @@ -912,6 +916,9 @@ void ProfileSyncService::ShutdownImpl(syncer::ShutdownReason reason) { UpdateAuthErrorState(GoogleServiceAuthError::AuthErrorNone()); NotifyObservers(); + + // Mark this as a clean shutdown(without crash). + sync_prefs_.SetCleanShutdown(true); } void ProfileSyncService::DisableForUser() { @@ -2720,3 +2727,31 @@ void ProfileSyncService::RemoveClientFromServer() const { access_token_, cache_guid, birthday); } } + +void ProfileSyncService::OnMemoryPressure( + base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level) { + if (memory_pressure_level == + base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL) { + sync_prefs_.SetMemoryPressureWarningCount( + sync_prefs_.GetMemoryPressureWarningCount() + 1); + } +} + +void ProfileSyncService::ReportPreviousSessionMemoryWarningCount() { + int warning_received = sync_prefs_.GetMemoryPressureWarningCount(); + + if (-1 != warning_received) { + // -1 means it is new client. + if (!sync_prefs_.DidSyncShutdownCleanly()) { + UMA_HISTOGRAM_COUNTS("Sync.MemoryPressureWarningBeforeUncleanShutdown", + warning_received); + } else { + UMA_HISTOGRAM_COUNTS("Sync.MemoryPressureWarningBeforeCleanShutdown", + warning_received); + } + } + sync_prefs_.SetMemoryPressureWarningCount(0); + // Will set to true during a clean shutdown, so crash or something else will + // remain this as false. + sync_prefs_.SetCleanShutdown(false); +} diff --git a/chrome/browser/sync/profile_sync_service.h b/chrome/browser/sync/profile_sync_service.h index 1ad2cb7..ff2e2b0 100644 --- a/chrome/browser/sync/profile_sync_service.h +++ b/chrome/browser/sync/profile_sync_service.h @@ -14,6 +14,7 @@ #include "base/files/file_path.h" #include "base/gtest_prod_util.h" #include "base/location.h" +#include "base/memory/memory_pressure_listener.h" #include "base/memory/scoped_ptr.h" #include "base/memory/scoped_vector.h" #include "base/memory/weak_ptr.h" @@ -891,6 +892,13 @@ class ProfileSyncService : public sync_driver::SyncService, // Tell the sync server that this client has disabled sync. void RemoveClientFromServer() const; + // Called when the system is under memory pressure. + void OnMemoryPressure( + base::MemoryPressureListener::MemoryPressureLevel memory_pressure_level); + + // Check if previous shutdown is shutdown cleanly. + void ReportPreviousSessionMemoryWarningCount(); + // Factory used to create various dependent objects. scoped_ptr<ProfileSyncComponentsFactory> factory_; @@ -1070,6 +1078,9 @@ class ProfileSyncService : public sync_driver::SyncService, scoped_ptr<browser_sync::SyncStoppedReporter> sync_stopped_reporter_; + // Listens for the system being under memory pressure. + scoped_ptr<base::MemoryPressureListener> memory_pressure_listener_; + base::WeakPtrFactory<ProfileSyncService> weak_factory_; // We don't use |weak_factory_| for the StartupController because the weak diff --git a/chrome/browser/sync/profile_sync_service_unittest.cc b/chrome/browser/sync/profile_sync_service_unittest.cc index cba1633..f4cb23a 100644 --- a/chrome/browser/sync/profile_sync_service_unittest.cc +++ b/chrome/browser/sync/profile_sync_service_unittest.cc @@ -659,5 +659,50 @@ TEST_F(ProfileSyncServiceTest, NoDisableSyncFlag) { EXPECT_TRUE(ProfileSyncService::IsSyncEnabled()); } +// Test Sync will stop after receive memory pressure +TEST_F(ProfileSyncServiceTest, MemoryPressureRecording) { + CreateService(browser_sync::AUTO_START); + IssueTestTokens(); + ExpectDataTypeManagerCreation(1); + ExpectSyncBackendHostCreation(1); + InitializeForNthSync(); + + EXPECT_TRUE(service()->SyncActive()); + EXPECT_FALSE(profile()->GetPrefs()->GetBoolean( + sync_driver::prefs::kSyncSuppressStart)); + + testing::Mock::VerifyAndClearExpectations(components_factory()); + + sync_driver::SyncPrefs sync_prefs(service()->profile()->GetPrefs()); + + EXPECT_EQ(profile()->GetPrefs()->GetInteger( + sync_driver::prefs::kSyncMemoryPressureWarningCount), + 0); + EXPECT_FALSE(sync_prefs.DidSyncShutdownCleanly()); + + // Simulate memory pressure notification. + base::MemoryPressureListener::NotifyMemoryPressure( + base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL); + base::RunLoop().RunUntilIdle(); + + // Verify memory pressure recorded. + EXPECT_EQ(profile()->GetPrefs()->GetInteger( + sync_driver::prefs::kSyncMemoryPressureWarningCount), + 1); + EXPECT_FALSE(sync_prefs.DidSyncShutdownCleanly()); + + // Simulate memory pressure notification. + base::MemoryPressureListener::NotifyMemoryPressure( + base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_CRITICAL); + base::RunLoop().RunUntilIdle(); + ShutdownAndDeleteService(); + + // Verify memory pressure and shutdown recorded. + EXPECT_EQ(profile()->GetPrefs()->GetInteger( + sync_driver::prefs::kSyncMemoryPressureWarningCount), + 2); + EXPECT_TRUE(sync_prefs.DidSyncShutdownCleanly()); +} + } // namespace } // namespace browser_sync diff --git a/components/sync_driver/pref_names.cc b/components/sync_driver/pref_names.cc index 1a5727a..8caf875 100644 --- a/components/sync_driver/pref_names.cc +++ b/components/sync_driver/pref_names.cc @@ -104,6 +104,12 @@ const char kSyncFirstSyncTime[] = "sync.first_sync_time"; // that we only want to use once. const char kSyncPassphrasePrompted[] = "sync.passphrase_prompted"; +// Stores how many times received MEMORY_PRESSURE_LEVEL_CRITICAL. +const char kSyncMemoryPressureWarningCount[] = "sync.memory_warning_count"; + +// Stores if sync shutdown cleanly. +const char kSyncShutdownCleanly[] = "sync.shutdown_cleanly"; + } // namespace prefs } // namespace sync_driver diff --git a/components/sync_driver/pref_names.h b/components/sync_driver/pref_names.h index 3cbce22..2491390 100644 --- a/components/sync_driver/pref_names.h +++ b/components/sync_driver/pref_names.h @@ -66,6 +66,9 @@ extern const char kSyncFirstSyncTime[]; extern const char kSyncPassphrasePrompted[]; +extern const char kSyncMemoryPressureWarningCount[]; +extern const char kSyncShutdownCleanly[]; + } // namespace prefs } // namespace sync_driver diff --git a/components/sync_driver/sync_prefs.cc b/components/sync_driver/sync_prefs.cc index 82106ee..9f80075 100644 --- a/components/sync_driver/sync_prefs.cc +++ b/components/sync_driver/sync_prefs.cc @@ -83,6 +83,10 @@ void SyncPrefs::RegisterProfilePrefs( registry->RegisterIntegerPref(prefs::kSyncRemainingRollbackTries, 0); registry->RegisterBooleanPref(prefs::kSyncPassphrasePrompted, false); + + registry->RegisterIntegerPref(prefs::kSyncMemoryPressureWarningCount, -1); + + registry->RegisterBooleanPref(prefs::kSyncShutdownCleanly, false); } void SyncPrefs::AddSyncPrefObserver(SyncPrefObserver* sync_pref_observer) { @@ -455,4 +459,22 @@ void SyncPrefs::SetPassphrasePrompted(bool value) { pref_service_->SetBoolean(prefs::kSyncPassphrasePrompted, value); } +int SyncPrefs::GetMemoryPressureWarningCount() const { + return pref_service_->GetInteger(prefs::kSyncMemoryPressureWarningCount); +} + +void SyncPrefs::SetMemoryPressureWarningCount(int value) { + pref_service_->SetInteger(prefs::kSyncMemoryPressureWarningCount, value); +} + +bool SyncPrefs::DidSyncShutdownCleanly() const { + return pref_service_->GetBoolean(prefs::kSyncShutdownCleanly); +} + +void SyncPrefs::SetCleanShutdown(bool value) { + pref_service_->SetBoolean(prefs::kSyncShutdownCleanly, value); +} + } // namespace sync_driver + + diff --git a/components/sync_driver/sync_prefs.h b/components/sync_driver/sync_prefs.h index 5a3abe5..10b3bcd 100644 --- a/components/sync_driver/sync_prefs.h +++ b/components/sync_driver/sync_prefs.h @@ -143,6 +143,16 @@ class SyncPrefs : NON_EXPORTED_BASE(public base::NonThreadSafe), // For testing. void SetManagedForTest(bool is_managed); + // Get/Set number of memory warnings received. + int GetMemoryPressureWarningCount() const; + void SetMemoryPressureWarningCount(int value); + + // Check if the previous shutdown was clean. + bool DidSyncShutdownCleanly() const; + + // Set whetherthe last shutdown was clean. + void SetCleanShutdown(bool value); + private: void RegisterPrefGroups(); diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 5e4cd33..66efb9d 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -41796,6 +41796,36 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="Sync.MemoryPressureWarningBeforeCleanShutdown" units="count"> + <owner>gangwu@chromium.org</owner> + <summary> + Counts the number of times a user's sync service received a + MEMORY_PRESSURE_LEVEL_CRITICAL warning before the sync service shut down + cleanly. The sync service emits this number the next time the user's sync + service is started, which will likely happen the next time the user's + profile is opened after a Chrome restart. This count is emitted once per + user/profile. Things like browser crashes that implicitly bring down all + users' sync services will cause unclean shutdown tags to appear on all open + profiles, meaning that there will be multiple emissions to this histogram as + those profiles are re-opened. + </summary> +</histogram> + +<histogram name="Sync.MemoryPressureWarningBeforeUncleanShutdown" units="count"> + <owner>gangwu@chromium.org</owner> + <summary> + Counts the number of times a user's sync service received a + MEMORY_PRESSURE_LEVEL_CRITICAL warning before the sync service shut down + uncleanly. The sync service emits this number the next time the user's sync + service is started, which will likely happen the next time the user's + profile is opened after a Chrome restart. This count is emitted once per + user/profile. Things like browser crashes that implicitly bring down all + users' sync services will cause unclean shutdown tags to appear on all open + profiles, meaning that there will be multiple emissions to this histogram as + those profiles are re-opened. + </summary> +</histogram> + <histogram name="Sync.NigoriMigrationState" enum="SyncNigoriMigrationState"> <owner>zea@chromium.org</owner> <summary>Breakdown of sync's nigori node keystore migration state.</summary> |