Apply individual policies for the various parts of device status reports.

BUG=chromium-os:24308
TEST=Modified DeviceStatusCollectorTest in unit_tests to check that
portions of the status are only included when the appropriate pref is
enabled.


Review URL: http://codereview.chromium.org/9289017

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

17 files changed, 278 insertions, 56 deletions

diff --git a/chrome/app/policy/policy_templates.json b/chrome/app/policy/policy_templates.json
index 4902f99..bea40f1 100644
--- a/chrome/app/policy/policy_templates.json
+++ b/chrome/app/policy/policy_templates.json
@@ -112,7 +112,7 @@
 #   persistent IDs for all fields (but not for groups!) are needed. These are
 #   specified by the 'id' keys of each policy. NEVER CHANGE EXISTING IDs,
 #   because doing so would break the deployed wire format!
-#   For your editing convenience: highest ID currently used: 120
+#   For your editing convenience: highest ID currently used: 121
 #
 # Placeholders:
 #   The following placeholder strings are automatically substituted:
@@ -2308,6 +2308,19 @@
 
       If this setting is set to True, enrolled devices will report time periods when a user is active on the device. If this setting is not set or set to False, device activity times will not be recorded or reported.''',
     },
+    {
+      'name': 'ReportDeviceBootMode',
+      'type': 'main',
+      'supported_on': ['chrome_os:18-'],
+      'device_only': True,
+      'features': {'dynamic_refresh': True},
+      'example_value': False,
+      'id': 121,
+      'caption': '''Report device boot mode''',
+      'desc': '''Report the state of the device's dev switch at boot.
+
+      If the policy is not set, or set to false, the state of the dev switch will not be reported.''',
+    },
   ],
   'messages': {
     # Messages that are not associated to any policies.
diff --git a/chrome/browser/chromeos/cros_settings.cc b/chrome/browser/chromeos/cros_settings.cc
index d2bb5c9..eac7148 100644
--- a/chrome/browser/chromeos/cros_settings.cc
+++ b/chrome/browser/chromeos/cros_settings.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -124,6 +124,14 @@ bool CrosSettings::FindEmailInList(const std::string& path,
 bool CrosSettings::AddSettingsProvider(CrosSettingsProvider* provider) {
   DCHECK(CalledOnValidThread());
   providers_.push_back(provider);
+
+  // Allow the provider to notify this object when settings have changed.
+  // Providers instantiated inside this class will have the same callback
+  // passed to their constructor, but doing it here allows for providers
+  // to be instantiated outside this class.
+  CrosSettingsProvider::NotifyObserversCallback notify_cb(
+      base::Bind(&CrosSettings::FireObservers, base::Unretained(this)));
+  provider->SetNotifyObserversCallback(notify_cb);
   return true;
 }
 
diff --git a/chrome/browser/chromeos/cros_settings_names.cc b/chrome/browser/chromeos/cros_settings_names.cc
index 47544578..72d169a2 100644
--- a/chrome/browser/chromeos/cros_settings_names.cc
+++ b/chrome/browser/chromeos/cros_settings_names.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -29,4 +29,18 @@ const char kStatsReportingPref[] = "cros.metrics.reportingEnabled";
 
 const char kReleaseChannel[] = "cros.system.releaseChannel";
 
+// A boolean pref that indicates whether OS & firmware version info should be
+// reported along with device policy requests.
+const char kReportDeviceVersionInfo[] =
+    "cros.device_status.report_version_info";
+
+// A boolean pref that indicates whether device activity times should be
+// recorded and reported along with device policy requests.
+const char kReportDeviceActivityTimes[] =
+    "cros.device_status.report_activity_times";
+
+// A boolean pref that indicates whether device the state of the dev switch
+// at boot mode should be reported along with device policy requests.
+const char kReportDeviceBootMode[] = "cros.device_status.report_boot_mode";
+
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/cros_settings_names.h b/chrome/browser/chromeos/cros_settings_names.h
index 5206875..a45701d6 100644
--- a/chrome/browser/chromeos/cros_settings_names.h
+++ b/chrome/browser/chromeos/cros_settings_names.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -27,6 +27,10 @@ extern const char kStatsReportingPref[];
 
 extern const char kReleaseChannel[];
 
+extern const char kReportDeviceVersionInfo[];
+extern const char kReportDeviceActivityTimes[];
+extern const char kReportDeviceBootMode[];
+
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_CHROMEOS_CROS_SETTINGS_NAMES_H_
diff --git a/chrome/browser/chromeos/cros_settings_provider.cc b/chrome/browser/chromeos/cros_settings_provider.cc
index ca6fa8d..a0d9b98 100644
--- a/chrome/browser/chromeos/cros_settings_provider.cc
+++ b/chrome/browser/chromeos/cros_settings_provider.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -34,7 +34,13 @@ void CrosSettingsProvider::Set(const std::string& path,
 }
 
 void CrosSettingsProvider::NotifyObservers(const std::string& path) {
-  notify_cb_.Run(path);
+  if (!notify_cb_.is_null())
+    notify_cb_.Run(path);
+}
+
+void CrosSettingsProvider::SetNotifyObserversCallback(
+    const NotifyObserversCallback& notify_cb) {
+  notify_cb_ = notify_cb;
 }
 
 };  // namespace chromeos
diff --git a/chrome/browser/chromeos/cros_settings_provider.h b/chrome/browser/chromeos/cros_settings_provider.h
index 6770377..16bf27f 100644
--- a/chrome/browser/chromeos/cros_settings_provider.h
+++ b/chrome/browser/chromeos/cros_settings_provider.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -45,6 +45,8 @@ class CrosSettingsProvider {
   // Reloads the caches if the provider has any.
   virtual void Reload() = 0;
 
+  void SetNotifyObserversCallback(const NotifyObserversCallback& notify_cb);
+
  protected:
   // Notifies the observers about a setting change.
   void NotifyObservers(const std::string& path);
diff --git a/chrome/browser/chromeos/device_settings_provider.cc b/chrome/browser/chromeos/device_settings_provider.cc
index 51d6dc6..1d1b892 100644
--- a/chrome/browser/chromeos/device_settings_provider.cc
+++ b/chrome/browser/chromeos/device_settings_provider.cc
@@ -41,7 +41,10 @@ const char* kBooleanSettings[] = {
   kAccountsPrefAllowGuest,
   kAccountsPrefShowUserNamesOnSignIn,
   kSignedDataRoamingEnabled,
-  kStatsReportingPref
+  kStatsReportingPref,
+  kReportDeviceVersionInfo,
+  kReportDeviceActivityTimes,
+  kReportDeviceBootMode
 };
 
 const char* kStringSettings[] = {
@@ -281,6 +284,9 @@ void DeviceSettingsProvider::SetInPolicy() {
         whitelist_proto->add_user_whitelist(email.c_str());
     }
   } else {
+    // kReportDeviceVersionInfo, kReportDeviceActivityTimes, and
+    // kReportDeviceBootMode do not support being set in the policy, since
+    // they are not intended to be user-controlled.
     NOTREACHED();
   }
   data.set_policy_value(pol.SerializeAsString());
@@ -403,6 +409,21 @@ void DeviceSettingsProvider::UpdateValuesCache() {
   }
   new_values_cache.SetValue(kAccountsPrefUsers, list);
 
+  if (pol.has_device_reporting()) {
+    if (pol.device_reporting().has_report_version_info()) {
+      new_values_cache.SetBoolean(kReportDeviceVersionInfo,
+          pol.device_reporting().report_version_info());
+    }
+    if (pol.device_reporting().has_report_activity_times()) {
+      new_values_cache.SetBoolean(kReportDeviceActivityTimes,
+          pol.device_reporting().report_activity_times());
+    }
+    if (pol.device_reporting().has_report_boot_mode()) {
+      new_values_cache.SetBoolean(kReportDeviceBootMode,
+          pol.device_reporting().report_boot_mode());
+    }
+  }
+
   // Collect all notifications but send them only after we have swapped the
   // cache so that if somebody actually reads the cache will be already valid.
   std::vector<std::string> notifications;
diff --git a/chrome/browser/chromeos/stub_cros_settings_provider.cc b/chrome/browser/chromeos/stub_cros_settings_provider.cc
index b4690e7..2ae3a3d 100644
--- a/chrome/browser/chromeos/stub_cros_settings_provider.cc
+++ b/chrome/browser/chromeos/stub_cros_settings_provider.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -21,6 +21,9 @@ const char* kHandledSettings[] = {
   kAccountsPrefUsers,
   kDeviceOwner,
   kReleaseChannel,
+  kReportDeviceVersionInfo,
+  kReportDeviceActivityTimes,
+  kReportDeviceBootMode,
   kSettingProxyEverywhere,
   kSignedDataRoamingEnabled,
   kStatsReportingPref
@@ -34,6 +37,11 @@ StubCrosSettingsProvider::StubCrosSettingsProvider(
   SetDefaults();
 }
 
+StubCrosSettingsProvider::StubCrosSettingsProvider()
+  : CrosSettingsProvider(CrosSettingsProvider::NotifyObserversCallback()) {
+  SetDefaults();
+}
+
 StubCrosSettingsProvider::~StubCrosSettingsProvider() {
 }
 
diff --git a/chrome/browser/chromeos/stub_cros_settings_provider.h b/chrome/browser/chromeos/stub_cros_settings_provider.h
index 4152851..b0c652f 100644
--- a/chrome/browser/chromeos/stub_cros_settings_provider.h
+++ b/chrome/browser/chromeos/stub_cros_settings_provider.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -19,6 +19,7 @@ class CrosSettings;
 class StubCrosSettingsProvider : public CrosSettingsProvider {
  public:
   explicit StubCrosSettingsProvider(const NotifyObserversCallback& notify_cb);
+  StubCrosSettingsProvider();
   virtual ~StubCrosSettingsProvider();
 
   // CrosSettingsProvider implementation.
diff --git a/chrome/browser/policy/configuration_policy_handler_list.cc b/chrome/browser/policy/configuration_policy_handler_list.cc
index d7287b4..c001837 100644
--- a/chrome/browser/policy/configuration_policy_handler_list.cc
+++ b/chrome/browser/policy/configuration_policy_handler_list.cc
@@ -296,12 +296,6 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
   { key::kChromeOsReleaseChannel,
     prefs::kChromeOsReleaseChannel,
     Value::TYPE_STRING },
-  { key::kReportDeviceVersionInfo,
-    prefs::kReportDeviceVersionInfo,
-    Value::TYPE_BOOLEAN },
-  { key::kReportDeviceActivityTimes,
-    prefs::kReportDeviceActivityTimes,
-    Value::TYPE_BOOLEAN },
 #endif  // defined(OS_CHROMEOS)
 };
 
diff --git a/chrome/browser/policy/device_policy_cache.cc b/chrome/browser/policy/device_policy_cache.cc
index 7424a37..1a4ea93 100644
--- a/chrome/browser/policy/device_policy_cache.cc
+++ b/chrome/browser/policy/device_policy_cache.cc
@@ -368,24 +368,6 @@ void DevicePolicyCache::DecodeDevicePolicy(
                   POLICY_SCOPE_MACHINE,
                   Value::CreateStringValue(config));
   }
-
-  if (policy.has_device_reporting()) {
-    if (policy.device_reporting().has_report_version_info()) {
-      bool enabled = policy.device_reporting().report_version_info();
-      policies->Set(key::kReportDeviceVersionInfo,
-                    POLICY_LEVEL_MANDATORY,
-                    POLICY_SCOPE_MACHINE,
-                    Value::CreateBooleanValue(enabled));
-    }
-    if (policy.device_reporting().has_report_activity_times()) {
-      bool enabled = policy.device_reporting().report_activity_times();
-      policies->Set(key::kReportDeviceActivityTimes,
-                    POLICY_LEVEL_MANDATORY,
-                    POLICY_SCOPE_MACHINE,
-                    Value::CreateBooleanValue(enabled));
-    }
-  }
-
 }
 
 }  // namespace policy
diff --git a/chrome/browser/policy/device_status_collector.cc b/chrome/browser/policy/device_status_collector.cc
index 3d82084..f4fb746 100644
--- a/chrome/browser/policy/device_status_collector.cc
+++ b/chrome/browser/policy/device_status_collector.cc
@@ -7,10 +7,13 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/string_number_conversions.h"
+#include "chrome/browser/chromeos/cros_settings.h"
+#include "chrome/browser/chromeos/cros_settings_names.h"
 #include "chrome/browser/chromeos/system/statistics_provider.h"
 #include "chrome/browser/policy/proto/device_management_backend.pb.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
+#include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/chrome_version_info.h"
 
 using base::Time;
@@ -26,9 +29,6 @@ const unsigned int kIdleStateThresholdSeconds = 300;
 // The maximum number of time periods stored in the local state.
 const unsigned int kMaxStoredActivePeriods = 500;
 
-// Stores the baseline timestamp, to which the active periods are relative.
-const char* const kPrefBaselineTime = "device_status.baseline_timestamp";
-
 // Stores a list of timestamps representing device active periods.
 const char* const kPrefDeviceActivePeriods = "device_status.active_periods";
 
@@ -50,12 +50,27 @@ DeviceStatusCollector::DeviceStatusCollector(
       local_state_(local_state),
       last_idle_check_(Time()),
       last_idle_state_(IDLE_STATE_UNKNOWN),
-      statistics_provider_(provider) {
+      statistics_provider_(provider),
+      report_version_info_(false),
+      report_activity_times_(false),
+      report_boot_mode_(false) {
   timer_.Start(FROM_HERE,
                TimeDelta::FromSeconds(
                    DeviceStatusCollector::kPollIntervalSeconds),
                this, &DeviceStatusCollector::CheckIdleState);
 
+  cros_settings_ = chromeos::CrosSettings::Get();
+
+  // Watch for changes to the individual policies that control what the status
+  // reports contain.
+  cros_settings_->AddSettingsObserver(chromeos::kReportDeviceVersionInfo, this);
+  cros_settings_->AddSettingsObserver(chromeos::kReportDeviceActivityTimes,
+                                      this);
+  cros_settings_->AddSettingsObserver(chromeos::kReportDeviceBootMode, this);
+
+  // Fetch the current values of the policies.
+  UpdateReportingSettings();
+
   // Get the the OS and firmware version info.
   version_loader_.GetVersion(&consumer_,
                              base::Bind(&DeviceStatusCollector::OnOSVersion,
@@ -67,11 +82,15 @@ DeviceStatusCollector::DeviceStatusCollector(
 }
 
 DeviceStatusCollector::~DeviceStatusCollector() {
+  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceVersionInfo,
+                                         this);
+  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceActivityTimes,
+                                         this);
+  cros_settings_->RemoveSettingsObserver(chromeos::kReportDeviceBootMode, this);
 }
 
 // static
 void DeviceStatusCollector::RegisterPrefs(PrefService* local_state) {
-  local_state->RegisterInt64Pref(kPrefBaselineTime, Time::Now().ToTimeT());
   local_state->RegisterListPref(kPrefDeviceActivePeriods, new ListValue);
 }
 
@@ -81,6 +100,24 @@ void DeviceStatusCollector::CheckIdleState() {
                  base::Unretained(this)));
 }
 
+void DeviceStatusCollector::UpdateReportingSettings() {
+  // Attempt to fetch the current value of the reporting settings.
+  // If trusted values are not available, register this function to be called
+  // back when they are available.
+  bool is_trusted = cros_settings_->GetTrusted(
+      chromeos::kReportDeviceVersionInfo,
+      base::Bind(&DeviceStatusCollector::UpdateReportingSettings,
+                 base::Unretained(this)));
+  if (is_trusted) {
+    cros_settings_->GetBoolean(
+        chromeos::kReportDeviceVersionInfo, &report_version_info_);
+    cros_settings_->GetBoolean(
+        chromeos::kReportDeviceActivityTimes, &report_activity_times_);
+    cros_settings_->GetBoolean(
+        chromeos::kReportDeviceBootMode, &report_boot_mode_);
+  }
+}
+
 Time DeviceStatusCollector::GetCurrentTime() {
   return Time::Now();
 }
@@ -118,6 +155,10 @@ void DeviceStatusCollector::AddActivePeriod(Time start, Time end) {
 }
 
 void DeviceStatusCollector::IdleStateCallback(IdleState state) {
+  // Do nothing if device activity reporting is disabled.
+  if (!report_activity_times_)
+    return;
+
   Time now = GetCurrentTime();
 
   if (state == IDLE_STATE_ACTIVE) {
@@ -134,8 +175,8 @@ void DeviceStatusCollector::IdleStateCallback(IdleState state) {
   last_idle_state_ = state;
 }
 
-void DeviceStatusCollector::GetStatus(em::DeviceStatusReportRequest* request) {
-  // Report device active periods.
+void DeviceStatusCollector::GetActivityTimes(
+    em::DeviceStatusReportRequest* request) {
   const ListValue* active_periods =
       local_state_->GetList(kPrefDeviceActivePeriods);
   em::TimePeriod* time_period;
@@ -159,13 +200,18 @@ void DeviceStatusCollector::GetStatus(em::DeviceStatusReportRequest* request) {
   }
   ListPrefUpdate update(local_state_, kPrefDeviceActivePeriods);
   update.Get()->Clear();
+}
 
+void DeviceStatusCollector::GetVersionInfo(
+    em::DeviceStatusReportRequest* request) {
   chrome::VersionInfo version_info;
   request->set_browser_version(version_info.Version());
   request->set_os_version(os_version_);
   request->set_firmware_version(firmware_version_);
+}
 
-  // Report the state of the dev switch at boot.
+void DeviceStatusCollector::GetBootMode(
+    em::DeviceStatusReportRequest* request) {
   std::string dev_switch_mode;
   if (statistics_provider_->GetMachineStatistic(
       "devsw_boot", &dev_switch_mode)) {
@@ -176,6 +222,17 @@ void DeviceStatusCollector::GetStatus(em::DeviceStatusReportRequest* request) {
   }
 }
 
+void DeviceStatusCollector::GetStatus(em::DeviceStatusReportRequest* request) {
+  if (report_activity_times_)
+    GetActivityTimes(request);
+
+  if (report_version_info_)
+    GetVersionInfo(request);
+
+  if (report_boot_mode_)
+    GetBootMode(request);
+}
+
 void DeviceStatusCollector::OnOSVersion(VersionLoader::Handle handle,
                                         std::string version) {
   os_version_ = version;
@@ -186,4 +243,14 @@ void DeviceStatusCollector::OnOSFirmware(VersionLoader::Handle handle,
   firmware_version_ = version;
 }
 
+void DeviceStatusCollector::Observe(
+    int type,
+    const content::NotificationSource& source,
+    const content::NotificationDetails& details) {
+  if (type == chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED)
+    UpdateReportingSettings();
+  else
+    NOTREACHED();
+}
+
 }  // namespace policy
diff --git a/chrome/browser/policy/device_status_collector.h b/chrome/browser/policy/device_status_collector.h
index 4f1c772..1d6d1f0 100644
--- a/chrome/browser/policy/device_status_collector.h
+++ b/chrome/browser/policy/device_status_collector.h
@@ -10,8 +10,10 @@
 #include "base/timer.h"
 #include "chrome/browser/chromeos/version_loader.h"
 #include "chrome/browser/idle.h"
+#include "content/public/browser/notification_observer.h"
 
 namespace chromeos {
+class CrosSettings;
 namespace system {
 class StatisticsProvider;
 }
@@ -26,7 +28,7 @@ class PrefService;
 namespace policy {
 
 // Collects and summarizes the status of an enterprised-managed ChromeOS device.
-class DeviceStatusCollector {
+class DeviceStatusCollector : public content::NotificationObserver {
  public:
   DeviceStatusCollector(PrefService* local_state,
                         chromeos::system::StatisticsProvider* provider);
@@ -61,6 +63,23 @@ class DeviceStatusCollector {
   void OnOSFirmware(chromeos::VersionLoader::Handle handle,
                     std::string version);
 
+  // Helpers for the various portions of the status.
+  void GetActivityTimes(
+      enterprise_management::DeviceStatusReportRequest* request);
+  void GetVersionInfo(
+      enterprise_management::DeviceStatusReportRequest* request);
+  void GetBootMode(
+      enterprise_management::DeviceStatusReportRequest* request);
+
+  // Update the cached values of the reporting settings.
+  void UpdateReportingSettings();
+
+  // content::NotificationObserver interface.
+  virtual void Observe(
+      int type,
+      const content::NotificationSource& source,
+      const content::NotificationDetails& details) OVERRIDE;
+
   // How often to poll to see if the user is idle.
   int poll_interval_seconds_;
 
@@ -82,6 +101,13 @@ class DeviceStatusCollector {
 
   chromeos::system::StatisticsProvider* statistics_provider_;
 
+  chromeos::CrosSettings* cros_settings_;
+
+  // Cached values of the reporting settings from the device policy.
+  bool report_version_info_;
+  bool report_activity_times_;
+  bool report_boot_mode_;
+
   DISALLOW_COPY_AND_ASSIGN(DeviceStatusCollector);
 };
 
diff --git a/chrome/browser/policy/device_status_collector_unittest.cc b/chrome/browser/policy/device_status_collector_unittest.cc
index 1dde8b6..96a77ed 100644
--- a/chrome/browser/policy/device_status_collector_unittest.cc
+++ b/chrome/browser/policy/device_status_collector_unittest.cc
@@ -7,10 +7,15 @@
 #include "base/message_loop.h"
 #include "base/time.h"
 #include "chrome/browser/idle.h"
+#include "chrome/browser/chromeos/cros_settings.h"
+#include "chrome/browser/chromeos/cros_settings_names.h"
+#include "chrome/browser/chromeos/cros_settings_provider.h"
+#include "chrome/browser/chromeos/stub_cros_settings_provider.h"
 #include "chrome/browser/chromeos/system/mock_statistics_provider.h"
 #include "chrome/browser/policy/proto/device_management_backend.pb.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/test/base/testing_pref_service.h"
+#include "content/test/test_browser_thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -91,11 +96,31 @@ using ::testing::SetArgPointee;
 class DeviceStatusCollectorTest : public testing::Test {
  public:
   DeviceStatusCollectorTest()
-    :  prefs_(),
-       status_collector_(&prefs_, &statistics_provider_) {
+    : message_loop_(MessageLoop::TYPE_UI),
+      ui_thread_(content::BrowserThread::UI, &message_loop_),
+      file_thread_(content::BrowserThread::FILE, &message_loop_),
+      status_collector_(&prefs_, &statistics_provider_) {
+
     DeviceStatusCollector::RegisterPrefs(&prefs_);
     EXPECT_CALL(statistics_provider_, GetMachineStatistic(_, NotNull()))
         .WillRepeatedly(Return(false));
+
+    cros_settings_ = chromeos::CrosSettings::Get();
+
+    // Remove the real DeviceSettingsProvider and replace it with a stub.
+    device_settings_provider_ =
+        cros_settings_->GetProvider(chromeos::kReportDeviceVersionInfo);
+    EXPECT_TRUE(device_settings_provider_ != NULL);
+    EXPECT_TRUE(
+        cros_settings_->RemoveSettingsProvider(device_settings_provider_));
+    cros_settings_->AddSettingsProvider(&stub_settings_provider_);
+  }
+
+  ~DeviceStatusCollectorTest() {
+    // Restore the real DeviceSettingsProvider.
+    EXPECT_TRUE(
+      cros_settings_->RemoveSettingsProvider(&stub_settings_provider_));
+    cros_settings_->AddSettingsProvider(device_settings_provider_);
   }
 
  protected:
@@ -105,10 +130,16 @@ class DeviceStatusCollectorTest : public testing::Test {
   }
 
   MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread file_thread_;
+
   TestingPrefService prefs_;
   chromeos::system::MockStatisticsProvider statistics_provider_;
   TestingDeviceStatusCollector status_collector_;
   em::DeviceStatusReportRequest status_;
+  chromeos::CrosSettings* cros_settings_;
+  chromeos::CrosSettingsProvider* device_settings_provider_;
+  chromeos::StubCrosSettingsProvider stub_settings_provider_;
 };
 
 TEST_F(DeviceStatusCollectorTest, AllIdle) {
@@ -117,6 +148,8 @@ TEST_F(DeviceStatusCollectorTest, AllIdle) {
     IDLE_STATE_IDLE,
     IDLE_STATE_IDLE
   };
+  cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
+
   // Test reporting with no data.
   status_collector_.GetStatus(&status_);
   EXPECT_EQ(0, status_.active_time_size());
@@ -142,6 +175,8 @@ TEST_F(DeviceStatusCollectorTest, AllActive) {
     IDLE_STATE_ACTIVE,
     IDLE_STATE_ACTIVE
   };
+  cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
+
   // Test a single active sample.
   status_collector_.Simulate(test_states, 1);
   status_collector_.GetStatus(&status_);
@@ -168,6 +203,7 @@ TEST_F(DeviceStatusCollectorTest, MixedStates) {
     IDLE_STATE_IDLE,
     IDLE_STATE_ACTIVE
   };
+  cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
   status_collector_.Simulate(test_states,
                              sizeof(test_states) / sizeof(IdleState));
   status_collector_.GetStatus(&status_);
@@ -184,6 +220,7 @@ TEST_F(DeviceStatusCollectorTest, StateKeptInPref) {
     IDLE_STATE_IDLE,
     IDLE_STATE_IDLE
   };
+  cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
   status_collector_.Simulate(test_states,
                              sizeof(test_states) / sizeof(IdleState));
 
@@ -209,6 +246,7 @@ TEST_F(DeviceStatusCollectorTest, Times) {
     IDLE_STATE_IDLE,
     IDLE_STATE_IDLE
   };
+  cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
   status_collector_.Simulate(test_states,
                              sizeof(test_states) / sizeof(IdleState));
   status_collector_.GetStatus(&status_);
@@ -224,6 +262,7 @@ TEST_F(DeviceStatusCollectorTest, MaxStoredPeriods) {
   };
   unsigned int max_periods = 10;
 
+  cros_settings_->SetBoolean(chromeos::kReportDeviceActivityTimes, true);
   status_collector_.set_max_stored_active_periods(max_periods);
 
   // Simulate 12 active periods.
@@ -237,12 +276,38 @@ TEST_F(DeviceStatusCollectorTest, MaxStoredPeriods) {
   EXPECT_EQ(static_cast<int>(max_periods), status_.active_time_size());
 }
 
+TEST_F(DeviceStatusCollectorTest, ActivityTimesDisabledByDefault) {
+  // If the pref for collecting device activity times isn't explicitly turned
+  // on, no data on activity times should be reported.
+
+  IdleState test_states[] = {
+    IDLE_STATE_ACTIVE,
+    IDLE_STATE_ACTIVE,
+    IDLE_STATE_ACTIVE
+  };
+  status_collector_.Simulate(test_states,
+                             sizeof(test_states) / sizeof(IdleState));
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ(0, status_.active_time_size());
+  EXPECT_EQ(0, GetActiveMilliseconds(status_));
+}
+
 TEST_F(DeviceStatusCollectorTest, DevSwitchBootMode) {
+  // Test that boot mode data is not reported if the pref is not turned on.
   status_collector_.GetStatus(&status_);
   EXPECT_EQ(false, status_.has_boot_mode());
 
   EXPECT_CALL(statistics_provider_,
               GetMachineStatistic("devsw_boot", NotNull()))
+      .WillRepeatedly(DoAll(SetArgPointee<1>("0"), Return(true)));
+  EXPECT_EQ(false, status_.has_boot_mode());
+
+  // Turn the pref on, and check that the status is reported iff the
+  // statistics provider returns valid data.
+  cros_settings_->SetBoolean(chromeos::kReportDeviceBootMode, true);
+
+  EXPECT_CALL(statistics_provider_,
+              GetMachineStatistic("devsw_boot", NotNull()))
       .WillOnce(DoAll(SetArgPointee<1>("(error)"), Return(true)));
   status_collector_.GetStatus(&status_);
   EXPECT_EQ(false, status_.has_boot_mode());
@@ -266,4 +331,24 @@ TEST_F(DeviceStatusCollectorTest, DevSwitchBootMode) {
   EXPECT_EQ("Dev", status_.boot_mode());
 }
 
+TEST_F(DeviceStatusCollectorTest, VersionInfo) {
+  // When the pref to collect this data is not enabled, expect that none of
+  // the fields are present in the protobuf.
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ(false, status_.has_browser_version());
+  EXPECT_EQ(false, status_.has_os_version());
+  EXPECT_EQ(false, status_.has_firmware_version());
+
+  cros_settings_->SetBoolean(chromeos::kReportDeviceVersionInfo, true);
+  status_collector_.GetStatus(&status_);
+  EXPECT_EQ(true, status_.has_browser_version());
+  EXPECT_EQ(true, status_.has_os_version());
+  EXPECT_EQ(true, status_.has_firmware_version());
+
+  // Check that the browser version is not empty. OS version & firmware
+  // don't have any reasonable values inside the unit test, so those
+  // aren't checked.
+  EXPECT_NE("", status_.browser_version());
+}
+
 }  // namespace policy
diff --git a/chrome/browser/policy/proto/chrome_device_policy.proto b/chrome/browser/policy/proto/chrome_device_policy.proto
index 17a5208..8503717 100644
--- a/chrome/browser/policy/proto/chrome_device_policy.proto
+++ b/chrome/browser/policy/proto/chrome_device_policy.proto
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -89,6 +89,7 @@ message DeviceOpenNetworkConfigurationProto {
 message DeviceReportingProto {
   optional bool report_version_info = 1;
   optional bool report_activity_times = 2;
+  optional bool report_boot_mode = 3;
 }
 
 message ChromeDeviceSettingsProto {
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index b171c53..a868577 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -1598,14 +1598,6 @@ const char kCarrierDealPromoShown[] =
 // auto-enrollment check has been done once and completed. It can be reset to
 // false if the user opts out of auto enrollment.
 const char kShouldAutoEnroll[] = "ShouldAutoEnroll";
-
-// A boolean pref that indicates whether OS & firmware version info should be
-// reported along with device policy requests.
-const char kReportDeviceVersionInfo[] = "device_status.report_version_info";
-
-// A boolean pref that indicates whether device activity times should be
-// recorded and reported along with device policy requests.
-const char kReportDeviceActivityTimes[] = "device_status.report_activity_times";
 #endif
 
 // Whether there is a Flash version installed that supports clearing LSO data.
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 62eba9a..1d0be21 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -615,8 +615,6 @@ extern const char kSignedSettingsCache[];
 extern const char kHardwareKeyboardLayout[];
 extern const char kCarrierDealPromoShown[];
 extern const char kShouldAutoEnroll[];
-extern const char kReportDeviceVersionInfo[];
-extern const char kReportDeviceActivityTimes[];
 #endif
 
 extern const char kClearPluginLSODataEnabled[];