Commit be77d98e authored by Jun Mukai's avatar Jun Mukai Committed by Commit Bot

Refine ForceUiTabletModeState

Previously, ForceUiTabletModeState uses kLockInTabletMode and
kLockInClamshellMode, and use forced_ui_mode_ to enforce the
tablet-mode state. However, this turns out not correct on
the handling of screen_orientation_controller.

To make ScreenOrientationController (and maybe some other entities)
work with testing situation, ForceUiTabletModeState should set
is_in_tablet_physical_state flag.

In this CL:
- change a simple 'force_physical_tablet_state' boolean to tri-state.
- introduce new TabletModeBehaviors for the new states.
- since it is considered to be 'physically tablet' status but
  we don't want to auto-rotate the screen due to accelerometer,
  create a new callback and add a mechanism to suppress the auto
  rotation.

This CL does not change all existing ash unit tests -- that will be done
in another CL.

Bug: b/161253230
Test: ScreenOrientationControllerTest, tast arc.WMNonresizableConversion, make sure the screen orientation
Change-Id: Ic48f8570df103fe58336faa675f9c1753bb63ee8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310773
Commit-Queue: Jun Mukai <mukai@chromium.org>
Reviewed-by: default avatarMitsuru Oshima <oshima@chromium.org>
Reviewed-by: default avatarAhmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791319}
parent 3055799b
...@@ -188,6 +188,8 @@ class AccelerometerFileReader ...@@ -188,6 +188,8 @@ class AccelerometerFileReader
void StartListenToTabletModeController(); void StartListenToTabletModeController();
void StopListenToTabletModeController(); void StopListenToTabletModeController();
void SetEmitEvents(bool emit_events);
// TabletModeObserver: // TabletModeObserver:
void OnTabletPhysicalStateChanged() override; void OnTabletPhysicalStateChanged() override;
...@@ -261,6 +263,8 @@ class AccelerometerFileReader ...@@ -261,6 +263,8 @@ class AccelerometerFileReader
// reading to an AccelerometerUpdate and notifies observers. // reading to an AccelerometerUpdate and notifies observers.
void ReadFileAndNotify(); void ReadFileAndNotify();
void SetEmitEventsInternal(bool emit_events);
// State of ChromeOS EC lid angle driver, if SUPPORTED, it means EC can handle // State of ChromeOS EC lid angle driver, if SUPPORTED, it means EC can handle
// lid angle calculation. // lid angle calculation.
ECLidAngleDriver ec_lid_angle_driver_ = UNKNOWN; ECLidAngleDriver ec_lid_angle_driver_ = UNKNOWN;
...@@ -271,6 +275,8 @@ class AccelerometerFileReader ...@@ -271,6 +275,8 @@ class AccelerometerFileReader
// True if periodical accelerometer read is on. // True if periodical accelerometer read is on.
bool accelerometer_read_on_ = false; bool accelerometer_read_on_ = false;
bool emit_events_ = true;
// The time at which initialization re-tries should stop. // The time at which initialization re-tries should stop.
base::TimeTicks initialization_timeout_; base::TimeTicks initialization_timeout_;
...@@ -541,6 +547,17 @@ void AccelerometerFileReader::StopListenToTabletModeController() { ...@@ -541,6 +547,17 @@ void AccelerometerFileReader::StopListenToTabletModeController() {
Shell::Get()->tablet_mode_controller()->RemoveObserver(this); Shell::Get()->tablet_mode_controller()->RemoveObserver(this);
} }
void AccelerometerFileReader::SetEmitEvents(bool emit_events) {
task_runner_->PostNonNestableTask(
FROM_HERE, base::BindOnce(&AccelerometerFileReader::SetEmitEventsInternal,
this, emit_events));
}
void AccelerometerFileReader::SetEmitEventsInternal(bool emit_events) {
DCHECK(base::SequencedTaskRunnerHandle::IsSet());
emit_events_ = emit_events;
}
void AccelerometerFileReader::OnTabletPhysicalStateChanged() { void AccelerometerFileReader::OnTabletPhysicalStateChanged() {
// When CrOS EC lid angle driver is not present, accelerometer read is always // When CrOS EC lid angle driver is not present, accelerometer read is always
// ON and can't be tuned. Thus AccelerometerFileReader no longer listens to // ON and can't be tuned. Thus AccelerometerFileReader no longer listens to
...@@ -704,6 +721,9 @@ void AccelerometerFileReader::ReadFileAndNotify() { ...@@ -704,6 +721,9 @@ void AccelerometerFileReader::ReadFileAndNotify() {
} }
} }
if (!emit_events_)
return;
observers_->Notify(FROM_HERE, observers_->Notify(FROM_HERE,
&AccelerometerReader::Observer::OnAccelerometerUpdated, &AccelerometerReader::Observer::OnAccelerometerUpdated,
update_); update_);
...@@ -749,6 +769,10 @@ void AccelerometerReader::StopListenToTabletModeController() { ...@@ -749,6 +769,10 @@ void AccelerometerReader::StopListenToTabletModeController() {
accelerometer_file_reader_->StopListenToTabletModeController(); accelerometer_file_reader_->StopListenToTabletModeController();
} }
void AccelerometerReader::SetEnabled(bool enabled) {
accelerometer_file_reader_->SetEmitEvents(enabled);
}
AccelerometerReader::AccelerometerReader() AccelerometerReader::AccelerometerReader()
: accelerometer_file_reader_(new AccelerometerFileReader()) {} : accelerometer_file_reader_(new AccelerometerFileReader()) {}
......
...@@ -49,6 +49,11 @@ class ASH_EXPORT AccelerometerReader { ...@@ -49,6 +49,11 @@ class ASH_EXPORT AccelerometerReader {
void StartListenToTabletModeController(); void StartListenToTabletModeController();
void StopListenToTabletModeController(); void StopListenToTabletModeController();
// Controls the availability of emitting acccelerometer reader events to
// its observers. This shouldn't be called normally, but Tast tests should
// be able to control the accelerometer feature.
void SetEnabled(bool enabled);
protected: protected:
AccelerometerReader(); AccelerometerReader();
virtual ~AccelerometerReader(); virtual ~AccelerometerReader();
......
...@@ -57,7 +57,7 @@ display::ManagedDisplayInfo CreateDisplayInfo(int64_t id, ...@@ -57,7 +57,7 @@ display::ManagedDisplayInfo CreateDisplayInfo(int64_t id,
} }
void EnableTabletMode(bool enable) { void EnableTabletMode(bool enable) {
Shell::Get()->tablet_mode_controller()->SetEnabledForTest(enable); Shell::Get()->tablet_mode_controller()->ForceUiTabletModeState(enable);
} }
bool RotationLocked() { bool RotationLocked() {
......
...@@ -213,7 +213,7 @@ constexpr TabletModeController::TabletModeBehavior kOnBySensor{ ...@@ -213,7 +213,7 @@ constexpr TabletModeController::TabletModeBehavior kOnBySensor{
/*observe_pointer_device_events=*/true, /*observe_pointer_device_events=*/true,
/*block_internal_input_device=*/true, /*block_internal_input_device=*/true,
/*always_show_overview_button=*/false, /*always_show_overview_button=*/false,
/*force_physical_tablet_state=*/false, TabletModeController::ForcePhysicalTabletState::kDefault,
}; };
// Defines the behavior that sticks to tablet mode. Used to implement the // Defines the behavior that sticks to tablet mode. Used to implement the
...@@ -224,7 +224,7 @@ constexpr TabletModeController::TabletModeBehavior kLockInTabletMode{ ...@@ -224,7 +224,7 @@ constexpr TabletModeController::TabletModeBehavior kLockInTabletMode{
/*observe_pointer_device_events=*/false, /*observe_pointer_device_events=*/false,
/*block_internal_input_device=*/false, /*block_internal_input_device=*/false,
/*always_show_overview_button=*/true, /*always_show_overview_button=*/true,
/*force_physical_tablet_state=*/false, TabletModeController::ForcePhysicalTabletState::kDefault,
}; };
// Defines the behavior that sticks to tablet mode. Used to implement the // Defines the behavior that sticks to tablet mode. Used to implement the
...@@ -235,7 +235,7 @@ constexpr TabletModeController::TabletModeBehavior kLockInClamshellMode{ ...@@ -235,7 +235,7 @@ constexpr TabletModeController::TabletModeBehavior kLockInClamshellMode{
/*observe_pointer_device_events=*/false, /*observe_pointer_device_events=*/false,
/*block_internal_input_device=*/false, /*block_internal_input_device=*/false,
/*always_show_overview_button=*/false, /*always_show_overview_button=*/false,
/*force_physical_tablet_state=*/false, TabletModeController::ForcePhysicalTabletState::kDefault,
}; };
// Defines the behavior used for testing. It prevents the device from // Defines the behavior used for testing. It prevents the device from
...@@ -246,7 +246,31 @@ constexpr TabletModeController::TabletModeBehavior kOnForTest{ ...@@ -246,7 +246,31 @@ constexpr TabletModeController::TabletModeBehavior kOnForTest{
/*observe_pointer_device_events=*/true, /*observe_pointer_device_events=*/true,
/*block_internal_input_device=*/true, /*block_internal_input_device=*/true,
/*always_show_overview_button=*/false, /*always_show_overview_button=*/false,
/*force_physical_tablet_state=*/true, TabletModeController::ForcePhysicalTabletState::kForceTabletMode,
};
// Used for the testing API to forcibly enter into the tablet mode. It should
// not observe hardware events as tests want to stick with the tablet mode, and
// it should not block internal keyboard as some tests may want to use keyboard
// events in the tablet mode.
// TODO(mukai): consolidate this with kOnFOrTest.
constexpr TabletModeController::TabletModeBehavior kOnForAutotest{
/*use_sensor=*/false,
/*observe_display_events=*/false,
/*observe_pointer_device_events=*/false,
/*block_internal_input_device=*/false,
/*always_show_overview_button=*/false,
TabletModeController::ForcePhysicalTabletState::kForceTabletMode,
};
// Used for the testing API to forcibly exit from the tablet mode.
constexpr TabletModeController::TabletModeBehavior kOffForAutotest{
/*use_sensor=*/false,
/*observe_display_events=*/false,
/*observe_pointer_device_events=*/false,
/*block_internal_input_device=*/false,
/*always_show_overview_button=*/false,
TabletModeController::ForcePhysicalTabletState::kForceClamshellMode,
}; };
// Used for development purpose (currently debug shortcut shift-ctrl-alt). This // Used for development purpose (currently debug shortcut shift-ctrl-alt). This
...@@ -258,7 +282,7 @@ constexpr TabletModeController::TabletModeBehavior kOnForDev{ ...@@ -258,7 +282,7 @@ constexpr TabletModeController::TabletModeBehavior kOnForDev{
/*observe_pointer_device_events=*/true, /*observe_pointer_device_events=*/true,
/*block_internal_input_device=*/false, /*block_internal_input_device=*/false,
/*always_show_overview_button=*/true, /*always_show_overview_button=*/true,
/*force_physical_tablet_state=*/true, TabletModeController::ForcePhysicalTabletState::kForceTabletMode,
}; };
using LidState = chromeos::PowerManagerClient::LidState; using LidState = chromeos::PowerManagerClient::LidState;
...@@ -531,19 +555,22 @@ void TabletModeController::ForceUiTabletModeState( ...@@ -531,19 +555,22 @@ void TabletModeController::ForceUiTabletModeState(
base::Optional<bool> enabled) { base::Optional<bool> enabled) {
if (!enabled.has_value()) { if (!enabled.has_value()) {
tablet_mode_behavior_ = kDefault; tablet_mode_behavior_ = kDefault;
forced_ui_mode_ = UiMode::kNone; AccelerometerReader::GetInstance()->SetEnabled(true);
if (!SetIsInTabletPhysicalState(CalculateIsInTabletPhysicalState())) if (!SetIsInTabletPhysicalState(CalculateIsInTabletPhysicalState()))
UpdateUiTabletState(); UpdateUiTabletState();
return; return;
} }
if (*enabled) { if (*enabled) {
tablet_mode_behavior_ = kLockInTabletMode; tablet_mode_behavior_ = kOnForAutotest;
forced_ui_mode_ = UiMode::kTabletMode;
} else { } else {
tablet_mode_behavior_ = kLockInClamshellMode; tablet_mode_behavior_ = kOffForAutotest;
forced_ui_mode_ = UiMode::kClamshell;
} }
UpdateUiTabletState(); // We want to suppress the accelerometer to auto-rotate the screen based on
// the physical orientation, as it will confuse the test scenarios. Note that
// this should not block ScreenOrientationController as the screen may want
// to be rotated for other factors.
AccelerometerReader::GetInstance()->SetEnabled(false);
SetIsInTabletPhysicalState(CalculateIsInTabletPhysicalState());
} }
void TabletModeController::SetEnabledForTest(bool enabled) { void TabletModeController::SetEnabledForTest(bool enabled) {
...@@ -1211,11 +1238,18 @@ void TabletModeController::OnScreenshotTaken( ...@@ -1211,11 +1238,18 @@ void TabletModeController::OnScreenshotTaken(
} }
bool TabletModeController::CalculateIsInTabletPhysicalState() const { bool TabletModeController::CalculateIsInTabletPhysicalState() const {
if (!HasActiveInternalDisplay()) switch (tablet_mode_behavior_.force_physical_tablet_state) {
case ForcePhysicalTabletState::kDefault:
// Don't return forced result. Check the hardware configuration.
break;
case ForcePhysicalTabletState::kForceTabletMode:
return true;
case ForcePhysicalTabletState::kForceClamshellMode:
return false; return false;
}
if (tablet_mode_behavior_.force_physical_tablet_state) if (!HasActiveInternalDisplay())
return true; return false;
// For updated EC, the tablet mode switch activates at 200 degrees, and // For updated EC, the tablet mode switch activates at 200 degrees, and
// deactivates at 160 degrees. // deactivates at 160 degrees.
......
...@@ -192,6 +192,15 @@ class ASH_EXPORT TabletModeController ...@@ -192,6 +192,15 @@ class ASH_EXPORT TabletModeController
// Returns true if the system tray should have a overview button. // Returns true if the system tray should have a overview button.
bool ShouldShowOverviewButton() const; bool ShouldShowOverviewButton() const;
// ForcePhysicalTabletState is to control physical tablet state. The default
// state is not to force the state, so the tablet-mode controller will observe
// device configurations.
enum class ForcePhysicalTabletState {
kDefault,
kForceTabletMode,
kForceClamshellMode,
};
// Defines how the tablet mode controller controls the // Defines how the tablet mode controller controls the
// tablet mode and its transition between clamshell mode. // tablet mode and its transition between clamshell mode.
// This is defined as a public to define constexpr in cc. // This is defined as a public to define constexpr in cc.
...@@ -201,7 +210,8 @@ class ASH_EXPORT TabletModeController ...@@ -201,7 +210,8 @@ class ASH_EXPORT TabletModeController
bool observe_pointer_device_events = true; bool observe_pointer_device_events = true;
bool block_internal_input_device = false; bool block_internal_input_device = false;
bool always_show_overview_button = false; bool always_show_overview_button = false;
bool force_physical_tablet_state = false; ForcePhysicalTabletState force_physical_tablet_state =
ForcePhysicalTabletState::kDefault;
}; };
private: private:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment