Refactored DeviceStatusCollector to enable truely asynchronous status queries

chrome/browser/chromeos/policy/device_status_collector.cc

Index: chrome/browser/chromeos/policy/device_status_collector.cc
diff --git a/chrome/browser/chromeos/policy/device_status_collector.cc b/chrome/browser/chromeos/policy/device_status_collector.cc
index 23946b949163e3ac551d750637074745529a2d58..23e282742e14813c4b5710e590d1c7c0f096362f 100644
--- a/chrome/browser/chromeos/policy/device_status_collector.cc
+++ b/chrome/browser/chromeos/policy/device_status_collector.cc
@@ -18,16 +18,17 @@
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/format_macros.h"
-#include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/memory/ref_counted.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/sys_info.h"
 #include "base/task_runner_util.h"
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "chrome/browser/browser_process.h"
@@ -56,6 +57,7 @@
 #include "components/user_manager/user_manager.h"
 #include "components/user_manager/user_type.h"
 #include "components/version_info/version_info.h"
+#include "content/public/browser/browser_thread.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
 #include "storage/browser/fileapi/external_mount_points.h"
@@ -76,21 +78,8 @@ const unsigned int kMaxStoredPastActivityDays = 30;
 // How many days in the future to store active periods for.
 const unsigned int kMaxStoredFutureActivityDays = 2;
 
-// How often, in seconds, to update the device location.
-const unsigned int kGeolocationPollIntervalSeconds = 30 * 60;
-
-// How often, in seconds, to sample the hardware state.
-const unsigned int kHardwareStatusSampleIntervalSeconds = 120;
-
-// Keys for the geolocation status dictionary in local state.
-const char kLatitude[] = "latitude";
-const char kLongitude[] = "longitude";
-const char kAltitude[] = "altitude";
-const char kAccuracy[] = "accuracy";
-const char kAltitudeAccuracy[] = "altitude_accuracy";
-const char kHeading[] = "heading";
-const char kSpeed[] = "speed";
-const char kTimestamp[] = "timestamp";
+// How often, in seconds, to sample the hardware resource usage.
+const unsigned int kResourceUsageSampleIntervalSeconds = 120;
 
 // The location we read our CPU statistics from.
 const char kProcStat[] = "/proc/stat";
