Report ARC status asynchronously via DeviceStatusCollector

chrome/browser/chromeos/policy/device_status_collector.cc

 // 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.
 
 #include "chrome/browser/chromeos/policy/device_status_collector.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 30 matching lines @@
 #include "chrome/common/pref_names.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/update_engine_client.h"
 #include "chromeos/disks/disk_mount_manager.h"
 #include "chromeos/network/device_state.h"
 #include "chromeos/network/network_handler.h"
 #include "chromeos/network/network_state.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/settings/cros_settings_names.h"
 #include "chromeos/system/statistics_provider.h"
+#include "components/arc/arc_bridge_service.h"
+#include "components/arc/common/enterprise_reporting.mojom.h"
 #include "components/policy/core/common/cloud/cloud_policy_constants.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
 #include "components/user_manager/user.h"
 #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"
@@ skipping 270 matching lines @@
 
   // Queues an async callback to query CPU temperature information.
   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));
   }
 
+  bool SampleAndroidInfo(
+      const policy::DeviceStatusCollector::AndroidStatusFetcher&
+          android_status_fetcher) {
+    if (android_status_fetcher.is_null()) {

[ljusten (tachyonic), 2016/10/04 08:56:45]

A more elegant approach would be to make android_status_fetcher asynchronous,
i.e. have it take a response callback, so that it matches instance->GetStatus,
and make the default implementation of android_status_fetcher (if no fetcher is
passed into the constructor of DeviceStatusCollector) to be the arc code below.
The  PostTaskAndReplyWithResult and the OnAndroidInfoPairReceived could be moved
to the unit test this way. It would make this class very clean and move details
like the pair<string, string> method to the test.

[phweiss, 2016/10/05 15:34:22]

Done.

+      arc::ArcBridgeService *arc_service = arc::ArcBridgeService::Get();
+      if (arc_service == nullptr) return false;
+      arc::InstanceHolder<arc::mojom::EnterpriseReportingInstance>
+          *instance_holder = arc_service->enterprise_reporting();
+      if (instance_holder == nullptr) return false;
+      arc::mojom::EnterpriseReportingInstance *instance =
+          instance_holder->GetInstanceForMethod("GetStatus", 1);
+      if (instance == nullptr) return false;
+      instance->GetStatus(
+          base::Bind(&GetStatusState::OnAndroidInfoReceived, this));
+      return true;
+    } else {
+      base::PostTaskAndReplyWithResult(
+          content::BrowserThread::GetBlockingPool(), FROM_HERE,
+          android_status_fetcher,
+          base::Bind(&GetStatusState::OnAndroidInfoPairReceived, this));
+      return true;
+    }
+  }
+
  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;
   }
 
+  void OnAndroidInfoPairReceived(std::pair<mojo::String, mojo::String> status) {

[ljusten (tachyonic), 2016/10/04 08:56:45]

Nit: Is it possible to change the signature of mojo::String to const
mojo::String& to prevent copies?

[phweiss, 2016/10/05 15:34:22]

No, mojo can connect C++ and Java, so their interfaces do not have such things.

+    OnAndroidInfoReceived(status.first, status.second);
+  }
+
+  void OnAndroidInfoReceived(mojo::String status,
+                             mojo::String droid_guard_info) {
+    em::AndroidStatus *android_status =
+        session_status_->mutable_android_status();
+    android_status->set_status_payload(status);
+    android_status->set_droid_guard_info(droid_guard_info);
+  }
+
   const scoped_refptr<base::SequencedTaskRunner> task_runner_;
   policy::DeviceStatusCollector::StatusCallback response_;
   std::unique_ptr<em::DeviceStatusReportRequest> device_status_ =
       base::MakeUnique<em::DeviceStatusReportRequest>();
   std::unique_ptr<em::SessionStatusReportRequest> session_status_ =
       base::MakeUnique<em::SessionStatusReportRequest>();
 };
 
 DeviceStatusCollector::DeviceStatusCollector(
     PrefService* local_state,
     chromeos::system::StatisticsProvider* provider,
     const VolumeInfoFetcher& volume_info_fetcher,
     const CPUStatisticsFetcher& cpu_statistics_fetcher,
-    const CPUTempFetcher& cpu_temp_fetcher)
+    const CPUTempFetcher& cpu_temp_fetcher,
+    const AndroidStatusFetcher& android_status_fetcher)
     : max_stored_past_activity_days_(kMaxStoredPastActivityDays),
       max_stored_future_activity_days_(kMaxStoredFutureActivityDays),
       local_state_(local_state),
       last_idle_check_(Time()),
       volume_info_fetcher_(volume_info_fetcher),
       cpu_statistics_fetcher_(cpu_statistics_fetcher),
       cpu_temp_fetcher_(cpu_temp_fetcher),
