Pass parent access code overrides to UsageTimeLimitProccessor.

Instead of enabling lock screen authentication, store info about
local overrides in a pref and pass it to UsageTimeLimitProccessor
to calculate state.

That way local override is preserved in-between the sessions and
works consistently with remote overrides.

Bug: 911326
Test: UsageTimeLimitProcessorTest
Change-Id: I8135a2093f805188dd75b48d10c5c1a8aac1c711
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1497611
Commit-Queue: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638369}

chrome/browser/chromeos/child_accounts/screen_time_controller.cc

@@ -18,6 +18,7 @@
 #include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service.h"
 #include "chrome/browser/chromeos/child_accounts/consumer_status_reporting_service_factory.h"
 #include "chrome/browser/chromeos/child_accounts/parent_access_code/policy_config_source.h"
+#include "chrome/browser/chromeos/child_accounts/time_limit_override.h"
 #include "chrome/browser/chromeos/login/lock/screen_locker.h"
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/profiles/profile.h"
@@ -54,6 +55,7 @@ constexpr char kScreenStateNextUnlockTime[] = "next_unlock_time";
 void ScreenTimeController::RegisterProfilePrefs(PrefRegistrySimple* registry) {
   registry->RegisterDictionaryPref(prefs::kScreenTimeLastState);
   registry->RegisterDictionaryPref(prefs::kUsageTimeLimit);
+  registry->RegisterDictionaryPref(prefs::kTimeLimitLocalOverride);
 }
 
 ScreenTimeController::ScreenTimeController(content::BrowserContext* context)