@@ -273,10 +262,100 @@ int ConvertWifiSignalStrength(int signal_strength) {
 
 namespace policy {
 
+// Helper class for state tracking of async status queries.

[Andrew T Wilson (Slow), 2016/09/26 16:33:42]

This does clever stuff in the destructor - you should have class-level
documentation of that behavior and how this class is intended to be used.

[ljusten (tachyonic), 2016/09/27 16:53:54]

Done.

+class GetStatusState : public base::RefCountedThreadSafe<GetStatusState> {
+ public:
+  explicit GetStatusState(
+      const scoped_refptr<base::SequencedTaskRunner> task_runner,
+      const policy::DeviceStatusCollector::StatusCallback& response)
+      : task_runner_(task_runner),
+        response_(response),
+        device_status_(base::MakeUnique<em::DeviceStatusReportRequest>()),

[Andrew T Wilson (Slow), 2016/09/26 16:33:42]

I thought unique_ptr initialized itself to null - do you have to have an
explicit initialization (and nullptr isn't sufficient, you actually need a
MakeUnique)?

Not saying this is wrong - I haven't written enough code with unique_ptr to have
a good grasp of the semantics, just checking.

[Thiemo Nagel, 2016/09/27 16:39:29]

+1 to this.  Also, for new code we prefer in-class initialization over
in-constructor initialization.  But a unique_ptr doesn't require any
initialization at all.

[ljusten (tachyonic), 2016/09/27 16:53:55]

unique_ptr does initialize itself to nullptr, but in this case I want to create
empty status blobs because the logic is easier this way. The logic roughly is:

- Create status blobs
- Call all Get* methods and record whether they actually wrote status
- If none of them wrote any status, delete the blob again

This will create a blob and destroy it again if none of the Get* methods reports
anything, but this is much easier and cleaner to do than the alternative:

- Initialize status blobs with null
- Check whether any of the Get* methods wants to report anything
- If so, create status blob and actually call the Get* methods

+        session_status_(base::MakeUnique<em::SessionStatusReportRequest>()) {}
+
+  inline em::DeviceStatusReportRequest* device_status() {
+    return device_status_.get();
+  }
+
+  inline em::SessionStatusReportRequest* session_status() {
+    return session_status_.get();
+  }
+
+  inline void ResetDeviceStatus() { device_status_.reset(); }
+
+  inline void ResetSessionStatus() { session_status_.reset(); }
+
+  // Queues an async callback to query disk volume information.
+  void SampleVolumeInfo(const policy::DeviceStatusCollector::VolumeInfoFetcher&
+                            volume_info_fetcher) {
+    // Create list of mounted disk volumes to query status.
+    std::vector<storage::MountPoints::MountPointInfo> external_mount_points;
+    storage::ExternalMountPoints::GetSystemInstance()->AddMountPointInfosTo(
+        &external_mount_points);
+
+    std::vector<std::string> mount_points;
+    for (const auto& info : external_mount_points)
+      mount_points.push_back(info.path.value());
+
+    for (const auto& mount_info :
+         chromeos::disks::DiskMountManager::GetInstance()->mount_points()) {
+      // Extract a list of mount points to populate.
+      mount_points.push_back(mount_info.first);
+    }
+
+    // Call out to the blocking pool to sample disk volume info.
+    base::PostTaskAndReplyWithResult(
+        content::BrowserThread::GetBlockingPool(), FROM_HERE,
+        base::Bind(volume_info_fetcher, mount_points),
+        base::Bind(&GetStatusState::OnVolumeInfoReceived, this));
+  }
+
+  // Queues an async callback to query CPU temperator information.

[Andrew T Wilson (Slow), 2016/09/26 16:33:42]

nit: temperator->temperature

[ljusten (tachyonic), 2016/09/27 16:53:55]

Done.

+  void SampleCPUTempInfo(
+      const policy::DeviceStatusCollector::CPUTempFetcher& cpu_temp_fetcher) {
+    // Call out to the blocking pool to sample CPU temp.
+    base::PostTaskAndReplyWithResult(
+        content::BrowserThread::GetBlockingPool(), FROM_HERE, cpu_temp_fetcher,
+        base::Bind(&GetStatusState::OnCPUTempInfoReceived, this));
+  }
+
+ private:
+  friend class RefCountedThreadSafe<GetStatusState>;
+
+  // Posts the response on the UI thread. As long as there is an outstanding
+  // async query, the query holds a reference to us, so the destructor is
+  // not called.
+  ~GetStatusState() {
+    task_runner_->PostTask(FROM_HERE,
+                           base::Bind(response_, base::Passed(&device_status_),
+                                      base::Passed(&session_status_)));
+  }
+
+  void OnVolumeInfoReceived(const std::vector<em::VolumeInfo>& volume_info) {
+    device_status_->clear_volume_info();
+    for (const em::VolumeInfo& info : volume_info)
+      *device_status_->add_volume_info() = info;
+  }
+
+  void OnCPUTempInfoReceived(
+      const std::vector<em::CPUTempInfo>& cpu_temp_info) {
+    if (cpu_temp_info.empty())
+      DLOG(WARNING) << "Unable to read CPU temp information.";
+
+    device_status_->clear_cpu_temp_info();
+    for (const em::CPUTempInfo& info : cpu_temp_info)
+      *device_status_->add_cpu_temp_info() = info;
+  }
+
+  const scoped_refptr<base::SequencedTaskRunner> task_runner_;
+  policy::DeviceStatusCollector::StatusCallback response_;
+  std::unique_ptr<em::DeviceStatusReportRequest> device_status_;
+  std::unique_ptr<em::SessionStatusReportRequest> session_status_;
+};
+
 DeviceStatusCollector::DeviceStatusCollector(
     PrefService* local_state,
     chromeos::system::StatisticsProvider* provider,
-    const LocationUpdateRequester& location_update_requester,
     const VolumeInfoFetcher& volume_info_fetcher,
     const CPUStatisticsFetcher& cpu_statistics_fetcher,
     const CPUTempFetcher& cpu_temp_fetcher)
@@ -289,9 +368,12 @@ DeviceStatusCollector::DeviceStatusCollector(
       cpu_temp_fetcher_(cpu_temp_fetcher),
       statistics_provider_(provider),
       cros_settings_(chromeos::CrosSettings::Get()),
-      location_update_requester_(location_update_requester),
+      task_runner_(nullptr),
       weak_factory_(this) {
-  CHECK(content::BrowserThread::GetCurrentThreadIdentifier(&creation_thread_));
+  // Get the task runner of the current thread, so we can queue status responses
+  // on this thread.
+  CHECK(base::SequencedTaskRunnerHandle::IsSet());
+  task_runner_ = base::SequencedTaskRunnerHandle::Get();
 
   if (volume_info_fetcher_.is_null())
     volume_info_fetcher_ = base::Bind(&GetVolumeInfo);
@@ -305,10 +387,9 @@ DeviceStatusCollector::DeviceStatusCollector(
   idle_poll_timer_.Start(FROM_HERE,
                          TimeDelta::FromSeconds(kIdlePollIntervalSeconds),
                          this, &DeviceStatusCollector::CheckIdleState);
-  hardware_status_sampling_timer_.Start(
-      FROM_HERE,
-      TimeDelta::FromSeconds(kHardwareStatusSampleIntervalSeconds),
-      this, &DeviceStatusCollector::SampleHardwareStatus);
+  resource_usage_sampling_timer_.Start(
+      FROM_HERE, TimeDelta::FromSeconds(kResourceUsageSampleIntervalSeconds),
+      this, &DeviceStatusCollector::SampleResourceUsage);
 
   // Watch for changes to the individual policies that control what the status
   // reports contain.
@@ -321,8 +402,6 @@ DeviceStatusCollector::DeviceStatusCollector(
       chromeos::kReportDeviceActivityTimes, callback);
   boot_mode_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportDeviceBootMode, callback);
-  location_subscription_ = cros_settings_->AddSettingsObserver(
-      chromeos::kReportDeviceLocation, callback);
   network_interfaces_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportDeviceNetworkInterfaces, callback);
   users_subscription_ = cros_settings_->AddSettingsObserver(
@@ -336,27 +415,6 @@ DeviceStatusCollector::DeviceStatusCollector(
   running_kiosk_app_subscription_ = cros_settings_->AddSettingsObserver(
       chromeos::kReportRunningKioskApp, callback);
 
-  // The last known location is persisted in local state. This makes location
-  // information available immediately upon startup and avoids the need to
-  // reacquire the location on every user session change or browser crash.
-  device::Geoposition position;
-  std::string timestamp_str;
-  int64_t timestamp;
-  const base::DictionaryValue* location =
-      local_state_->GetDictionary(prefs::kDeviceLocation);
-  if (location->GetDouble(kLatitude, &position.latitude) &&
-      location->GetDouble(kLongitude, &position.longitude) &&
-      location->GetDouble(kAltitude, &position.altitude) &&
-      location->GetDouble(kAccuracy, &position.accuracy) &&
-      location->GetDouble(kAltitudeAccuracy, &position.altitude_accuracy) &&
-      location->GetDouble(kHeading, &position.heading) &&
-      location->GetDouble(kSpeed, &position.speed) &&
-      location->GetString(kTimestamp, &timestamp_str) &&
-      base::StringToInt64(timestamp_str, &timestamp)) {
-    position.timestamp = Time::FromInternalValue(timestamp);
-    position_ = position;
-  }
-
   // Fetch the current values of the policies.
   UpdateReportingSettings();
 
@@ -383,8 +441,6 @@ DeviceStatusCollector::~DeviceStatusCollector() {
 void DeviceStatusCollector::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(prefs::kDeviceActivityTimes,
                                    new base::DictionaryValue);
-  registry->RegisterDictionaryPref(prefs::kDeviceLocation,
-                                   new base::DictionaryValue);
 }
 
 void DeviceStatusCollector::CheckIdleState() {
@@ -438,27 +494,12 @@ void DeviceStatusCollector::UpdateReportingSettings() {
     report_session_status_ = true;
   }
 
-  // Device location reporting is disabled by default because it is
-  // not launched yet.
-  if (!cros_settings_->GetBoolean(
-      chromeos::kReportDeviceLocation, &report_location_)) {
-    report_location_ = false;
-  }
-
-  if (report_location_) {
-    ScheduleGeolocationUpdateRequest();
-  } else {
-    geolocation_update_timer_.Stop();
-    position_ = device::Geoposition();
-    local_state_->ClearPref(prefs::kDeviceLocation);
-  }
-
   if (!report_hardware_status_) {
-    ClearCachedHardwareStatus();
+    ClearCachedResourceUsage();
   } else if (!already_reporting_hardware_status) {
     // Turning on hardware status reporting - fetch an initial sample
     // immediately instead of waiting for the sampling timer to fire.
-    SampleHardwareStatus();
+    SampleResourceUsage();
   }
 
   // Os update status and running kiosk app reporting are disabled by default.
@@ -536,8 +577,7 @@ void DeviceStatusCollector::AddActivePeriod(Time start, Time end) {
   }
 }
 
-void DeviceStatusCollector::ClearCachedHardwareStatus() {
-  volume_info_.clear();
+void DeviceStatusCollector::ClearCachedResourceUsage() {
   resource_usage_.clear();
   last_cpu_active_ = 0;
   last_cpu_idle_ = 0;
@@ -584,48 +624,21 @@ DeviceStatusCollector::GetAutoLaunchedKioskSessionInfo() {
   return std::unique_ptr<DeviceLocalAccount>();
 }
 
-void DeviceStatusCollector::SampleHardwareStatus() {
+void DeviceStatusCollector::SampleResourceUsage() {
   // Results must be written in the creation thread since that's where they
   // are read from in the Get*StatusAsync methods.
-  CHECK(content::BrowserThread::CurrentlyOn(creation_thread_));
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   // If hardware reporting has been disabled, do nothing here.
   if (!report_hardware_status_)
     return;
 
-  // Create list of mounted disk volumes to query status.
-  std::vector<storage::MountPoints::MountPointInfo> external_mount_points;
-  storage::ExternalMountPoints::GetSystemInstance()->AddMountPointInfosTo(
-      &external_mount_points);
-
-  std::vector<std::string> mount_points;
-  for (const auto& info : external_mount_points)
-    mount_points.push_back(info.path.value());
-
-  for (const auto& mount_info :
-           chromeos::disks::DiskMountManager::GetInstance()->mount_points()) {
-    // Extract a list of mount points to populate.
-    mount_points.push_back(mount_info.first);
-  }
-
-  // Call out to the blocking pool to measure disk, CPU usage and CPU temp.
-  base::PostTaskAndReplyWithResult(
-      content::BrowserThread::GetBlockingPool(),
-      FROM_HERE,
-      base::Bind(volume_info_fetcher_, mount_points),
-      base::Bind(&DeviceStatusCollector::ReceiveVolumeInfo,
-                 weak_factory_.GetWeakPtr()));
-
+  // Call out to the blocking pool to sample CPU stats.
   base::PostTaskAndReplyWithResult(
       content::BrowserThread::GetBlockingPool(), FROM_HERE,
       cpu_statistics_fetcher_,
       base::Bind(&DeviceStatusCollector::ReceiveCPUStatistics,
                  weak_factory_.GetWeakPtr()));
-
-  base::PostTaskAndReplyWithResult(
-      content::BrowserThread::GetBlockingPool(), FROM_HERE, cpu_temp_fetcher_,
-      base::Bind(&DeviceStatusCollector::StoreCPUTempInfo,
-                 weak_factory_.GetWeakPtr()));
 }
 
 void DeviceStatusCollector::ReceiveCPUStatistics(const std::string& stats) {
@@ -681,18 +694,8 @@ void DeviceStatusCollector::ReceiveCPUStatistics(const std::string& stats) {
     resource_usage_.pop_front();
 }
 
-void DeviceStatusCollector::StoreCPUTempInfo(
-    const std::vector<em::CPUTempInfo>& info) {
-  if (info.empty()) {
-    DLOG(WARNING) << "Unable to read CPU temp information.";
-  }
-
-  if (report_hardware_status_)
-    cpu_temp_info_ = info;
-}
-
 bool DeviceStatusCollector::GetActivityTimes(
-    em::DeviceStatusReportRequest* request) {
+    em::DeviceStatusReportRequest* status) {
   DictionaryPrefUpdate update(local_state_, prefs::kDeviceActivityTimes);
   base::DictionaryValue* activity_times = update.Get();
 
@@ -707,7 +710,7 @@ bool DeviceStatusCollector::GetActivityTimes(
       // second occurs, two consecutive seconds have the same timestamp.
       int64_t end_timestamp = start_timestamp + Time::kMillisecondsPerDay;
 
-      em::ActiveTimePeriod* active_period = request->add_active_period();
+      em::ActiveTimePeriod* active_period = status->add_active_period();
       em::TimePeriod* period = active_period->mutable_time_period();
       period->set_start_timestamp(start_timestamp);
       period->set_end_timestamp(end_timestamp);
@@ -725,57 +728,29 @@ bool DeviceStatusCollector::GetActivityTimes(
 }
 
 bool DeviceStatusCollector::GetVersionInfo(
-    em::DeviceStatusReportRequest* request) {
-  request->set_browser_version(version_info::GetVersionNumber());
-  request->set_os_version(os_version_);
-  request->set_firmware_version(firmware_version_);
+    em::DeviceStatusReportRequest* status) {
+  status->set_browser_version(version_info::GetVersionNumber());
+  status->set_os_version(os_version_);
+  status->set_firmware_version(firmware_version_);
   return true;
 }
 
-bool DeviceStatusCollector::GetBootMode(
-    em::DeviceStatusReportRequest* request) {
+bool DeviceStatusCollector::GetBootMode(em::DeviceStatusReportRequest* status) {
   std::string dev_switch_mode;
   bool anything_reported = false;
   if (statistics_provider_->GetMachineStatistic(
           chromeos::system::kDevSwitchBootKey, &dev_switch_mode)) {
     if (dev_switch_mode == chromeos::system::kDevSwitchBootValueDev)
-      request->set_boot_mode("Dev");
+      status->set_boot_mode("Dev");

[Andrew T Wilson (Slow), 2016/09/26 16:33:42]

This is all fine, although note that your patch is significantly larger because
you decided to rename this variable. I might've split these into two patches
(one for a mechanical renaming, one for logic changes).

[ljusten (tachyonic), 2016/09/27 16:53:55]

Acknowledged.

     else if (dev_switch_mode == chromeos::system::kDevSwitchBootValueVerified)
-      request->set_boot_mode("Verified");
+      status->set_boot_mode("Verified");
     anything_reported = true;
   }
   return anything_reported;
 }
 
-bool DeviceStatusCollector::GetLocation(
-    em::DeviceStatusReportRequest* request) {
-  em::DeviceLocation* location = request->mutable_device_location();
-  if (!position_.Validate()) {
-    location->set_error_code(
-        em::DeviceLocation::ERROR_CODE_POSITION_UNAVAILABLE);
-    location->set_error_message(position_.error_message);
-  } else {
-    location->set_latitude(position_.latitude);
-    location->set_longitude(position_.longitude);
-    location->set_accuracy(position_.accuracy);
-    location->set_timestamp(
-        (position_.timestamp - Time::UnixEpoch()).InMilliseconds());
-    // Lowest point on land is at approximately -400 meters.
-    if (position_.altitude > -10000.)
-      location->set_altitude(position_.altitude);
-    if (position_.altitude_accuracy >= 0.)
-      location->set_altitude_accuracy(position_.altitude_accuracy);
-    if (position_.heading >= 0. && position_.heading <= 360)
-      location->set_heading(position_.heading);
-    if (position_.speed >= 0.)
-      location->set_speed(position_.speed);
-    location->set_error_code(em::DeviceLocation::ERROR_CODE_NONE);
-  }
-  return true;
-}
-
 bool DeviceStatusCollector::GetNetworkInterfaces(
-    em::DeviceStatusReportRequest* request) {
+    em::DeviceStatusReportRequest* status) {
   // Maps shill device type strings to proto enum constants.
   static const struct {
     const char* type_string;
@@ -827,7 +802,7 @@ bool DeviceStatusCollector::GetNetworkInterfaces(
     if (type_idx >= arraysize(kDeviceTypeMap))
       continue;
 
-    em::NetworkInterface* interface = request->add_network_interface();
+    em::NetworkInterface* interface = status->add_network_interface();
     interface->set_type(kDeviceTypeMap[type_idx].type_constant);
     if (!(*device)->mac_address().empty())
       interface->set_mac_address((*device)->mac_address());
@@ -867,7 +842,7 @@ bool DeviceStatusCollector::GetNetworkInterfaces(
     }
 
     // Copy fields from NetworkState into the status report.
-    em::NetworkState* proto_state = request->add_network_state();
+    em::NetworkState* proto_state = status->add_network_state();
     proto_state->set_connection_state(connection_state_enum);
     anything_reported = true;
 
@@ -895,7 +870,7 @@ bool DeviceStatusCollector::GetNetworkInterfaces(
   return anything_reported;
 }
 
-bool DeviceStatusCollector::GetUsers(em::DeviceStatusReportRequest* request) {
+bool DeviceStatusCollector::GetUsers(em::DeviceStatusReportRequest* status) {
   const user_manager::UserList& users =
       chromeos::ChromeUserManager::Get()->GetUsers();
 
@@ -905,7 +880,7 @@ bool DeviceStatusCollector::GetUsers(em::DeviceStatusReportRequest* request) {
     if (!user->HasGaiaAccount())
       continue;
 
-    em::DeviceUser* device_user = request->add_user();
+    em::DeviceUser* device_user = status->add_user();
     if (chromeos::ChromeUserManager::Get()->ShouldReportUser(user->email())) {
       device_user->set_type(em::DeviceUser::USER_TYPE_MANAGED);
       device_user->set_email(user->email());
@@ -919,13 +894,17 @@ bool DeviceStatusCollector::GetUsers(em::DeviceStatusReportRequest* request) {
 }
 
 bool DeviceStatusCollector::GetHardwareStatus(
-    em::DeviceStatusReportRequest* status) {
-  // Add volume info.
-  status->clear_volume_info();
-  for (const em::VolumeInfo& info : volume_info_) {
-    *status->add_volume_info() = info;
-  }
+    em::DeviceStatusReportRequest* status,
+    scoped_refptr<GetStatusState> state) {
+  // Sample disk volume info in a background thread.
+  state->SampleVolumeInfo(volume_info_fetcher_);
 
+  // Sample CPU temperature in a background thread.
+  state->SampleCPUTempInfo(cpu_temp_fetcher_);
+
+  // Add CPU utilization and free RAM. Note that these stats are sampled in
+  // regular intervals. Unlike CPU temp and volume info these are not one-time
+  // sampled values, hence the difference in logic.
   status->set_system_ram_total(base::SysInfo::AmountOfPhysicalMemory());
   status->clear_system_ram_free();
   status->clear_cpu_utilization_pct();
@@ -933,12 +912,6 @@ bool DeviceStatusCollector::GetHardwareStatus(
     status->add_cpu_utilization_pct(usage.cpu_usage_percent);
     status->add_system_ram_free(usage.bytes_of_ram_free);
   }
-
-  // Add CPU temp info.
-  status->clear_cpu_temp_info();
-  for (const em::CPUTempInfo& info : cpu_temp_info_) {
-    *status->add_cpu_temp_info() = info;
-  }
   return true;
 }
 
@@ -1000,7 +973,7 @@ bool DeviceStatusCollector::GetRunningKioskApp(
     em::DeviceStatusReportRequest* status) {
   // Must be on creation thread since some stats are written to in that thread
   // and accessing them from another thread would lead to race conditions.
-  CHECK(content::BrowserThread::CurrentlyOn(creation_thread_));
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   std::unique_ptr<const DeviceLocalAccount> account =
       GetAutoLaunchedKioskSessionInfo();
@@ -1029,73 +1002,82 @@ bool DeviceStatusCollector::GetRunningKioskApp(
   return true;
 }
 
-void DeviceStatusCollector::GetDeviceStatusAsync(
-    const DeviceStatusCallback& response) {
+void DeviceStatusCollector::GetDeviceAndSessionStatusAsync(
+    const StatusCallback& response) {
   // Must be on creation thread since some stats are written to in that thread
   // and accessing them from another thread would lead to race conditions.
-  CHECK(content::BrowserThread::CurrentlyOn(creation_thread_));
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // Some of the data we're collecting is gathered in background threads.
+  // This object keeps track of the state of each async request.
+  scoped_refptr<GetStatusState> state(
+      new GetStatusState(task_runner_, response));
+
+  // Gather device status (might queue some async queries)
+  GetDeviceStatus(state);
 
-  std::unique_ptr<em::DeviceStatusReportRequest> status =
-      base::MakeUnique<em::DeviceStatusReportRequest>();
-  bool got_status = false;
+  // Gather device status (might queue some async queries)
+  GetSessionStatus(state);
+
+  // If there are no outstanding async queries, e.g. from GetHardwareStatus(),
+  // the destructor of |state| calls |response|. If there are async queries, the
+  // queries hold references to |state|, so that |state| is only destroyed when
+  // the last async query has finished.
+}
+
+void DeviceStatusCollector::GetDeviceStatus(
+    scoped_refptr<GetStatusState> state) {
+  em::DeviceStatusReportRequest* status = state->device_status();
+  bool anything_reported = false;
 
   if (report_activity_times_)
-    got_status |= GetActivityTimes(status.get());
+    anything_reported |= GetActivityTimes(status);
 
   if (report_version_info_)
-    got_status |= GetVersionInfo(status.get());
+    anything_reported |= GetVersionInfo(status);
 
   if (report_boot_mode_)
-    got_status |= GetBootMode(status.get());
-
-  if (report_location_)
-    got_status |= GetLocation(status.get());
+    anything_reported |= GetBootMode(status);
 
   if (report_network_interfaces_)
-    got_status |= GetNetworkInterfaces(status.get());
+    anything_reported |= GetNetworkInterfaces(status);
 
   if (report_users_)
-    got_status |= GetUsers(status.get());
+    anything_reported |= GetUsers(status);
 
   if (report_hardware_status_)
-    got_status |= GetHardwareStatus(status.get());
+    anything_reported |= GetHardwareStatus(status, state);
 
   if (report_os_update_status_)
-    got_status |= GetOsUpdateStatus(status.get());
+    anything_reported |= GetOsUpdateStatus(status);
 
   if (report_running_kiosk_app_)
-    got_status |= GetRunningKioskApp(status.get());
+    anything_reported |= GetRunningKioskApp(status);
 
   // Wipe pointer if we didn't actually add any data.
-  if (!got_status)
-    status.reset();
-
-  content::BrowserThread::PostTask(creation_thread_, FROM_HERE,
-                                   base::Bind(response, base::Passed(&status)));
+  if (!anything_reported)
+    state->ResetDeviceStatus();
 }
 
-void DeviceStatusCollector::GetDeviceSessionStatusAsync(
-    const DeviceSessionStatusCallback& response) {
-  // Only generate session status reports if session status reporting is
-  // enabled.
-  if (!report_session_status_) {
-    content::BrowserThread::PostTask(creation_thread_, FROM_HERE,
-                                     base::Bind(response, nullptr));
-    return;
-  }
+void DeviceStatusCollector::GetSessionStatus(
+    scoped_refptr<GetStatusState> state) {
+  em::SessionStatusReportRequest* status = state->session_status();
+  bool anything_reported = false;
+
+  if (report_session_status_)
+    anything_reported |= GetAccountStatus(status);
+
+  // Wipe pointer if we didn't actually add any data.
+  if (!anything_reported)
+    state->ResetSessionStatus();
+}
 
+bool DeviceStatusCollector::GetAccountStatus(
+    em::SessionStatusReportRequest* status) {
   std::unique_ptr<const DeviceLocalAccount> account =
       GetAutoLaunchedKioskSessionInfo();
-  // Only generate session status reports if we are in an auto-launched kiosk
-  // session.
-  if (!account) {
-    content::BrowserThread::PostTask(creation_thread_, FROM_HERE,
-                                     base::Bind(response, nullptr));
-    return;
-  }
-
-  std::unique_ptr<em::SessionStatusReportRequest> status =
-      base::MakeUnique<em::SessionStatusReportRequest>();
+  if (!account)
+    return false;
 
   // Get the account ID associated with this user.
   status->set_device_local_account_id(account->account_id);
@@ -1111,8 +1093,7 @@ void DeviceStatusCollector::GetDeviceSessionStatusAsync(
     app_status->set_extension_version(app_version);
   }
 
-  content::BrowserThread::PostTask(creation_thread_, FROM_HERE,
-                                   base::Bind(response, base::Passed(&status)));
+  return true;
 }
 
 std::string DeviceStatusCollector::GetAppVersion(
@@ -1142,68 +1123,4 @@ void DeviceStatusCollector::OnOSFirmware(const std::string& version) {
   firmware_version_ = version;
 }
 
-void DeviceStatusCollector::ScheduleGeolocationUpdateRequest() {
-  if (geolocation_update_timer_.IsRunning() || geolocation_update_in_progress_)
-    return;
-
-  if (position_.Validate()) {
-    TimeDelta elapsed = GetCurrentTime() - position_.timestamp;
-    TimeDelta interval =
-        TimeDelta::FromSeconds(kGeolocationPollIntervalSeconds);
-    if (elapsed <= interval) {
-      geolocation_update_timer_.Start(
-          FROM_HERE,
-          interval - elapsed,
-          this,
-          &DeviceStatusCollector::ScheduleGeolocationUpdateRequest);
-      return;
-    }
-  }
-
-  geolocation_update_in_progress_ = true;
-  if (location_update_requester_.is_null()) {
-    geolocation_subscription_ =
-        device::GeolocationProvider::GetInstance()->AddLocationUpdateCallback(
-            base::Bind(&DeviceStatusCollector::ReceiveGeolocationUpdate,
-                       weak_factory_.GetWeakPtr()),
-            true);
-  } else {
-    location_update_requester_.Run(base::Bind(
-        &DeviceStatusCollector::ReceiveGeolocationUpdate,
-        weak_factory_.GetWeakPtr()));
-  }
-}
-
-void DeviceStatusCollector::ReceiveGeolocationUpdate(
-    const device::Geoposition& position) {
-  geolocation_update_in_progress_ = false;
-
-  // Ignore update if device location reporting has since been disabled.
-  if (!report_location_)
-    return;
-
-  if (position.Validate()) {
-    position_ = position;
-    base::DictionaryValue location;
-    location.SetDouble(kLatitude, position.latitude);
-    location.SetDouble(kLongitude, position.longitude);
-    location.SetDouble(kAltitude, position.altitude);
-    location.SetDouble(kAccuracy, position.accuracy);
-    location.SetDouble(kAltitudeAccuracy, position.altitude_accuracy);
-    location.SetDouble(kHeading, position.heading);
-    location.SetDouble(kSpeed, position.speed);
-    location.SetString(kTimestamp,
-        base::Int64ToString(position.timestamp.ToInternalValue()));
-    local_state_->Set(prefs::kDeviceLocation, location);
-  }
-
-  ScheduleGeolocationUpdateRequest();
-}
-
-void DeviceStatusCollector::ReceiveVolumeInfo(
-    const std::vector<em::VolumeInfo>& info) {
-  if (report_hardware_status_)
-    volume_info_ = info;
-}
-
 }  // namespace policy