+      android_status_fetcher_(android_status_fetcher),
       statistics_provider_(provider),
       cros_settings_(chromeos::CrosSettings::Get()),
       task_runner_(nullptr),
       weak_factory_(this) {
   // 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())
@@ skipping 108 matching lines @@
   if (!cros_settings_->GetBoolean(
           chromeos::kReportDeviceHardwareStatus, &report_hardware_status_)) {
     report_hardware_status_ = true;
   }
 
   if (!cros_settings_->GetBoolean(
           chromeos::kReportDeviceSessionStatus, &report_session_status_)) {
     report_session_status_ = true;
   }
 
+  // TODO(phweiss): Create policy to control this, and turn it on by default.
+  report_android_status_ = false;
+
   if (!report_hardware_status_) {
     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.
     SampleResourceUsage();
   }
 
   // Os update status and running kiosk app reporting are disabled by default.
   if (!cros_settings_->GetBoolean(chromeos::kReportOsUpdateStatus,
@@ skipping 550 matching lines @@
   // Wipe pointer if we didn't actually add any data.
   if (!anything_reported)
     state->ResetDeviceStatus();
 }
 
 void DeviceStatusCollector::GetSessionStatus(
     scoped_refptr<GetStatusState> state) {
   em::SessionStatusReportRequest* status = state->session_status();
   bool anything_reported = false;
 
-  if (report_session_status_)
+  if (report_session_status_) {
     anything_reported |= GetAccountStatus(status);
+    if (report_android_status_)
+      anything_reported |= GetAndroidStatus(status, state);
+  }
 
   // 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();
@@ skipping 10 matching lines @@
   if (app_version.empty()) {
     DLOG(ERROR) << "Unable to get version for extension: "
                 << account->kiosk_app_id;
   } else {
     app_status->set_extension_version(app_version);
   }
 
   return true;
 }
 
+bool DeviceStatusCollector::GetAndroidStatus(
+    em::SessionStatusReportRequest* status,
+    scoped_refptr<GetStatusState> state) {
+  return state->SampleAndroidInfo(android_status_fetcher_);
+}
+
 std::string DeviceStatusCollector::GetAppVersion(
     const std::string& kiosk_app_id) {
   Profile* const profile =
       chromeos::ProfileHelper::Get()->GetProfileByUser(
           user_manager::UserManager::Get()->GetActiveUser());
   const extensions::ExtensionRegistry* const registry =
       extensions::ExtensionRegistry::Get(profile);
   const extensions::Extension* const extension = registry->GetExtensionById(
       kiosk_app_id, extensions::ExtensionRegistry::EVERYTHING);
   if (!extension)
     return std::string();
   return extension->VersionString();
 }
 
 void DeviceStatusCollector::OnSubmittedSuccessfully() {
   TrimStoredActivityPeriods(last_reported_day_, duration_for_last_reported_day_,
                             std::numeric_limits<int64_t>::max());
 }
 
 void DeviceStatusCollector::OnOSVersion(const std::string& version) {
   os_version_ = version;
 }
 
 void DeviceStatusCollector::OnOSFirmware(const std::string& version) {
   firmware_version_ = version;
 }
 
 }  // namespace policy