Respect toggle for app activity data reporting

This Cl ensures that app activity values will
not be reported if app activity data reporting
toggle is set to false in PerAppTimeLimits policy.

Bug: 1042477
Change-Id: Id400aebd71be7a2eac5c35f1b59c4c70dc7f6f33
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2096914
Commit-Queue: Yilkal Abe <yilkal@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#750217}

chrome/browser/chromeos/child_accounts/time_limits/app_activity_registry.cc

@@ -378,11 +378,27 @@ base::Optional<base::TimeDelta> AppActivityRegistry::GetTimeLimit(
   return limit->daily_limit();
 }
 
+void AppActivityRegistry::SetReportingEnabled(base::Optional<bool> value) {
+  if (value.has_value())
+    activity_reporting_enabled_ = value.value();
+}
+
 AppActivityReportInterface::ReportParams
 AppActivityRegistry::GenerateAppActivityReport(
     enterprise_management::ChildStatusReportRequest* report) {
+  // Calling SaveAppActivity is beneficial even if this method is returning
+  // early due to reporting not being enabled. This is because it helps move the
+  // ActiveTimes information from AppActivityRegistry to the stored pref data
+  // which will then be cleaned in the direct CleanRegistry() call below.
   SaveAppActivity();
 
+  // If app activity reporting is not enabled, simply return.
+  if (!activity_reporting_enabled_) {
+    base::Time timestamp = base::Time::Now();
+    CleanRegistry(timestamp);
+    return AppActivityReportInterface::ReportParams{timestamp, false};
+  }
+
   PrefService* pref_service = profile_->GetPrefs();
   const base::Value* value =
       pref_service->GetList(prefs::kPerAppTimeLimitsAppActivities);
@@ -1025,9 +1041,15 @@ PersistedAppInfo AppActivityRegistry::GetPersistedAppInfoForApp(
   // |timestamp|.
   details.activity.CaptureOngoingActivity(timestamp);
 
+  std::vector<AppActivity::ActiveTime> activity =
+      details.activity.TakeActiveTimes();
+
+  // If reporting is not enabled, don't save unnecessary data.
+  if (!activity_reporting_enabled_)
+    activity.clear();
+
   return PersistedAppInfo(app_id, details.activity.app_state(),
-                          running_active_time,
-                          details.activity.TakeActiveTimes());
+                          running_active_time, std::move(activity));
 }
 
 bool AppActivityRegistry::ShouldCleanUpStoredPref() {

chrome/browser/chromeos/child_accounts/time_limits/app_activity_registry.h

@@ -131,6 +131,9 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
   // Will return nullopt if there is no limit set.
   base::Optional<base::TimeDelta> GetTimeLimit(const AppId& app_id) const;
 
+  // Reporting enablement is set if |enabled| has value.
+  void SetReportingEnabled(base::Optional<bool> enabled);
+
   // Populates |report| with collected app activity. Returns whether any data
   // were reported.
   AppActivityReportInterface::ReportParams GenerateAppActivityReport(
@@ -309,6 +312,9 @@ class AppActivityRegistry : public AppServiceWrapper::EventListener {
 
   // This records the timestamp of the latest set app limit.
   base::Time latest_app_limit_update_;
+
+  // Boolean to capture if app activity data reporting is enabled.
+  bool activity_reporting_enabled_ = true;
 };
 
 }  // namespace app_time

chrome/browser/chromeos/child_accounts/time_limits/app_time_controller.cc

@@ -341,10 +341,12 @@ void AppTimeController::TimeLimitsPolicyUpdated(const std::string& pref_name) {
     LOG(WARNING) << "Invalid PerAppTimeLimits policy.";
     return;
   }
-
   bool updated =
       app_registry_->UpdateAppLimits(policy::AppLimitsFromDict(*policy));
 
+  app_registry_->SetReportingEnabled(
+      policy::ActivityReportingEnabledFromDict(*policy));
+
   base::Optional<base::TimeDelta> new_reset_time =
       policy::ResetTimeFromDict(*policy);
   // TODO(agawronska): Propagate the information about reset time change.

chrome/browser/chromeos/child_accounts/time_limits/app_time_limits_policy_builder.cc

@@ -13,6 +13,7 @@ AppTimeLimitsPolicyBuilder::AppTimeLimitsPolicyBuilder() {
   value_.SetKey(policy::kAppLimitsArray, base::Value(base::Value::Type::LIST));
   value_.SetKey(policy::kResetAtDict,
                 base::Value(base::Value::Type::DICTIONARY));
+  value_.SetBoolKey(policy::kActivityReportingEnabled, true);
 }
 
 AppTimeLimitsPolicyBuilder::~AppTimeLimitsPolicyBuilder() = default;
@@ -33,5 +34,9 @@ void AppTimeLimitsPolicyBuilder::SetResetTime(int hour, int minutes) {
   value_.SetKey(policy::kResetAtDict, policy::ResetTimeToDict(hour, minutes));
 }
 
+void AppTimeLimitsPolicyBuilder::SetAppActivityReportingEnabled(bool enabled) {
+  value_.SetBoolKey(policy::kActivityReportingEnabled, enabled);
+}
+
 }  // namespace app_time
 }  // namespace chromeos

chrome/browser/chromeos/child_accounts/time_limits/app_time_limits_policy_builder.h

