metrics: Do not gather perf data until logged in

PerfProvider should not be gathering any perf data (nor counting down
the time until the next collection) if the user is not logged in.

BUG=chromium:375001
TEST=See bug report for test instructions: http://crbug.com/375001

Signed-off-by: Simon Que <sque@chromium.org>

R=asharif@chromium.org, isherman@chromium.org, rapati@chromium.org

Review URL: https://codereview.chromium.org/294873003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273493 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 99 insertions, 7 deletions

diff --git a/chrome/browser/metrics/chromeos_metrics_provider_unittest.cc b/chrome/browser/metrics/chromeos_metrics_provider_unittest.cc
index 4ed21a0..c6a7a8d 100644
--- a/chrome/browser/metrics/chromeos_metrics_provider_unittest.cc
+++ b/chrome/browser/metrics/chromeos_metrics_provider_unittest.cc
@@ -18,6 +18,7 @@
 #include "chromeos/dbus/fake_bluetooth_gatt_service_client.h"
 #include "chromeos/dbus/fake_bluetooth_input_client.h"
 #include "chromeos/dbus/fake_dbus_thread_manager.h"
+#include "chromeos/login/login_state.h"
 #include "components/metrics/proto/system_profile.pb.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
@@ -74,6 +75,9 @@ class ChromeOSMetricsProviderTest : public testing::Test {
         DBusThreadManager::Get()->GetBluetoothAdapterClient());
     fake_bluetooth_device_client_ = static_cast<FakeBluetoothDeviceClient*>(
         DBusThreadManager::Get()->GetBluetoothDeviceClient());
+
+    // Initialize the login state trackers.
+    chromeos::LoginState::Initialize();
   }
 
   virtual void TearDown() OVERRIDE { DBusThreadManager::Shutdown(); }
diff --git a/chrome/browser/metrics/metrics_log_unittest.cc b/chrome/browser/metrics/metrics_log_unittest.cc
index 410329f..5a536ef 100644
--- a/chrome/browser/metrics/metrics_log_unittest.cc
+++ b/chrome/browser/metrics/metrics_log_unittest.cc
@@ -36,6 +36,10 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
+#if defined(OS_CHROMEOS)
+#include "chromeos/login/login_state.h"
+#endif  // defined(OS_CHROMEOS)
+
 using base::TimeDelta;
 using metrics::ProfilerEventProto;
 using tracked_objects::ProcessDataSnapshot;
