Do not populate device status during child status reporting

The migration to child status reporting should be finished.
For child user we only rely on child user status proto.

Bug: 827386
Test: ChildStatusCollectorTest + manually
Change-Id: I4d9e724039a3bbc887445c23f1ee318a46602f17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1987829
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732065}

chrome/browser/chromeos/policy/status_collector/child_status_collector.cc

@@ -31,7 +31,6 @@
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
 #include "chrome/browser/chromeos/policy/status_collector/child_activity_storage.h"
-#include "chrome/browser/chromeos/policy/status_collector/device_status_collector.h"
 #include "chrome/browser/chromeos/policy/status_collector/status_collector_state.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/settings/cros_settings.h"
@@ -102,8 +101,8 @@ class ChildStatusCollectorState : public StatusCollectorState {
       const StatusCollectorCallback& response)
       : StatusCollectorState(task_runner, response) {}
 
-  bool FetchAndroidStatus(const DeviceStatusCollector::AndroidStatusFetcher&
-                              android_status_fetcher) {
+  bool FetchAndroidStatus(
+      const StatusCollector::AndroidStatusFetcher& android_status_fetcher) {
     return android_status_fetcher.Run(base::BindRepeating(
         &ChildStatusCollectorState::OnAndroidInfoReceived, this));
   }
@@ -113,13 +112,6 @@ class ChildStatusCollectorState : public StatusCollectorState {
 
   void OnAndroidInfoReceived(const std::string& status,
                              const std::string& droid_guard_info) {
-    // TODO(crbug.com/827386): remove after migration.
-    em::AndroidStatus* const session_android_status =
-        response_params_.session_status->mutable_android_status();
-    session_android_status->set_status_payload(status);
-    session_android_status->set_droid_guard_info(droid_guard_info);
-    // END.
-
     em::AndroidStatus* const child_android_status =
         response_params_.child_status->mutable_android_status();
     child_android_status->set_status_payload(status);
@@ -201,9 +193,6 @@ void ChildStatusCollector::UpdateReportingSettings() {
     return;
   }
 
-  // Activity times.
-  report_activity_times_ = true;
-
   // Settings related.
   report_version_info_ = true;
   cros_settings_->GetBoolean(chromeos::kReportDeviceVersionInfo,
@@ -222,10 +211,6 @@ void ChildStatusCollector::OnUsageTimeStateChange(
 }
 
 void ChildStatusCollector::UpdateChildUsageTime() {
-  if (!report_activity_times_) {
-    return;
-  }
-
   Time now = GetCurrentTime();
   Time reset_time = activity_storage_->GetBeginningOfDay(now);
   if (reset_time > now)
@@ -257,38 +242,6 @@ void ChildStatusCollector::UpdateChildUsageTime() {
   last_active_check_ = now;
 }
 
-bool ChildStatusCollector::GetActivityTimes(
-    em::DeviceStatusReportRequest* status) {
-  UpdateChildUsageTime();
-
-  // Signed-in user is reported in child reporting.
-  std::vector<ActivityStorage::ActivityPeriod> activity_times =
-      activity_storage_->GetStoredActivityPeriods();
-
-  bool anything_reported = false;
-  for (const auto& activity_period : activity_times) {
-    // This is correct even when there are leap seconds, because when a leap
-    // second occurs, two consecutive seconds have the same timestamp.
-    int64_t end_timestamp =
-        activity_period.start_timestamp + Time::kMillisecondsPerDay;
-
-    em::ActiveTimePeriod* active_period = status->add_active_periods();
-    em::TimePeriod* period = active_period->mutable_time_period();
-    period->set_start_timestamp(activity_period.start_timestamp);
-    period->set_end_timestamp(end_timestamp);
-    active_period->set_active_duration(activity_period.activity_milliseconds);
-    // Report user email only if users reporting is turned on.
-    if (!activity_period.user_email.empty())
-      active_period->set_user_email(activity_period.user_email);
-    if (activity_period.start_timestamp >= last_reported_day_) {
-      last_reported_day_ = activity_period.start_timestamp;
-      duration_for_last_reported_day_ = activity_period.activity_milliseconds;
-    }
-    anything_reported = true;
-  }
-  return anything_reported;
-}
-
 bool ChildStatusCollector::GetActivityTimes(
     em::ChildStatusReportRequest* status) {
   UpdateChildUsageTime();
@@ -319,12 +272,6 @@ bool ChildStatusCollector::GetActivityTimes(
   return anything_reported;
 }
 
-bool ChildStatusCollector::GetVersionInfo(
-    em::DeviceStatusReportRequest* status) {
-  status->set_os_version(os_version_);
-  return true;
-}
-
 bool ChildStatusCollector::GetVersionInfo(
     em::ChildStatusReportRequest* status) {
   status->set_os_version(os_version_);
@@ -342,9 +289,7 @@ void ChildStatusCollector::GetStatusAsync(
   scoped_refptr<ChildStatusCollectorState> state(
       new ChildStatusCollectorState(task_runner_, response));
 
-  // Gather status data. The following calls might queue some async queries.
-  GetDeviceStatus(state);
-  GetSessionStatus(state);
+  // Gather status data might queue some async queries.
   FillChildStatusReportRequest(state);
 
   // If there are no outstanding async queries, the destructor of |state| calls
@@ -353,62 +298,6 @@ void ChildStatusCollector::GetStatusAsync(
   // finished.
 }
 
-void ChildStatusCollector::GetDeviceStatus(
-    scoped_refptr<ChildStatusCollectorState> state) {
-  em::DeviceStatusReportRequest* status =
-      state->response_params().device_status.get();
-  bool anything_reported = false;
-
-  if (report_version_info_)
-    anything_reported |= GetVersionInfo(status);
-
-  if (report_activity_times_)
-    anything_reported |= GetActivityTimes(status);
-
-  if (report_boot_mode_) {
-    base::Optional<std::string> boot_mode =
-        StatusCollector::GetBootMode(statistics_provider_);
-    if (boot_mode) {
-      status->set_boot_mode(*boot_mode);
-      anything_reported = true;
-    }
-  }
-
-  // Wipe pointer if we didn't actually add any data.
-  if (!anything_reported)
-    state->response_params().device_status.reset();
-}
-
-bool ChildStatusCollector::GetSessionStatusForUser(
-    scoped_refptr<ChildStatusCollectorState> state,
-    em::SessionStatusReportRequest* status,
-    const user_manager::User* user) {
-  // Child accounts are not local accounts.
-  DCHECK(!user->IsDeviceLocalAccount());
-
-  Profile* const profile =
-      chromeos::ProfileHelper::Get()->GetProfileByUser(user);
-  if (!profile)
-    return false;
-
-  // Time zone.
-  const std::string current_timezone =
-      base::UTF16ToUTF8(chromeos::system::TimezoneSettings::GetInstance()
-                            ->GetCurrentTimezoneID());
-  status->set_time_zone(current_timezone);
-
-  // Android status.
-  const bool report_android_status =
-      profile->GetPrefs()->GetBoolean(prefs::kReportArcStatusEnabled);
-  if (report_android_status)
-    GetAndroidStatus(state);
-
-  status->set_user_dm_token(GetDMTokenForProfile(profile));
-
-  // At least time zone is always reported.
-  return true;
-}
-
 bool ChildStatusCollector::FillUserSpecificFields(
     scoped_refptr<ChildStatusCollectorState> state,
     em::ChildStatusReportRequest* status,
@@ -437,23 +326,6 @@ bool ChildStatusCollector::FillUserSpecificFields(
   return true;
 }
 
-void ChildStatusCollector::GetSessionStatus(
-    scoped_refptr<ChildStatusCollectorState> state) {
-  em::SessionStatusReportRequest* status =
-      state->response_params().session_status.get();
-  bool anything_reported = false;
-
-  user_manager::UserManager* user_manager = user_manager::UserManager::Get();
-  const user_manager::User* const primary_user = user_manager->GetPrimaryUser();
-  DCHECK(primary_user != nullptr);
-
-  anything_reported |= GetSessionStatusForUser(state, status, primary_user);
-
-  // Wipe pointer if we didn't actually add any data.
-  if (!anything_reported)
-    state->response_params().session_status.reset();
-}
-
 bool ChildStatusCollector::GetAndroidStatus(
     const scoped_refptr<ChildStatusCollectorState>& state) {
   return state->FetchAndroidStatus(android_status_fetcher_);
@@ -474,8 +346,7 @@ void ChildStatusCollector::FillChildStatusReportRequest(
   if (report_version_info_)
     anything_reported |= GetVersionInfo(status);
 
-  if (report_activity_times_)
-    anything_reported |= GetActivityTimes(status);
+  anything_reported |= GetActivityTimes(status);
 
   if (report_boot_mode_) {
     base::Optional<std::string> boot_mode =
@@ -498,7 +369,7 @@ void ChildStatusCollector::OnSubmittedSuccessfully() {
 }
 
 bool ChildStatusCollector::ShouldReportActivityTimes() const {
-  return report_activity_times_;
+  return true;
 }
 
 bool ChildStatusCollector::ShouldReportNetworkInterfaces() const {

chrome/browser/chromeos/policy/status_collector/child_status_collector.h

@@ -79,11 +79,6 @@ class ChildStatusCollector : public StatusCollector,
   bool ShouldReportUsers() const override;
   bool ShouldReportHardwareStatus() const override;
 
-  // How often, in seconds, to poll to see if the user is idle.
-  // Note: This in only used in tests and not referenced in .cc. It should
-  // probably be moved.
-  static const unsigned int kIdlePollIntervalSeconds = 30;
-
   // Returns the amount of time the child has used so far today. If there is no
   // user logged in, it returns 0.
   base::TimeDelta GetActiveChildScreenTime();
@@ -120,18 +115,6 @@ class ChildStatusCollector : public StatusCollector,
   // Queues async queries!
   bool GetAndroidStatus(const scoped_refptr<ChildStatusCollectorState>& state);
 
-  // TODO(crbug.com/827386): remove after migration.
-  void GetDeviceStatus(scoped_refptr<ChildStatusCollectorState> state);
-  void GetSessionStatus(scoped_refptr<ChildStatusCollectorState> state);
-  bool GetSessionStatusForUser(
-      scoped_refptr<ChildStatusCollectorState> state,
-      enterprise_management::SessionStatusReportRequest* status,
-      const user_manager::User* user);
-  bool GetActivityTimes(
-      enterprise_management::DeviceStatusReportRequest* status);
-  bool GetVersionInfo(enterprise_management::DeviceStatusReportRequest* status);
-  // END.
-
   // Update the cached values of the reporting settings.
   void UpdateReportingSettings();
 

chrome/browser/chromeos/policy/status_collector/device_status_collector.cc

@@ -397,7 +397,7 @@ void FetchGraphicsStatus(
 }
 
 bool ReadAndroidStatus(
-    const policy::DeviceStatusCollector::AndroidStatusReceiver& receiver) {
+    const policy::StatusCollector::AndroidStatusReceiver& receiver) {
   auto* const arc_service_manager = arc::ArcServiceManager::Get();
   if (!arc_service_manager)
     return false;
@@ -622,8 +622,8 @@ class DeviceStatusCollectorState : public StatusCollectorState {
         base::Bind(&DeviceStatusCollectorState::OnCPUTempInfoReceived, this));
   }
 
-  bool FetchAndroidStatus(const DeviceStatusCollector::AndroidStatusFetcher&
-                              android_status_fetcher) {
+  bool FetchAndroidStatus(
+      const StatusCollector::AndroidStatusFetcher& android_status_fetcher) {
     return android_status_fetcher.Run(
         base::Bind(&DeviceStatusCollectorState::OnAndroidInfoReceived, this));
   }
@@ -1023,7 +1023,7 @@ DeviceStatusCollector::DeviceStatusCollector(
           DeviceStatusCollector::VolumeInfoFetcher(),
           DeviceStatusCollector::CPUStatisticsFetcher(),
           DeviceStatusCollector::CPUTempFetcher(),
-          DeviceStatusCollector::AndroidStatusFetcher(),
+          StatusCollector::AndroidStatusFetcher(),
           DeviceStatusCollector::TpmStatusFetcher(),
           DeviceStatusCollector::EMMCLifetimeFetcher(),
           DeviceStatusCollector::StatefulPartitionInfoFetcher(),

chrome/browser/chromeos/policy/status_collector/device_status_collector.h

@@ -130,17 +130,6 @@ class DeviceStatusCollector : public StatusCollector,
   using CPUTempFetcher =
       base::Callback<std::vector<enterprise_management::CPUTempInfo>()>;
 
-  // Passed into asynchronous mojo interface for communicating with Android.
-  using AndroidStatusReceiver =
-      base::Callback<void(const std::string&, const std::string&)>;
-  // Calls the enterprise reporting mojo interface, passing over the
-  // AndroidStatusReceiver. Returns false if the mojo interface isn't available,
-  // in which case no asynchronous query is emitted and the android status query
-  // fails synchronously. The |AndroidStatusReceiver| is not called in this
-  // case.
-  using AndroidStatusFetcher =
-      base::Callback<bool(const AndroidStatusReceiver&)>;
-
   // Format of the function that asynchronously receives TpmStatusInfo.
   using TpmStatusReceiver = base::OnceCallback<void(const TpmStatusInfo&)>;
   // Gets the TpmStatusInfo and passes it to TpmStatusReceiver.

chrome/browser/chromeos/policy/status_collector/device_status_collector_browsertest.cc

@@ -154,7 +154,7 @@ class TestingDeviceStatusCollectorOptions {
   policy::DeviceStatusCollector::VolumeInfoFetcher volume_info_fetcher;
   policy::DeviceStatusCollector::CPUStatisticsFetcher cpu_fetcher;
   policy::DeviceStatusCollector::CPUTempFetcher cpu_temp_fetcher;
-  policy::DeviceStatusCollector::AndroidStatusFetcher android_status_fetcher;
+  policy::StatusCollector::AndroidStatusFetcher android_status_fetcher;
   policy::DeviceStatusCollector::TpmStatusFetcher tpm_status_fetcher;
   policy::DeviceStatusCollector::EMMCLifetimeFetcher emmc_lifetime_fetcher;
   policy::DeviceStatusCollector::StatefulPartitionInfoFetcher
@@ -313,14 +313,14 @@ std::vector<em::CPUTempInfo> GetFakeCPUTempInfo(
 }
 
 void CallAndroidStatusReceiver(
-    const policy::DeviceStatusCollector::AndroidStatusReceiver& receiver,
+    const policy::StatusCollector::AndroidStatusReceiver& receiver,
     const std::string& status,
     const std::string& droid_guard_info) {
   receiver.Run(status, droid_guard_info);
 }
 
 bool GetEmptyAndroidStatus(
-    const policy::DeviceStatusCollector::AndroidStatusReceiver& receiver) {
+    const policy::StatusCollector::AndroidStatusReceiver& receiver) {
   // Post it to the thread because this call is expected to be asynchronous.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(&CallAndroidStatusReceiver, receiver, "", ""));
@@ -330,7 +330,7 @@ bool GetEmptyAndroidStatus(
 bool GetFakeAndroidStatus(
     const std::string& status,
     const std::string& droid_guard_info,
-    const policy::DeviceStatusCollector::AndroidStatusReceiver& receiver) {
+    const policy::StatusCollector::AndroidStatusReceiver& receiver) {
   // Post it to the thread because this call is expected to be asynchronous.
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::BindOnce(&CallAndroidStatusReceiver, receiver, status,

chrome/browser/chromeos/policy/status_collector/status_collector.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_CHROMEOS_POLICY_STATUS_COLLECTOR_STATUS_COLLECTOR_H_
 
 #include <memory>
+#include <string>
 
 #include "base/callback.h"
 #include "base/optional.h"
@@ -64,6 +65,17 @@ using StatusCollectorCallback =
 // Defines the API for a status collector.
 class StatusCollector {
  public:
+  // Passed into asynchronous mojo interface for communicating with Android.
+  using AndroidStatusReceiver =
+      base::Callback<void(const std::string&, const std::string&)>;
+  // Calls the enterprise reporting mojo interface, passing over the
+  // AndroidStatusReceiver. Returns false if the mojo interface isn't available,
+  // in which case no asynchronous query is emitted and the android status query
+  // fails synchronously. The |AndroidStatusReceiver| is not called in this
+  // case.
+  using AndroidStatusFetcher =
+      base::Callback<bool(const AndroidStatusReceiver&)>;
+
   static void RegisterProfilePrefs(PrefRegistrySimple* registry);
 
   // Simplifies filling the boot mode for any of the relevant status report