@@ -28,6 +28,8 @@ class AppTimeLimitsPolicyBuilder {
   // Sets reset time in the policy.
   void SetResetTime(int hour, int minutes);
 
+  void SetAppActivityReportingEnabled(bool enabled);
+
   const base::Value& value() const { return value_; }
 
  private:

chrome/browser/chromeos/child_accounts/time_limits/app_time_policy_helpers.cc

@@ -28,6 +28,7 @@ const char kLastUpdatedString[] = "last_updated_millis";
 const char kResetAtDict[] = "reset_at";
 const char kHourInt[] = "hour";
 const char kMinInt[] = "minute";
+const char kActivityReportingEnabled[] = "activity_reporting_enabled";
 
 apps::mojom::AppType PolicyStringToAppType(const std::string& app_type) {
   if (app_type == "ARC")
@@ -217,6 +218,12 @@ base::Value ResetTimeToDict(int hour, int minutes) {
   return value;
 }
 
+base::Optional<bool> ActivityReportingEnabledFromDict(const base::Value& dict) {
+  if (!dict.is_dict())
+    return base::nullopt;
+  return dict.FindBoolPath(kActivityReportingEnabled);
+}
+
 std::map<AppId, AppLimit> AppLimitsFromDict(const base::Value& dict) {
   std::map<AppId, AppLimit> app_limits;
 

chrome/browser/chromeos/child_accounts/time_limits/app_time_policy_helpers.h

@@ -38,6 +38,7 @@ extern const char kLastUpdatedString[];
 extern const char kResetAtDict[];
 extern const char kHourInt[];
 extern const char kMinInt[];
+extern const char kActivityReportingEnabled[];
 
 // Converts between apps::mojom::AppType and string used by app time limits
 // policies.
@@ -74,6 +75,10 @@ base::Optional<base::TimeDelta> ResetTimeFromDict(const base::Value& dict);
 // Serializes daily limits reset to the dictionary.
 base::Value ResetTimeToDict(int hour, int minutes);
 
+// Deserializes activity reporting enabled boolean from |dict|.
+// Returns value if |dict| contains a valid entry.
+base::Optional<bool> ActivityReportingEnabledFromDict(const base::Value& dict);
+
 // Deserializes app limits data from the |dict|.
 // Will return empty map if |dict| is invalid.
 std::map<AppId, AppLimit> AppLimitsFromDict(const base::Value& dict);

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

@@ -23,11 +23,13 @@
 #include "base/test/scoped_path_override.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
+#include "base/values.h"
 #include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/chromeos/child_accounts/child_user_service.h"
 #include "chrome/browser/chromeos/child_accounts/child_user_service_factory.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_activity_registry.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_time_controller.h"
+#include "chrome/browser/chromeos/child_accounts/time_limits/app_time_limits_policy_builder.h"
 #include "chrome/browser/chromeos/child_accounts/time_limits/app_types.h"
 #include "chrome/browser/chromeos/login/users/mock_user_manager.h"
 #include "chrome/browser/chromeos/ownership/fake_owner_settings_service.h"
@@ -55,6 +57,7 @@
 #include "components/account_id/account_id.h"
 #include "components/policy/proto/device_management_backend.pb.h"
 #include "components/prefs/pref_service.h"
+#include "components/prefs/scoped_user_pref_update.h"
 #include "components/prefs/testing_pref_service.h"
 #include "components/session_manager/core/session_manager.h"
 #include "components/user_manager/scoped_user_manager.h"
@@ -421,6 +424,8 @@ class ChildStatusCollectorTest : public testing::Test {
             time, &profile_pref_service_));
   }
 
+  Profile* testing_profile() { return testing_profile_.get(); }
+
   // Since this is a unit test running in browser_tests we must do additional
   // unit test setup and make a TestingBrowserProcess. Must be first member.
   TestingBrowserProcessInitializer initializer_;
@@ -761,4 +766,41 @@ TEST_F(ChildStatusCollectorTest, ReportingAppActivity) {
   EXPECT_EQ(0, child_status_.app_activity_size());
 }
 
+TEST_F(ChildStatusCollectorTest, ReportingAppActivityNoReport) {
+  // Nothing reported yet.
+  GetStatus();
+  EXPECT_EQ(0, child_status_.app_activity_size());
+  status_collector_->OnSubmittedSuccessfully();
+
+  const chromeos::app_time::AppId app1(apps::mojom::AppType::kWeb, "app1");
+  const chromeos::app_time::AppId app2(apps::mojom::AppType::kExtension,
+                                       "app2");
+  const base::TimeDelta app1_interval = base::TimeDelta::FromMinutes(1);
+  const base::TimeDelta app2_interval = base::TimeDelta::FromMinutes(2);
+
+  SimulateAppActivity(app1, app1_interval);
+  SimulateAppActivity(app2, app2_interval);
+  SimulateAppActivity(app1, app1_interval);
+  SimulateAppActivity(app2, app2_interval);
+  SimulateAppActivity(app1, app1_interval);
+
+  {
+    chromeos::app_time::AppTimeLimitsPolicyBuilder builder;
+    builder.SetAppActivityReportingEnabled(/* enabled */ false);
+    DictionaryPrefUpdate update(testing_profile()->GetPrefs(),
+                                prefs::kPerAppTimeLimitsPolicy);
+    base::Value* value = update.Get();
+    *value = builder.value().Clone();
+  }
+
+  SimulateAppActivity(app1, app1_interval);
+  SimulateAppActivity(app2, app2_interval);
+  SimulateAppActivity(app1, app1_interval);
+  SimulateAppActivity(app2, app2_interval);
+  SimulateAppActivity(app1, app1_interval);
+
+  GetStatus();
+  EXPECT_EQ(0, child_status_.app_activity_size());
+}
+
 }  // namespace policy