Skip to content

Commit

Permalink
oobe: change demo mode detector to use screen exit codes
Browse files Browse the repository at this point in the history 
This CL moves responsibility of detecting idleness for starting
demo mode from CoreOobeHandler to two relevant screens.

Bug: 1100909, 1100910
Change-Id: Icd4142fdb75f3f7376b01d50b808078b42fb6872
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277722
Commit-Queue: Denis Kuznetsov [CET] <antrim@chromium.org>
Reviewed-by: Roman Aleksandrov <raleksandrov@google.com>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Roman Sorokin [CET] <rsorokin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#785821}
  • Loading branch information
Denis Kuznetsov authored and Commit Bot committed Jul 7, 2020
1 parent 92b9899 commit 44d0462
Show file tree
Hide file tree
Showing 17 changed files with 152 additions and 99 deletions.
19 changes: 6 additions & 13 deletions chrome/browser/chromeos/login/demo_mode/demo_mode_detector.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "base/command_line.h"
#include "base/strings/string_number_conversions.h"
#include "base/system/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"
Expand All @@ -32,17 +31,16 @@ void DemoModeDetector::RegisterPrefs(PrefRegistrySimple* registry) {
registry->RegisterInt64Pref(prefs::kTimeOnOobe, 0);
}

DemoModeDetector::DemoModeDetector()
: tick_clock_(base::DefaultTickClock::GetInstance()) {
DemoModeDetector::DemoModeDetector(const base::TickClock* clock,
Observer* observer)
: observer_(observer), tick_clock_(clock) {
DCHECK(observer_);
SetupTimeouts();
InitDetection();
}

DemoModeDetector::~DemoModeDetector() {}

void DemoModeDetector::SetTickClockForTest(const base::TickClock* test_clock) {
tick_clock_ = test_clock;
}

void DemoModeDetector::InitDetection() {
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableDemoMode))
Expand Down Expand Up @@ -77,11 +75,6 @@ void DemoModeDetector::InitDetection() {
StartOobeTimer();
}

void DemoModeDetector::StopDetection() {
oobe_timer_.Stop();
idle_detector_.reset();
}

