summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorgangwu <gangwu@chromium.org>2015-05-15 17:52:32 -0700
committerCommit bot <commit-bot@chromium.org>2015-05-16 00:52:39 +0000
commit68208a4cb9def359c3054f0769853f31602968bf (patch)
tree92d62a2065822d553cdce98ebb7131802951ca8d
parentc0ca01a78f0cce5dff227f7aab0b6f0c1755724d (diff)
downloadchromium_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.cc35
-rw-r--r--chrome/browser/sync/profile_sync_service.h11
-rw-r--r--chrome/browser/sync/profile_sync_service_unittest.cc45
-rw-r--r--components/sync_driver/pref_names.cc6
-rw-r--r--components/sync_driver/pref_names.h3
-rw-r--r--components/sync_driver/sync_prefs.cc22
-rw-r--r--components/sync_driver/sync_prefs.h10
-rw-r--r--tools/metrics/histograms/histograms.xml30
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>