Revert "Disable Demo app launch when device is in Dev mode"

This reverts commit 4c9f4636.

Reason for revert: Broke build: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg/8728

Original change's description:
> Disable Demo app launch when device is in Dev mode
> 
> Wrote unit test for DemoModeDetector,  accounting for existing behavior,
> and also the new behavior introduced in this CL.
> 
> R=​michaelpg@chromium.org
> TEST=tested manually on dev chromebook, and with demo_mode_detector_unittest.cc
> 
> Bug: 877655
> Change-Id: Ibfd78ec1cf2d87c3acad71a8244e519f70601cec
> Reviewed-on: https://chromium-review.googlesource.com/c/1298417
> Commit-Queue: Danan S <danan@chromium.org>
> Reviewed-by: Dan Erat <derat@chromium.org>
> Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
> Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#605408}

TBR=derat@chromium.org,sadrul@chromium.org,michaelpg@chromium.org,danan@chromium.org

Change-Id: Ia8275a42d27411ce72075b2de83d37dd01ea4924
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 877655
Reviewed-on: https://chromium-review.googlesource.com/c/1318546Reviewed-by: Dmitry Titov <dimich@chromium.org>
Commit-Queue: Dmitry Titov <dimich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605467}

chrome/browser/chromeos/BUILD.gn