@@ -120,7 +122,16 @@ void ScreenTimeController::OnAccessCodeValidation(bool result) {
   if (!session_manager::SessionManager::Get()->IsScreenLocked())
     return;
 
-  UpdateLockScreenState(false /*blocked*/, base::Time());
+  usage_time_limit::TimeLimitOverride local_override(
+      usage_time_limit::TimeLimitOverride::Action::kUnlock, clock_->Now(),
+      base::nullopt);
+  // Replace previous local override stored in pref, because PAC can only be
+  // entered if previous override is not active anymore.
+  pref_service_->Set(prefs::kTimeLimitLocalOverride,
+                     local_override.ToDictionary());
+  pref_service_->CommitPendingWrite();
+
+  CheckTimeLimit("OnAccessCodeValidation");
 }
 
 void ScreenTimeController::SetClocksForTesting(
@@ -154,10 +165,13 @@ void ScreenTimeController::CheckTimeLimit(const std::string& source) {
   base::Optional<usage_time_limit::State> last_state = GetLastStateFromPref();
   const base::DictionaryValue* time_limit =
       pref_service_->GetDictionary(prefs::kUsageTimeLimit);
+  const base::DictionaryValue* local_override =
+      pref_service_->GetDictionary(prefs::kTimeLimitLocalOverride);
 
+  // TODO(agawronska): Usage timestamp should be passed instead of second |now|.
   usage_time_limit::State state = usage_time_limit::GetState(
-      time_limit->CreateDeepCopy(), GetScreenTimeDuration(), now, now,
-      &time_zone, last_state);
+      time_limit->CreateDeepCopy(), local_override, GetScreenTimeDuration(),
+      now, now, &time_zone, last_state);
   SaveCurrentStateToPref(state);
 
   // Show/hide time limits message based on the policy enforcement.
@@ -206,10 +220,12 @@ void ScreenTimeController::CheckTimeLimit(const std::string& source) {
       ScheduleUsageTimeLimitWarning(state);
   }
 
-  base::Time next_get_state_time =
-      std::min(state.next_state_change_time,
-               usage_time_limit::GetExpectedResetTime(
-                   time_limit->CreateDeepCopy(), now, &time_zone));
+  // TODO(agawronska): We are creating UsageTimeLimitProcessor second time in
+  // this method. Could expected reset time be returned as a part of the state?
+  base::Time next_get_state_time = std::min(
+      state.next_state_change_time,
+      usage_time_limit::GetExpectedResetTime(time_limit->CreateDeepCopy(),
+                                             local_override, now, &time_zone));
   if (!next_get_state_time.is_null()) {
     VLOG(1) << "Scheduling state change timer in " << next_get_state_time - now;
     next_state_timer_->Start(
@@ -416,11 +432,13 @@ void ScreenTimeController::UsageTimeLimitWarning() {
       system::TimezoneSettings::GetInstance()->GetTimezone();
   const base::DictionaryValue* time_limit =
       pref_service_->GetDictionary(prefs::kUsageTimeLimit);
+  const base::DictionaryValue* local_override =
+      pref_service_->GetDictionary(prefs::kTimeLimitLocalOverride);
 
   base::Optional<base::TimeDelta> remaining_usage =
-      usage_time_limit::GetRemainingTimeUsage(time_limit->CreateDeepCopy(), now,
-                                              GetScreenTimeDuration(),
-                                              &time_zone);
+      usage_time_limit::GetRemainingTimeUsage(
+          time_limit->CreateDeepCopy(), local_override, now,
+          GetScreenTimeDuration(), &time_zone);
 
   // Remaining time usage can be bigger than |kUsageTimeLimitWarningTime|
   // because it is counted in another class so the timers might be called with

chrome/browser/chromeos/child_accounts/time_limit_test_utils.cc

@@ -162,6 +162,7 @@ void AddOverride(base::DictionaryValue* policy,
         policy->SetKey(usage_time_limit::TimeLimitOverride::kOverridesDictKey,
                        base::Value(base::Value::Type::LIST));
   }
+
   usage_time_limit::TimeLimitOverride new_override(action, created_at,
                                                    base::nullopt);
   overrides->GetList().push_back(new_override.ToDictionary());

chrome/browser/chromeos/child_accounts/usage_time_limit_processor.cc

@@ -76,6 +76,7 @@ class UsageTimeLimitProcessor {
       base::Optional<internal::TimeWindowLimit> time_window_limit,
       base::Optional<internal::TimeUsageLimit> time_usage_limit,
       base::Optional<TimeLimitOverride> time_limit_override,
+      base::Optional<TimeLimitOverride> local_time_limit_override,
       const base::TimeDelta& used_time,
       const base::Time& usage_timestamp,
       const base::Time& current_time,
@@ -192,6 +193,9 @@ class UsageTimeLimitProcessor {
   // The policy override object.
   base::Optional<TimeLimitOverride> time_limit_override_;
 
+  // The local override object.
+  base::Optional<TimeLimitOverride> local_time_limit_override_;
+
   // How long the user has used the device.
   const base::TimeDelta used_time_;
 
@@ -239,6 +243,7 @@ UsageTimeLimitProcessor::UsageTimeLimitProcessor(
     base::Optional<internal::TimeWindowLimit> time_window_limit,
     base::Optional<internal::TimeUsageLimit> time_usage_limit,
     base::Optional<TimeLimitOverride> time_limit_override,
+    base::Optional<TimeLimitOverride> local_time_limit_override,
     const base::TimeDelta& used_time,
     const base::Time& usage_timestamp,
     const base::Time& current_time,
@@ -246,7 +251,6 @@ UsageTimeLimitProcessor::UsageTimeLimitProcessor(
     const base::Optional<State>& previous_state)
     : time_window_limit_(std::move(time_window_limit)),
       time_usage_limit_(std::move(time_usage_limit)),
-      time_limit_override_(std::move(time_limit_override)),
       used_time_(used_time),
       usage_timestamp_(usage_timestamp),
       current_time_(current_time),
@@ -254,6 +258,18 @@ UsageTimeLimitProcessor::UsageTimeLimitProcessor(
       current_weekday_(GetCurrentWeekday()),
       previous_state_(previous_state),
       enabled_time_usage_limit_(GetEnabledTimeUsageLimit()) {
+  // Use local override if it is newer than policy override, otherwise ignore
+  // local override.
+  // Note: |time_limit_override_| needs to be set before calculating
+  // |active_time_window_limit_| and |active_time_usage_limit_|.
+  bool should_use_local_override = local_time_limit_override.has_value() &&
+                                   (!time_limit_override.has_value() ||
+                                    local_time_limit_override->created_at() >
+                                        time_limit_override->created_at());
+  time_limit_override_ = should_use_local_override
+                             ? std::move(local_time_limit_override)
+                             : std::move(time_limit_override);
+
   // This will also set overridden_window_limit_ to true if applicable.
   // TODO: refactor GetActiveTimeWindowLimit to stop updating the state on a
   // getter method.
@@ -645,7 +661,6 @@ bool UsageTimeLimitProcessor::HasActiveOverride() {
   }
 
   bool has_valid_lock_override =
-      time_limit_override_->IsLock() &&
       time_limit_override_->created_at() > last_reset_time &&
       !override_cancelled_by_window_limit;
 
@@ -1181,6 +1196,7 @@ base::Optional<TimeLimitOverride> OverrideFromPolicy(
 }
 
 State GetState(const std::unique_ptr<base::DictionaryValue>& time_limit,
+               const base::Value* local_override,
                const base::TimeDelta& used_time,
                const base::Time& usage_timestamp,
                const base::Time& current_time,
@@ -1192,15 +1208,20 @@ State GetState(const std::unique_ptr<base::DictionaryValue>& time_limit,
       TimeUsageLimitFromPolicy(time_limit);
   base::Optional<TimeLimitOverride> time_limit_override =
       OverrideFromPolicy(time_limit);
+  base::Optional<TimeLimitOverride> local_time_limit_override =
+      TimeLimitOverride::FromDictionary(local_override);
+  // TODO(agawronska): Pass |usage_timestamp| instead of second |current_time|.
   return internal::UsageTimeLimitProcessor(
              std::move(time_window_limit), std::move(time_usage_limit),
-             std::move(time_limit_override), used_time, current_time,
+             std::move(time_limit_override),
+             std::move(local_time_limit_override), used_time, current_time,
              current_time, time_zone, previous_state)
       .GetState();
 }
 
 base::Time GetExpectedResetTime(
     const std::unique_ptr<base::DictionaryValue>& time_limit,
+    const base::Value* local_override,
     const base::Time current_time,
     const icu::TimeZone* const time_zone) {
   base::Optional<internal::TimeWindowLimit> time_window_limit =
@@ -1209,15 +1230,20 @@ base::Time GetExpectedResetTime(
       TimeUsageLimitFromPolicy(time_limit);
   base::Optional<TimeLimitOverride> time_limit_override =
       OverrideFromPolicy(time_limit);
+  base::Optional<TimeLimitOverride> local_time_limit_override =
+      TimeLimitOverride::FromDictionary(local_override);
   return internal::UsageTimeLimitProcessor(
              std::move(time_window_limit), std::move(time_usage_limit),
-             std::move(time_limit_override), base::TimeDelta::FromMinutes(0),
-             base::Time(), current_time, time_zone, base::nullopt)
+             std::move(time_limit_override),
+             std::move(local_time_limit_override),
+             base::TimeDelta::FromMinutes(0), base::Time(), current_time,
+             time_zone, base::nullopt)
       .GetExpectedResetTime();
 }
 
 base::Optional<base::TimeDelta> GetRemainingTimeUsage(
     const std::unique_ptr<base::DictionaryValue>& time_limit,
+    const base::Value* local_override,
     const base::Time current_time,
     const base::TimeDelta& used_time,
     const icu::TimeZone* const time_zone) {
@@ -1227,9 +1253,12 @@ base::Optional<base::TimeDelta> GetRemainingTimeUsage(
       TimeUsageLimitFromPolicy(time_limit);
   base::Optional<TimeLimitOverride> time_limit_override =
       OverrideFromPolicy(time_limit);
+  base::Optional<TimeLimitOverride> local_time_limit_override =
+      TimeLimitOverride::FromDictionary(local_override);
   return internal::UsageTimeLimitProcessor(
              std::move(time_window_limit), std::move(time_usage_limit),
-             std::move(time_limit_override), used_time, base::Time(),
+             std::move(time_limit_override),
+             std::move(local_time_limit_override), used_time, base::Time(),
              current_time, time_zone, base::nullopt)
       .GetRemainingTimeUsage();
 }

chrome/browser/chromeos/child_accounts/usage_time_limit_processor.h

@@ -141,7 +141,17 @@ struct State {
 
 // Returns the current state of the user session with the given usage time limit
 // policy.
+// |time_limit| dictionary with UsageTimeLimit policy data.
+// |local_override| dictionary with data of the last local override (authorized
+//                  by parent access code).
+// |used_time| time used in the current day.
+// |usage_timestamp| when was |used_time| data collected. Usually differs from
+//                   |current_time| by milliseconds.
+// |previous_state| state previously returned by UsageTimeLimitProcessor.
+// TODO(agawronska): Passing unique_ptr by const ref is strange and advised
+// against by style guide. Probably should be refactorred.
 State GetState(const std::unique_ptr<base::DictionaryValue>& time_limit,
+               const base::Value* local_override,
                const base::TimeDelta& used_time,
                const base::Time& usage_timestamp,
                const base::Time& current_time,
@@ -149,14 +159,23 @@ State GetState(const std::unique_ptr<base::DictionaryValue>& time_limit,
                const base::Optional<State>& previous_state);
 
 // Returns the expected time that the used time stored should be reset.
+// |time_limit| dictionary with UsageTimeLimit policy data.
+// |local_override| dictionary with data of the last local override (authorized
+//                  by parent access code).
 base::Time GetExpectedResetTime(
     const std::unique_ptr<base::DictionaryValue>& time_limit,
+    const base::Value* local_override,
     base::Time current_time,
     const icu::TimeZone* const time_zone);
 
 // Returns the remaining time usage if the time usage limit is enabled.
+// |time_limit| dictionary with UsageTimeLimit policy data.
+// |local_override| dictionary with data of the last local override (authorized
+//                  by parent access code).
+// |used_time| time used in the current day.
 base::Optional<base::TimeDelta> GetRemainingTimeUsage(
     const std::unique_ptr<base::DictionaryValue>& time_limit,
+    const base::Value* local_override,
     const base::Time current_time,
     const base::TimeDelta& used_time,
     const icu::TimeZone* const time_zone);

chrome/common/pref_names.cc

@@ -847,6 +847,10 @@ const char kTextToSpeechPitch[] = "settings.tts.speech_pitch";
 // system volume, and higher than 1.0 is louder.
 const char kTextToSpeechVolume[] = "settings.tts.speech_volume";
 
+// A dictionary containing the latest Time Limits override authorized by parent
+// access code.
+const char kTimeLimitLocalOverride[] = "screen_time.local_override";
+
 // A dictionary preference holding the usage time limit definitions for a user.
 const char kUsageTimeLimit[] = "screen_time.limit";
 

chrome/common/pref_names.h

@@ -275,6 +275,7 @@ extern const char kTextToSpeechLangToVoiceName[];
 extern const char kTextToSpeechRate[];
 extern const char kTextToSpeechPitch[];
 extern const char kTextToSpeechVolume[];
+extern const char kTimeLimitLocalOverride[];
 extern const char kUsageTimeLimit[];
 extern const char kScreenTimeLastState[];
 extern const char kEnableSyncConsent[];