void DemoModeDetector::StartIdleDetection() {
if (!idle_detector_) {
auto callback = base::BindRepeating(&DemoModeDetector::OnIdle,
Expand All @@ -102,7 +95,7 @@ void DemoModeDetector::OnIdle() {
if (demo_launched_)
return;
demo_launched_ = true;
LoginDisplayHost::default_host()->StartDemoAppLaunch();
observer_->OnShouldStartDemoMode();
}

void DemoModeDetector::OnOobeTimerUpdate() {
Expand Down
19 changes: 12 additions & 7 deletions chrome/browser/chromeos/login/demo_mode/demo_mode_detector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,30 +23,35 @@ namespace chromeos {
// Should be initialized on OOBE start.
class DemoModeDetector {
public:
DemoModeDetector();
// Interface for notification that device stayed in demo mode long enough
// to trigger Demo mode.
class Observer {
public:
virtual void OnShouldStartDemoMode() {}
virtual ~Observer() = default;
};

DemoModeDetector(const base::TickClock* clock, Observer* observer);
virtual ~DemoModeDetector();

void InitDetection();
void StopDetection();

// 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 const base::TimeDelta kDerelictDetectionTimeout;
static const base::TimeDelta kDerelictIdleTimeout;
static const base::TimeDelta kOobeTimerUpdateInterval;

private:
void InitDetection();
void StartIdleDetection();
void StartOobeTimer();
void OnIdle();
void OnOobeTimerUpdate();
void SetupTimeouts();
bool IsDerelict();

Observer* observer_;

// Total time this machine has spent on OOBE.
base::TimeDelta time_on_oobe_;

Expand Down
23 changes: 17 additions & 6 deletions chrome/browser/chromeos/login/demo_mode/demo_mode_detector_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,18 @@

namespace chromeos {

namespace {

class MockDetectorObserver : public DemoModeDetector::Observer {
public:
MockDetectorObserver() = default;
virtual ~MockDetectorObserver() = default;

MOCK_METHOD(void, OnShouldStartDemoMode, (), (override));
};

} // namespace

class DemoModeDetectorTest : public testing::Test {
protected:
DemoModeDetectorTest();
Expand All @@ -42,7 +54,7 @@ class DemoModeDetectorTest : public testing::Test {

private:
TestingPrefServiceSimple local_state_;
MockLoginDisplayHost login_display_host_;
MockDetectorObserver observer_;
ui::UserActivityDetector user_activity_detector_;
std::unique_ptr<DemoModeDetector> demo_mode_detector_;
std::unique_ptr<base::ThreadTaskRunnerHandle> runner_handle_;
Expand All @@ -65,17 +77,16 @@ DemoModeDetectorTest::~DemoModeDetectorTest() {
}

void DemoModeDetectorTest::ExpectDemoModeWillLaunch() {
EXPECT_CALL(login_display_host_, StartDemoAppLaunch());
EXPECT_CALL(observer_, OnShouldStartDemoMode());
}

void DemoModeDetectorTest::ExpectDemoModeWillNotLaunch() {
EXPECT_CALL(login_display_host_, StartDemoAppLaunch()).Times(0);
EXPECT_CALL(observer_, OnShouldStartDemoMode()).Times(0);
}

void DemoModeDetectorTest::StartDemoModeDetection() {
demo_mode_detector_ = std::make_unique<DemoModeDetector>();
demo_mode_detector_->SetTickClockForTest(runner_->GetMockTickClock());
demo_mode_detector_->InitDetection();
demo_mode_detector_ = std::make_unique<DemoModeDetector>(
runner_->GetMockTickClock(), &observer_);
}

void DemoModeDetectorTest::SetTimeOnOobePref(base::TimeDelta time_on_oobe) {
Expand Down
37 changes: 27 additions & 10 deletions chrome/browser/chromeos/login/screens/hid_detection_screen.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/metrics/histogram_macros.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/utf_string_conversions.h"
#include "base/time/default_tick_clock.h"
#include "chrome/browser/chromeos/login/wizard_controller.h"
#include "chrome/browser/ui/webui/chromeos/login/hid_detection_screen_handler.h"
#include "chrome/grit/generated_resources.h"
Expand Down Expand Up @@ -61,15 +62,21 @@ GetInputDeviceManagerBinderOverride() {

namespace chromeos {

HIDDetectionScreen::HIDDetectionScreen(
HIDDetectionView* view,
CoreOobeView* core_oobe_view,
const base::RepeatingClosure& exit_callback)
// static
std::string HIDDetectionScreen::GetResultString(Result result) {
switch (result) {
case Result::NEXT:
return "Next";
case Result::START_DEMO:
return "StartDemo";
}
}

HIDDetectionScreen::HIDDetectionScreen(HIDDetectionView* view,
const ScreenExitCallback& exit_callback)
: BaseScreen(HIDDetectionView::kScreenId, OobeScreenPriority::DEFAULT),
view_(view),
core_oobe_view_(core_oobe_view),
exit_callback_(exit_callback) {
DCHECK(core_oobe_view_);
if (view_)
view_->Bind(this);

Expand All @@ -95,7 +102,6 @@ void HIDDetectionScreen::OverrideInputDeviceManagerBinderForTesting(
}

void HIDDetectionScreen::OnContinueButtonClicked() {
core_oobe_view_->StopDemoModeDetection();
ContinueScenarioType scenario_type;
if (!pointing_device_id_.empty() && !keyboard_device_id_.empty())
scenario_type = All_DEVICES_DETECTED;
Expand All @@ -107,6 +113,16 @@ void HIDDetectionScreen::OnContinueButtonClicked() {
UMA_HISTOGRAM_ENUMERATION("HIDDetection.OOBEDevicesDetectedOnContinuePressed",
scenario_type, CONTINUE_SCENARIO_TYPE_SIZE);

CleanupOnExit();
exit_callback_.Run(Result::NEXT);
}

void HIDDetectionScreen::OnShouldStartDemoMode() {
CleanupOnExit();
exit_callback_.Run(Result::START_DEMO);
}

void HIDDetectionScreen::CleanupOnExit() {
// Switch off BT adapter if it was off before the screen and no BT device
// connected.
const bool adapter_is_powered =
Expand All @@ -116,7 +132,7 @@ void HIDDetectionScreen::OnContinueButtonClicked() {
if (adapter_is_powered && need_switching_off)
PowerOff();

exit_callback_.Run();
demo_mode_detector_.reset();
}

void HIDDetectionScreen::OnViewDestroyed(HIDDetectionView* view) {
Expand Down Expand Up @@ -145,16 +161,17 @@ void HIDDetectionScreen::ShowImpl() {
GetInputDevicesList();
else
UpdateDevices();

demo_mode_detector_ = std::make_unique<DemoModeDetector>(
base::DefaultTickClock::GetInstance(), this);
if (view_) {
view_->Show();
core_oobe_view_->InitDemoModeDetection();
}
}

void HIDDetectionScreen::HideImpl() {
if (is_hidden())
return;
demo_mode_detector_.reset();

if (discovery_session_.get())
discovery_session_->Stop();
Expand Down
36 changes: 23 additions & 13 deletions chrome/browser/chromeos/login/screens/hid_detection_screen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "chrome/browser/chromeos/login/demo_mode/demo_mode_detector.h"
#include "chrome/browser/chromeos/login/screens/base_screen.h"
#include "chrome/browser/ui/webui/chromeos/login/core_oobe_handler.h"
#include "device/bluetooth/bluetooth_adapter.h"
#include "device/bluetooth/bluetooth_device.h"
#include "device/bluetooth/bluetooth_discovery_session.h"
Expand All @@ -34,18 +34,21 @@ class HIDDetectionView;
class HIDDetectionScreen : public BaseScreen,
public device::BluetoothAdapter::Observer,
public device::BluetoothDevice::PairingDelegate,
public device::mojom::InputDeviceManagerClient {
public device::mojom::InputDeviceManagerClient,
public DemoModeDetector::Observer {
public:
using InputDeviceInfoPtr = device::mojom::InputDeviceInfoPtr;
using DeviceMap = std::map<std::string, InputDeviceInfoPtr>;

enum class Result { NEXT, START_DEMO };

using ScreenExitCallback = base::RepeatingCallback<void(Result result)>;

HIDDetectionScreen(HIDDetectionView* view,
CoreOobeView* core_oobe_view,
const base::RepeatingClosure& exit_callback);
const ScreenExitCallback& exit_callback);
~HIDDetectionScreen() override;

// Called when continue button was clicked.
void OnContinueButtonClicked();
static std::string GetResultString(Result result);

// This method is called when the view is being destroyed.
void OnViewDestroyed(HIDDetectionView* view);
Expand All @@ -64,12 +67,12 @@ class HIDDetectionScreen : public BaseScreen,
private:
friend class HIDDetectionScreenTest;

// BaseScreen implementation:
// BaseScreen:
void ShowImpl() override;
void HideImpl() override;
void OnUserAction(const std::string& action_id) override;

// device::BluetoothDevice::PairingDelegate implementation:
// device::BluetoothDevice::PairingDelegate:
void RequestPinCode(device::BluetoothDevice* device) override;
void RequestPasskey(device::BluetoothDevice* device) override;
void DisplayPinCode(device::BluetoothDevice* device,
Expand All @@ -81,7 +84,7 @@ class HIDDetectionScreen : public BaseScreen,
uint32_t passkey) override;
void AuthorizePairing(device::BluetoothDevice* device) override;

// device::BluetoothAdapter::Observer implementation.
// device::BluetoothAdapter::Observer:
void AdapterPresentChanged(device::BluetoothAdapter* adapter,
bool present) override;
void DeviceAdded(device::BluetoothAdapter* adapter,
Expand All @@ -91,10 +94,18 @@ class HIDDetectionScreen : public BaseScreen,
void DeviceRemoved(device::BluetoothAdapter* adapter,
device::BluetoothDevice* device) override;

// device::mojom::InputDeviceManagerClient implementation.
// device::mojom::InputDeviceManagerClient:
void InputDeviceAdded(InputDeviceInfoPtr info) override;
void InputDeviceRemoved(const std::string& id) override;

// DemoModeDetector::Observer:
void OnShouldStartDemoMode() override;

// Called when continue button was clicked.
void OnContinueButtonClicked();

void CleanupOnExit();

// Types of dialog leaving scenarios for UMA metric.
enum ContinueScenarioType {
// Only pointing device detected, user pressed 'Continue'.
Expand Down Expand Up @@ -205,10 +216,9 @@ class HIDDetectionScreen : public BaseScreen,

HIDDetectionView* view_;

// CoreOobeView is necessary for starting/stopping the demo mode detection
CoreOobeView* core_oobe_view_ = nullptr;
const ScreenExitCallback exit_callback_;

base::RepeatingClosure exit_callback_;
std::unique_ptr<DemoModeDetector> demo_mode_detector_;

// Default bluetooth adapter, used for all operations.
scoped_refptr<device::BluetoothAdapter> adapter_;
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/chromeos/login/screens/mock_welcome_screen.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ namespace chromeos {

MockWelcomeScreen::MockWelcomeScreen(
WelcomeView* view,
const base::RepeatingClosure& exit_callback)
const WelcomeScreen::ScreenExitCallback& exit_callback)
: WelcomeScreen(view, exit_callback) {}

void MockWelcomeScreen::ExitScreen() {
exit_callback()->Run();
exit_callback()->Run(WelcomeScreen::Result::NEXT);
}

MockWelcomeScreen::~MockWelcomeScreen() = default;
Expand Down
3 changes: 1 addition & 2 deletions chrome/browser/chromeos/login/screens/mock_welcome_screen.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace chromeos {
class MockWelcomeScreen : public WelcomeScreen {
public:
MockWelcomeScreen(WelcomeView* view,
const base::RepeatingClosure& exit_callback);
const WelcomeScreen::ScreenExitCallback& exit_callback);
~MockWelcomeScreen() override;

MOCK_METHOD(void, ShowImpl, ());
Expand All @@ -42,7 +42,6 @@ class MockWelcomeView : public WelcomeView {
MOCK_METHOD(void, MockUnbind, ());
MOCK_METHOD(void, Show, ());
MOCK_METHOD(void, Hide, ());
MOCK_METHOD(void, StopDemoModeDetection, ());
MOCK_METHOD(void, ReloadLocalizedContent, ());
MOCK_METHOD(void, SetInputMethodId, (const std::string& input_method_id));
MOCK_METHOD(void, SetTimezoneId, (const std::string& timezone_id));
Expand Down

0 comments on commit 44d0462

Please sign in to comment.