@@ -2199,7 +2199,6 @@ source_set("unit_tests") {
     "lock_screen_apps/state_controller_unittest.cc",
     "login/auth/cryptohome_authenticator_unittest.cc",
     "login/demo_mode/demo_extensions_external_loader_unittest.cc",
-    "login/demo_mode/demo_mode_detector_unittest.cc",
     "login/demo_mode/demo_mode_resources_remover_unittest.cc",
     "login/demo_mode/demo_session_unittest.cc",
     "login/demo_mode/demo_setup_controller_unittest.cc",

chrome/browser/chromeos/idle_detector.cc

@@ -4,16 +4,14 @@
 
 #include "chrome/browser/chromeos/idle_detector.h"
 
-#include "base/location.h"
-#include "base/time/default_tick_clock.h"
+#include "base/bind.h"
+#include "base/logging.h"
 #include "ui/base/user_activity/user_activity_detector.h"
 
 namespace chromeos {
 
-IdleDetector::IdleDetector(const base::RepeatingClosure& on_idle_callback,
-                           const base::TickClock* tick_clock)
-    : timer_(std::make_unique<base::OneShotTimer>(tick_clock)),
-      idle_callback_(on_idle_callback) {}
+IdleDetector::IdleDetector(const base::Closure& on_idle_callback)
+    : idle_callback_(on_idle_callback) {}
 
 IdleDetector::~IdleDetector() {
   ui::UserActivityDetector* user_activity_detector =
@@ -34,10 +32,10 @@ void IdleDetector::Start(const base::TimeDelta& timeout) {
 }
 
 void IdleDetector::ResetTimer() {
-  if (timer_->IsRunning())
-    timer_->Reset();
+  if (timer_.IsRunning())
+    timer_.Reset();
   else
-    timer_->Start(FROM_HERE, timeout_, idle_callback_);
+    timer_.Start(FROM_HERE, timeout_, idle_callback_);
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/idle_detector.h

@@ -6,8 +6,8 @@
 #define CHROME_BROWSER_CHROMEOS_IDLE_DETECTOR_H_
 
 #include "base/callback.h"
+#include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "ui/base/user_activity/user_activity_observer.h"
@@ -16,15 +16,11 @@ namespace chromeos {
 
 class IdleDetector : public ui::UserActivityObserver {
  public:
-  IdleDetector(const base::RepeatingClosure& on_idle_callback,
-               const base::TickClock* tick_clock);
-
+  explicit IdleDetector(const base::Closure& on_idle_callback);
   ~IdleDetector() override;
 
   void Start(const base::TimeDelta& timeout);
 
-  void SetTickClockForTest(const base::TickClock* test_clock);
-
  private:
   // ui::UserActivityObserver overrides:
   void OnUserActivity(const ui::Event* event) override;
@@ -32,9 +28,9 @@ class IdleDetector : public ui::UserActivityObserver {
   // Resets |timer_| to fire when we reach our idle timeout.
   void ResetTimer();
 
-  std::unique_ptr<base::OneShotTimer> timer_;
+  base::OneShotTimer timer_;
 
-  base::RepeatingClosure idle_callback_;
+  base::Closure idle_callback_;
 
   base::TimeDelta timeout_;
 

chrome/browser/chromeos/login/demo_mode/demo_mode_detector.cc

@@ -5,18 +5,20 @@
 #include "chrome/browser/chromeos/login/demo_mode/demo_mode_detector.h"
 
 #include "base/command_line.h"
-#include "base/logging.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/sys_info.h"
-#include "base/time/default_tick_clock.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/login/ui/login_display_host.h"
 #include "chrome/common/pref_names.h"
 #include "chromeos/chromeos_switches.h"
-#include "chromeos/dbus/dbus_switches.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 
+namespace {
+const int kDerelectDetectionTimeoutSeconds = 8 * 60 * 60;  // 8 hours.
+const int kDerelectIdleTimeoutSeconds = 5 * 60;            // 5 minutes.
+const int kOobeTimerUpdateIntervalSeconds = 5 * 60;        // 5 minutes.
+}  // namespace
 
 namespace chromeos {
 
@@ -25,18 +27,12 @@ void DemoModeDetector::RegisterPrefs(PrefRegistrySimple* registry) {
   registry->RegisterInt64Pref(prefs::kTimeOnOobe, 0);
 }
 
-DemoModeDetector::DemoModeDetector()
-    : tick_clock_(base::DefaultTickClock::GetInstance()),
-      weak_ptr_factory_(this) {
+DemoModeDetector::DemoModeDetector() : weak_ptr_factory_(this) {
   SetupTimeouts();
 }
 
 DemoModeDetector::~DemoModeDetector() {}
 
-void DemoModeDetector::SetTickClockForTest(const base::TickClock* test_clock) {
-  tick_clock_ = test_clock;
-}
-
 void DemoModeDetector::InitDetection() {
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDisableDemoMode))
@@ -48,14 +44,6 @@ void DemoModeDetector::InitDetection() {
       base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kDerelictIdleTimeout);
 
-  // Devices in retail won't be in dev mode, and DUTs (devices under test) often
-  // sit unused at OOBE for a while.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kSystemDevMode) &&
-      !has_derelict_switch) {
-    return;
-  }
-
   if (base::SysInfo::IsRunningOnChromeOS() && !has_derelict_switch) {
     std::string track;
     // We're running on an actual device; if we cannot find our release track
@@ -78,9 +66,8 @@ void DemoModeDetector::StopDetection() {
 
 void DemoModeDetector::StartIdleDetection() {
   if (!idle_detector_) {
-    auto callback = base::BindRepeating(&DemoModeDetector::OnIdle,
-                                        weak_ptr_factory_.GetWeakPtr());
-    idle_detector_ = std::make_unique<IdleDetector>(callback, tick_clock_);
+    idle_detector_.reset(new IdleDetector(
+        base::Bind(&DemoModeDetector::OnIdle, weak_ptr_factory_.GetWeakPtr())));
   }
   idle_detector_->Start(derelict_idle_timeout_);
 }
@@ -120,37 +107,33 @@ void DemoModeDetector::SetupTimeouts() {
       base::TimeDelta::FromSeconds(prefs->GetInt64(prefs::kTimeOnOobe));
 
   int derelict_detection_timeout;
-  if (cmdline->HasSwitch(switches::kDerelictDetectionTimeout) &&
-      base::StringToInt(
+  if (!cmdline->HasSwitch(switches::kDerelictDetectionTimeout) ||
+      !base::StringToInt(
           cmdline->GetSwitchValueASCII(switches::kDerelictDetectionTimeout),
           &derelict_detection_timeout)) {
-    derelict_detection_timeout_ =
-        base::TimeDelta::FromSeconds(derelict_detection_timeout);
-  } else {
-    derelict_detection_timeout_ = kDerelictDetectionTimeout;
+    derelict_detection_timeout = kDerelectDetectionTimeoutSeconds;
   }
+  derelict_detection_timeout_ =
+      base::TimeDelta::FromSeconds(derelict_detection_timeout);
 
   int derelict_idle_timeout;
-  if (cmdline->HasSwitch(switches::kDerelictIdleTimeout) &&
-      base::StringToInt(
+  if (!cmdline->HasSwitch(switches::kDerelictIdleTimeout) ||
+      !base::StringToInt(
           cmdline->GetSwitchValueASCII(switches::kDerelictIdleTimeout),
           &derelict_idle_timeout)) {
-    derelict_idle_timeout_ =
-        base::TimeDelta::FromSeconds(derelict_idle_timeout);
-  } else {
-    derelict_idle_timeout_ = kDerelictIdleTimeout;
+    derelict_idle_timeout = kDerelectIdleTimeoutSeconds;
   }
+  derelict_idle_timeout_ = base::TimeDelta::FromSeconds(derelict_idle_timeout);
 
   int oobe_timer_update_interval;
-  if (cmdline->HasSwitch(switches::kOobeTimerInterval) &&
-      base::StringToInt(
+  if (!cmdline->HasSwitch(switches::kOobeTimerInterval) ||
+      !base::StringToInt(
           cmdline->GetSwitchValueASCII(switches::kOobeTimerInterval),
           &oobe_timer_update_interval)) {
-    oobe_timer_update_interval_ =
-        base::TimeDelta::FromSeconds(oobe_timer_update_interval);
-  } else {
-    oobe_timer_update_interval_ = kOobeTimerUpdateInterval;
+    oobe_timer_update_interval = kOobeTimerUpdateIntervalSeconds;
   }
+  oobe_timer_update_interval_ =
+      base::TimeDelta::FromSeconds(oobe_timer_update_interval);
 
   // In case we'd be derelict before our timer is set to trigger, reduce
   // the interval so we check again when we're scheduled to go derelict.

chrome/browser/chromeos/login/demo_mode/demo_mode_detector.h

@@ -10,7 +10,6 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/time/tick_clock.h"
 #include "base/time/time.h"
 #include "base/timer/timer.h"
 #include "chrome/browser/chromeos/idle_detector.h"
@@ -32,16 +31,6 @@ class DemoModeDetector {
   // Registers the preference for derelict state.
   static void RegisterPrefs(PrefRegistrySimple* registry);
 
-  // Sets an alternative clock for testing purposes.
-  void SetTickClockForTest(const base::TickClock* test_clock);
-
-  static constexpr base::TimeDelta kDerelictDetectionTimeout =
-      base::TimeDelta::FromHours(8);
-  static constexpr base::TimeDelta kDerelictIdleTimeout =
-      base::TimeDelta::FromMinutes(5);
-  static constexpr base::TimeDelta kOobeTimerUpdateInterval =
-      base::TimeDelta::FromMinutes(5);
-
  private:
   void StartIdleDetection();
   void StartOobeTimer();
@@ -68,8 +57,6 @@ class DemoModeDetector {
 
   bool demo_launched_ = false;
 
-  const base::TickClock* tick_clock_;
-
   base::WeakPtrFactory<DemoModeDetector> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(DemoModeDetector);

chrome/browser/chromeos/login/demo_mode/demo_mode_detector_unittest.cc

-// Copyright 2018 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/login/demo_mode/demo_mode_detector.h"
-
-#include <memory>
-
-#include "base/memory/ref_counted.h"
-#include "base/test/scoped_command_line.h"
-#include "base/test/test_mock_time_task_runner.h"
-#include "base/threading/thread_task_runner_handle.h"
-#include "base/values.h"
-#include "chrome/browser/chromeos/login/ui/mock_login_display_host.h"
-#include "chrome/browser/chromeos/scoped_set_running_on_chromeos_for_testing.h"
-#include "chrome/common/pref_names.h"
-#include "chrome/test/base/testing_browser_process.h"
-#include "chromeos/chromeos_switches.h"
-#include "chromeos/dbus/dbus_switches.h"
-#include "components/prefs/testing_pref_service.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "ui/base/user_activity/user_activity_detector.h"
-#include "ui/events/base_event_utils.h"
-
-namespace chromeos {
-
-class DemoModeDetectorTest : public testing::Test {
- protected:
-  DemoModeDetectorTest();
-  ~DemoModeDetectorTest() override;
-
-  void StartDemoModeDetection();
-  void ExpectDemoModeWillLaunch();
-  void ExpectDemoModeWillNotLaunch();
-  void SetTimeOnOobePref(base::TimeDelta time_on_oobe);
-  base::TimeDelta GetTimeOnOobePref();
-  void DestroyDemoModeDetector();
-  void SimulateUserActivity();
-  scoped_refptr<base::TestMockTimeTaskRunner> runner_;
-
- private:
-  TestingPrefServiceSimple local_state_;
-  MockLoginDisplayHost login_display_host_;
-  ui::UserActivityDetector user_activity_detector_;
-  std::unique_ptr<DemoModeDetector> demo_mode_detector_;
-  std::unique_ptr<base::ThreadTaskRunnerHandle> runner_handle_;
-};
-
-DemoModeDetectorTest::DemoModeDetectorTest() {
-  TestingBrowserProcess::GetGlobal()->SetLocalState(&local_state_);
-  DemoModeDetector::RegisterPrefs(local_state_.registry());
-  runner_ = base::MakeRefCounted<base::TestMockTimeTaskRunner>();
-  // If we don't register the TestMockTimeTaskRunner with a
-  // ThreadTaskRunnerHandle, the timers in the test fail
-  // to initialize, saying they need a SequencedContext.
-  runner_handle_ = std::make_unique<base::ThreadTaskRunnerHandle>(runner_);
-}
-
-DemoModeDetectorTest::~DemoModeDetectorTest() {
-  demo_mode_detector_.reset();
-  runner_handle_.reset();
-  TestingBrowserProcess::GetGlobal()->SetLocalState(nullptr);
-}
-
-void DemoModeDetectorTest::ExpectDemoModeWillLaunch() {
-  EXPECT_CALL(login_display_host_, StartDemoAppLaunch());
-}
-
-void DemoModeDetectorTest::ExpectDemoModeWillNotLaunch() {
-  EXPECT_CALL(login_display_host_, StartDemoAppLaunch()).Times(0);
-}
-
-void DemoModeDetectorTest::StartDemoModeDetection() {
-  demo_mode_detector_ = std::make_unique<DemoModeDetector>();
-  demo_mode_detector_->SetTickClockForTest(runner_->GetMockTickClock());
-  demo_mode_detector_->InitDetection();
-}
-
-void DemoModeDetectorTest::SetTimeOnOobePref(base::TimeDelta time_on_oobe) {
-  local_state_.SetUserPref(prefs::kTimeOnOobe,
-                           std::make_unique<base::Value>(
-                               base::Int64ToString(time_on_oobe.InSeconds())));
-}
-
-base::TimeDelta DemoModeDetectorTest::GetTimeOnOobePref() {
-  return base::TimeDelta::FromSeconds(
-      local_state_.GetInt64(prefs::kTimeOnOobe));
-}
-
-void DemoModeDetectorTest::SimulateUserActivity() {
-  user_activity_detector_.HandleExternalUserActivity();
-}
-
-// Tests follow.
-
-// Test to ensure that Demo mode isn't launched before the detector
-// has entered the derelict state.
-TEST_F(DemoModeDetectorTest, DemoModeWillNotLaunchBeforeDerelict) {
-  ExpectDemoModeWillNotLaunch();
-
-  StartDemoModeDetection();
-  // Run for half the timeout.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictDetectionTimeout -
-                         DemoModeDetector::kDerelictDetectionTimeout / 2);
-}
-
-// Test to ensure that Demo mode isn't launched after the detector
-// has entered the derelict state but before the idle timeout.
-TEST_F(DemoModeDetectorTest,
-       DemoModeWillNotLaunchAfterDerelictAndBeforeIdleTimeout) {
-  ExpectDemoModeWillNotLaunch();
-
-  StartDemoModeDetection();
-  // Run through the derelict threshold.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictDetectionTimeout);
-  // Run for 1 minute less than the idle threshold.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictIdleTimeout -
-                         base::TimeDelta::FromMinutes(1));
-}
-
-// Test to ensure that Demo mode isn't launched after the detector
-// has entered the derelict state but user activity is preventing the idle
-// timeout.
-TEST_F(DemoModeDetectorTest,
-       DemoModeWillNotLaunchAfterDerelictWithUserActivity) {
-  ExpectDemoModeWillNotLaunch();
-
-  StartDemoModeDetection();
-
-  // Run for through the derelict threshold.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictIdleTimeout);
-
-  // Run for 2 more minutes (less than the idle threshold).
-  runner_->FastForwardBy(base::TimeDelta::FromMinutes(2));
-
-  // Simulate a user activity event.
-  SimulateUserActivity();
-
-  // Run for 3 more minutes (more than the idle threshold).
-  runner_->FastForwardBy(base::TimeDelta::FromMinutes(3));
-
-  // Simulate a user activity event.
-  SimulateUserActivity();
-}
-
-// Test to ensure that Demo mode is launched after the detector
-// has entered the derelict state and after the idle timeout.
-TEST_F(DemoModeDetectorTest, DemoModeWillLaunchAfterDerelictAndIdleTimeout) {
-  ExpectDemoModeWillLaunch();
-
-  StartDemoModeDetection();
-  // Run for long enough for all thresholds to be exceeded.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictDetectionTimeout +
-                         DemoModeDetector::kDerelictIdleTimeout);
-}
-
-// Test to ensure that a device in dev mode disables the demo mode.
-TEST_F(DemoModeDetectorTest, DemoModeWillNotLaunchInDevMode) {
-  // Set the command line dev mode switch.
-  auto command_line_ = std::make_unique<base::test::ScopedCommandLine>();
-  command_line_->GetProcessCommandLine()->AppendSwitch(
-      switches::kSystemDevMode);
-
-  ExpectDemoModeWillNotLaunch();
-
-  StartDemoModeDetection();
-
-  // Run through the derelict threshold.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictDetectionTimeout);
-}
-
-// Test to ensure that the --disable-demo-mode switch disables demo mode.
-TEST_F(DemoModeDetectorTest, DemoModeWillNotLaunchWhenDisabledBySwitch) {
-  // Set the command line dev mode switch.
-  auto command_line_ = std::make_unique<base::test::ScopedCommandLine>();
-  command_line_->GetProcessCommandLine()->AppendSwitch(
-      switches::kDisableDemoMode);
-
-  ExpectDemoModeWillNotLaunch();
-
-  StartDemoModeDetection();
-  // Run through the derelict threshold.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictDetectionTimeout);
-}
-
-// Test to ensure that demo mode is disabled on "testimage" LsbRelease
-// CHROMEOS_RELEASE_TRACK values.
-TEST_F(DemoModeDetectorTest, DemoModeWillNotLaunchWhenTestimageInLsbRelease) {
-  std::string lsb_release =
-      "CHROMEOS_RELEASE_NAME=Chromium OS\n"
-      "CHROMEOS_RELEASE_TRACK=testimage\n";
-
-  base::Time release_time;
-  EXPECT_TRUE(
-      base::Time::FromString("Wed, 24 Oct 2018 12:00:00 PDT", &release_time));
-
-  ScopedSetRunningOnChromeOSForTesting version_info(lsb_release, release_time);
-
-  ExpectDemoModeWillNotLaunch();
-
-  StartDemoModeDetection();
-  // Run through the derelict threshold.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictDetectionTimeout);
-}
-
-// Test to ensure that Demo mode is launched after the detector
-// has resumed (i.e. after shutdown/reboot).
-TEST_F(DemoModeDetectorTest,
-       DemoModeWillLaunchAfterResumedAndDerelictAndIdleTimeout) {
-  ExpectDemoModeWillLaunch();
-
-  // Simulate 1 hour less than the threshold elapsed by setting pref.
-  const auto elapsed_time = DemoModeDetector::kDerelictDetectionTimeout -
-                            base::TimeDelta::FromHours(1);
-  SetTimeOnOobePref(elapsed_time);
-  EXPECT_EQ(GetTimeOnOobePref(), elapsed_time);
-
-  StartDemoModeDetection();
-
-  // Run another hour to hit the threshold.
-  runner_->FastForwardBy(base::TimeDelta::FromHours(1));
-
-  // Run through the idle timeout.
-  runner_->FastForwardBy(DemoModeDetector::kDerelictIdleTimeout);
-}
-
-}  // namespace chromeos

chrome/browser/chromeos/login/ui/preloaded_web_view.cc

@@ -5,7 +5,6 @@
 #include "chrome/browser/chromeos/login/ui/preloaded_web_view.h"
 
 #include "base/callback_helpers.h"
-#include "base/time/default_tick_clock.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/chromeos/idle_detector.h"
 #include "chrome/browser/profiles/profile.h"
@@ -36,9 +35,7 @@ PreloadedWebView::~PreloadedWebView() {}
 void PreloadedWebView::PreloadOnIdle(PreloadCallback preload) {
   preload_function_ = std::move(preload);
   idle_detector_ = std::make_unique<chromeos::IdleDetector>(
-      base::BindRepeating(&PreloadedWebView::RunPreloader,
-                          weak_factory_.GetWeakPtr()),
-      base::DefaultTickClock::GetInstance());
+      base::Bind(&PreloadedWebView::RunPreloader, weak_factory_.GetWeakPtr()));
   idle_detector_->Start(
       base::TimeDelta::FromSeconds(kIdleSecondsBeforePreloadingLockScreen));
 }