@@ -108,7 +112,15 @@ class TestMetricsLog : public MetricsLog {
 
 class MetricsLogTest : public testing::Test {
  public:
-  MetricsLogTest() { MetricsService::RegisterPrefs(prefs_.registry()); }
+  MetricsLogTest() {
+    MetricsService::RegisterPrefs(prefs_.registry());
+#if defined(OS_CHROMEOS)
+    // TODO(blundell): Remove this code once MetricsService no longer creates
+    // ChromeOSMetricsProvider. Also remove the #include of login_state.h
+    // (http://crbug.com/375776)
+    chromeos::LoginState::Initialize();
+#endif  // defined(OS_CHROMEOS)
+  }
 
  protected:
   // Check that the values in |system_values| correspond to the test data
diff --git a/chrome/browser/metrics/metrics_service_unittest.cc b/chrome/browser/metrics/metrics_service_unittest.cc
index aebb05a..da0e32d 100644
--- a/chrome/browser/metrics/metrics_service_unittest.cc
+++ b/chrome/browser/metrics/metrics_service_unittest.cc
@@ -21,6 +21,10 @@
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if defined(OS_CHROMEOS)
+#include "chromeos/login/login_state.h"
+#endif  // defined(OS_CHROMEOS)
+
 namespace {
 
 using metrics::MetricsLogManager;
@@ -65,6 +69,12 @@ class MetricsServiceTest : public testing::Test {
         GetLocalState(),
         base::Bind(&MetricsServiceTest::is_metrics_reporting_enabled,
                    base::Unretained(this)));
+#if defined(OS_CHROMEOS)
+    // TODO(blundell): Remove this code once MetricsService no longer creates
+    // ChromeOSMetricsProvider. Also remove the #include of login_state.h.
+    // (http://crbug.com/375776)
+    chromeos::LoginState::Initialize();
+#endif  // defined(OS_CHROMEOS)
   }
 
   virtual ~MetricsServiceTest() {
diff --git a/chrome/browser/metrics/perf_provider_chromeos.cc b/chrome/browser/metrics/perf_provider_chromeos.cc
index 95ff150..efa0d00 100644
--- a/chrome/browser/metrics/perf_provider_chromeos.cc
+++ b/chrome/browser/metrics/perf_provider_chromeos.cc
@@ -75,6 +75,14 @@ void AddToPerfHistogram(GetPerfDataOutcome outcome) {
                             NUM_OUTCOMES);
 }
 
+// Returns true if a normal user is logged in. Returns false if logged in as an
+// guest or as a kiosk app.
+bool IsNormalUserLoggedIn() {
+  chromeos::LoginState* login_state = chromeos::LoginState::Get();
+  return (login_state->IsUserLoggedIn() && !login_state->IsGuestUser() &&
+          !login_state->IsKioskApp());
+}
+
 }  // namespace
 
 
@@ -110,14 +118,22 @@ class WindowedIncognitoObserver : public chrome::BrowserListObserver {
 };
 
 PerfProvider::PerfProvider()
-      : weak_factory_(this) {
-  size_t collection_interval_minutes = base::RandInt(
-      kPerfCommandStartIntervalLowerBoundMinutes,
-      kPerfCommandStartIntervalUpperBoundMinutes);
-  ScheduleCollection(base::TimeDelta::FromMinutes(collection_interval_minutes));
+      : login_observer_(this),
+        weak_factory_(this) {
+  // Register the login observer with LoginState.
+  chromeos::LoginState::Get()->AddObserver(&login_observer_);
+
+  // Check the login state. At the time of writing, this class is instantiated
+  // before login. A subsequent login would activate the profiling. However,
+  // that behavior may change in the future so that the user is already logged
+  // when this class is instantiated. By calling LoggedInStateChanged() here,
+  // PerfProvider will recognize that the system is already logged in.
+  login_observer_.LoggedInStateChanged();
 }
 
-PerfProvider::~PerfProvider() {}
+PerfProvider::~PerfProvider() {
+  chromeos::LoginState::Get()->RemoveObserver(&login_observer_);
+}
 
 bool PerfProvider::GetPerfData(std::vector<PerfDataProto>* perf_data) {
   DCHECK(CalledOnValidThread());
@@ -133,6 +149,28 @@ bool PerfProvider::GetPerfData(std::vector<PerfDataProto>* perf_data) {
   return true;
 }
 
+PerfProvider::LoginObserver::LoginObserver(PerfProvider* perf_provider)
+    : perf_provider_(perf_provider) {}
+
+void PerfProvider::LoginObserver::LoggedInStateChanged() {
+  if (IsNormalUserLoggedIn())
+    perf_provider_->Activate();
+  else
+    perf_provider_->Deactivate();
+}
+
+void PerfProvider::Activate() {
+  size_t collection_interval_minutes = base::RandInt(
+      kPerfCommandStartIntervalLowerBoundMinutes,
+      kPerfCommandStartIntervalUpperBoundMinutes);
+  ScheduleCollection(base::TimeDelta::FromMinutes(collection_interval_minutes));
+}
+
+void PerfProvider::Deactivate() {
+  // Stop the timer, but leave |cached_perf_data_| intact.
+  timer_.Stop();
+}
+
 void PerfProvider::ScheduleCollection(const base::TimeDelta& interval) {
   DCHECK(CalledOnValidThread());
   if (timer_.IsRunning())
diff --git a/chrome/browser/metrics/perf_provider_chromeos.h b/chrome/browser/metrics/perf_provider_chromeos.h
index 2822710..3bd0f46 100644
--- a/chrome/browser/metrics/perf_provider_chromeos.h
+++ b/chrome/browser/metrics/perf_provider_chromeos.h
@@ -12,6 +12,7 @@
 #include "base/threading/non_thread_safe.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
+#include "chromeos/login/login_state.h"
 #include "components/metrics/proto/perf_data.pb.h"
 
 namespace metrics {
@@ -31,6 +32,30 @@ class PerfProvider : public base::NonThreadSafe {
   bool GetPerfData(std::vector<PerfDataProto>* perf_data);
 
  private:
+  // Class that listens for changes to the login state. When a normal user logs
+  // in, it updates PerfProvider to start collecting data.
+  class LoginObserver : public chromeos::LoginState::Observer {
+   public:
+    explicit LoginObserver(PerfProvider* perf_provider);
+
+    // Called when either the login state or the logged in user type changes.
+    // Activates |perf_provider_| to start collecting.
+    virtual void LoggedInStateChanged() OVERRIDE;
+
+   private:
+    // Points to a PerfProvider instance that can be turned on or off based on
+    // the login state.
+    PerfProvider* perf_provider_;
+  };
+
+  // Turns on perf collection. Resets the timer that's used to schedule
+  // collections.
+  void Activate();
+
+  // Turns off perf collection. Does not delete any data that was already
+  // collected and stored in |cached_perf_data_|.
+  void Deactivate();
+
   // Starts an internal timer to start collecting perf data. The timer is set to
   // trigger |interval| after this function call.
   void ScheduleCollection(const base::TimeDelta& interval);
@@ -56,6 +81,9 @@ class PerfProvider : public base::NonThreadSafe {
   // For scheduling collection of perf data.
   base::OneShotTimer<PerfProvider> timer_;
 
+  // For detecting when changes to the login state.
+  LoginObserver login_observer_;
+
   // To pass around the "this" pointer across threads safely.
   base::WeakPtrFactory<PerfProvider> weak_factory_;