Fix the issue that whiskers keyboard is disabled incorrectly.

Refactoring of the internal input events blocker in
TabletModeController.

The input events should only be blocked if 1) we're currently in tablet
mode or 2) we're currently in clamshell mode but the lid is flipped over,
i.e., we are in laptop mode because of an attached external mouse.

Bug: b/118049922, 887042
Test: Newly added test and existing tests passed.
      Also manually tested on convertible/clamshell/tablet devices

Change-Id: I93d963cfab2ae14bd1cad19675f0ed4fed88c1b0
Reviewed-on: https://chromium-review.googlesource.com/c/1297577Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602370}

ash/keyboard/virtual_keyboard_controller_unittest.cc

@@ -367,7 +367,7 @@ TEST_F(VirtualKeyboardControllerAutoTest, EnabledDuringTabletMode) {
   TabletModeControllerTestApi().EnterTabletMode();
   ASSERT_TRUE(keyboard_controller_->IsKeyboardEnableRequested());
   // Toggle tablet mode off.
-  TabletModeControllerTestApi().LeaveTabletMode(false);
+  TabletModeControllerTestApi().LeaveTabletMode();
   ASSERT_FALSE(keyboard_controller_->IsKeyboardEnableRequested());
 }
 
@@ -410,7 +410,7 @@ TEST_F(VirtualKeyboardControllerAutoTest, SuppressedInTabletMode) {
   ASSERT_TRUE(notified());
   ASSERT_FALSE(IsVirtualKeyboardSuppressed());
   // Toggle tablet mode oFF.
-  TabletModeControllerTestApi().LeaveTabletMode(false);
+  TabletModeControllerTestApi().LeaveTabletMode();
   ASSERT_FALSE(keyboard_controller_->IsKeyboardEnableRequested());
 }
 

ash/system/overview/overview_button_tray_unittest.cc

@@ -113,7 +113,7 @@ TEST_F(OverviewButtonTrayTest, TabletModeObserverOnTabletModeToggled) {
   TabletModeControllerTestApi().EnterTabletMode();
   EXPECT_TRUE(GetTray()->visible());
 
-  TabletModeControllerTestApi().LeaveTabletMode(false);
+  TabletModeControllerTestApi().LeaveTabletMode();
   EXPECT_FALSE(GetTray()->visible());
 }
 
@@ -303,7 +303,7 @@ TEST_F(OverviewButtonTrayTest, VisibilityChangesForSystemModalWindow) {
   ASSERT_TRUE(Shell::IsSystemModalWindowOpen());
   TabletModeControllerTestApi().EnterTabletMode();
   EXPECT_TRUE(GetTray()->visible());
-  TabletModeControllerTestApi().LeaveTabletMode(false);
+  TabletModeControllerTestApi().LeaveTabletMode();
   EXPECT_FALSE(GetTray()->visible());
 }
 

ash/wm/tablet_mode/tablet_mode_controller.cc

@@ -204,6 +204,8 @@ void TabletModeController::EnableTabletModeWindowManager(bool should_enable) {
     if (client_)  // Null at startup and in tests.
       client_->OnTabletModeToggled(false);
   }
+
+  UpdateInternalMouseAndKeyboardEventBlocker();
 }
 
 bool TabletModeController::IsTabletModeWindowManagerEnabled() const {
@@ -265,7 +267,7 @@ void TabletModeController::OnDisplayConfigurationChanged() {
   if (!display::Display::HasInternalDisplay() ||
       !Shell::Get()->display_manager()->IsActiveDisplayId(
           display::Display::InternalDisplayId())) {
-    AttemptLeaveTabletMode(false);
+    AttemptLeaveTabletMode();
   } else if (tablet_mode_switch_is_on_ && !IsTabletModeWindowManagerEnabled()) {
     // The internal display has returned, as we are exiting docked mode.
     // The device is still in tablet mode, so trigger tablet mode, as this
@@ -345,7 +347,7 @@ void TabletModeController::LidEventReceived(
   lid_is_closed_ = !open;
 
   if (!tablet_mode_switch_is_on_)
-    AttemptLeaveTabletMode(false);
+    AttemptLeaveTabletMode();
 }
 
 void TabletModeController::TabletModeEventReceived(
@@ -369,8 +371,13 @@ void TabletModeController::TabletModeEventReceived(
     AttemptEnterTabletMode();
   } else if (!on && IsTabletModeWindowManagerEnabled() &&
              !can_detect_lid_angle_) {
-    AttemptLeaveTabletMode(false);
+    AttemptLeaveTabletMode();
   }
+
+  // Even if we do not change its ui mode, we should update its input device
+  // blocker as tablet mode events may come in because of the lid angle/or folio
+  // keyboard state changes but ui mode might still stay the same.
+  UpdateInternalMouseAndKeyboardEventBlocker();
 }
 
 void TabletModeController::SuspendImminent(
@@ -454,7 +461,7 @@ void TabletModeController::HandleHingeRotation(
 
   // Toggle tablet mode on or off when corresponding thresholds are passed.
   if (is_angle_stable && lid_angle_ <= kExitTabletModeAngle) {
-    AttemptLeaveTabletMode(false);
+    AttemptLeaveTabletMode();
   } else if (!lid_is_closed_ && lid_angle_ >= kEnterTabletModeAngle &&
              (is_angle_stable || CanUseUnstableLidAngle())) {
     AttemptEnterTabletMode();
@@ -494,37 +501,19 @@ bool TabletModeController::CanEnterTabletMode() {
 }
 
 void TabletModeController::AttemptEnterTabletMode() {
-  event_blocker_.reset();
-  event_blocker_ = CreateScopedDisableInternalMouseAndKeyboard();
-  for (auto& observer : tablet_mode_observers_)
-    observer.OnTabletModeEventsBlockingChanged();
-
-  if (IsTabletModeWindowManagerEnabled())
-    return;
-
-  should_enter_tablet_mode_ = true;
-
-  if (has_external_mouse_)
+  if (IsTabletModeWindowManagerEnabled() || has_external_mouse_) {
+    UpdateInternalMouseAndKeyboardEventBlocker();
     return;
+  }
 
   EnableTabletModeWindowManager(true);
 }
 
-void TabletModeController::AttemptLeaveTabletMode(
-    bool called_by_device_update) {
-  // Do not unlock internal keyboard if we enter clamshell by plugging in an
-  // external mouse.
-  if (!called_by_device_update) {
-    event_blocker_.reset();
-    for (auto& observer : tablet_mode_observers_)
-      observer.OnTabletModeEventsBlockingChanged();
-  }
-
-  if (!IsTabletModeWindowManagerEnabled())
+void TabletModeController::AttemptLeaveTabletMode() {
+  if (!IsTabletModeWindowManagerEnabled()) {
+    UpdateInternalMouseAndKeyboardEventBlocker();
     return;
-
-  if (!called_by_device_update)
-    should_enter_tablet_mode_ = false;
+  }
 
   EnableTabletModeWindowManager(false);
 }
@@ -594,17 +583,18 @@ void TabletModeController::HandleMouseAddedOrRemoved() {
 
   has_external_mouse_ = has_external_mouse;
 
-  // Try to tablet mode if we are already in tablet mode and
-  // |has_external_mouse| is true.
+  // Enter clamshell mode whenever an external mouse is attached.
   if (has_external_mouse) {
-    AttemptLeaveTabletMode(true);
-    return;
-  }
-
-  // Try to enter tablet mode if |has_external_mouse| is false and we
-  // are in an orientation that should be tablet mode.
-  if (should_enter_tablet_mode_)
+    AttemptLeaveTabletMode();
+  } else if (LidAngleIsInTabletModeRange() || tablet_mode_switch_is_on_) {
+    // If there is no external mouse, only enter tablet mode if 1) the lid angle
+    // can be detected and is in tablet mode angle range. or 2) if the lid angle
+    // can't be detected (e.g., tablet device or clamshell device) and
+    // |tablet_mode_switch_is_on_| is true (it can only happen for tablet device
+    // as |tablet_mode_switch_is_on_| should never be true for a clamshell
+    // device).
     AttemptEnterTabletMode();
+  }
 }
 
 void TabletModeController::UpdateBluetoothDevice(
@@ -622,4 +612,34 @@ void TabletModeController::UpdateBluetoothDevice(
   HandleMouseAddedOrRemoved();
 }
 
+void TabletModeController::UpdateInternalMouseAndKeyboardEventBlocker() {
+  bool should_block_internal_events = false;
+  if (IsTabletModeWindowManagerEnabled()) {
+    // If we are currently in tablet mode, the internal input events should
+    // always be blocked.
+    should_block_internal_events = true;
+  } else if (LidAngleIsInTabletModeRange() || tablet_mode_switch_is_on_) {
+    // If we are currently in clamshell mode, the intenral input events should
+    // only be blocked if the current lid angle belongs to tablet mode angle
+    // or |tablet_mode_switch_is_on_| is true.
+    should_block_internal_events = true;
+  }
+
+  if (should_block_internal_events == AreInternalInputDeviceEventsBlocked())
+    return;
+
+  if (should_block_internal_events)
+    event_blocker_ = CreateScopedDisableInternalMouseAndKeyboard();
+  else
+    event_blocker_.reset();
+
+  for (auto& observer : tablet_mode_observers_)
+    observer.OnTabletModeEventsBlockingChanged();
+}
+
+bool TabletModeController::LidAngleIsInTabletModeRange() {
+  return can_detect_lid_angle_ && !lid_is_closed_ &&
+         lid_angle_ >= kEnterTabletModeAngle;
+}
+
 }  // namespace ash

ash/wm/tablet_mode/tablet_mode_controller.h

@@ -169,12 +169,13 @@ class ASH_EXPORT TabletModeController
   // tablet mode becomes enabled.
   bool CanEnterTabletMode();
 
-  // Attempts to enter tablet mode and locks the internal keyboard and touchpad.
+  // Attempts to enter tablet mode and updates the internal keyboard and
+  // touchpad.
   void AttemptEnterTabletMode();
 
-  // Attempts to exit tablet mode and unlocks the internal keyboard and touchpad
-  // if |called_by_device_update| is false.
-  void AttemptLeaveTabletMode(bool called_by_device_update);
+  // Attempts to exit tablet mode and updates the internal keyboard and
+  // touchpad.
+  void AttemptLeaveTabletMode();
 
   // Record UMA stats tracking TabletMode usage. If |type| is
   // TABLET_MODE_INTERVAL_INACTIVE, then record that TabletMode has been
@@ -207,6 +208,17 @@ class ASH_EXPORT TabletModeController
   // changes.
   void UpdateBluetoothDevice(device::BluetoothDevice* device);
 
+  // Update the internal mouse and keyboard event blocker |event_blocker_|
+  // according to current configuration. The internal input events should be
+  // blocked if 1) we are currently in tablet mode or 2) we are currently in
+  // laptop mode but the lid is flipped over (i.e., we are in laptop mode
+  // because of an external attached mouse).
+  void UpdateInternalMouseAndKeyboardEventBlocker();
+
+  // Returns true if the current lid angle can be detected and is in tablet mode
+  // angle range.
+  bool LidAngleIsInTabletModeRange();
+
   // The maximized window manager (if enabled).
   std::unique_ptr<TabletModeWindowManager> tablet_mode_window_manager_;
 
@@ -253,11 +265,6 @@ class ASH_EXPORT TabletModeController
   // not enter tablet mode if this is true.
   bool has_external_mouse_ = false;
 
-  // Tracks if the device would enter tablet mode, but does not because of a
-  // attached external mouse. If the external mouse is detached and this is
-  // true, we will enter tablet mode.
-  bool should_enter_tablet_mode_ = false;
-
   // Tracks smoothed accelerometer data over time. This is done when the hinge
   // is approaching vertical to remove abrupt acceleration that can lead to
   // incorrect calculations of hinge angles.

ash/wm/tablet_mode/tablet_mode_controller_test_api.cc

@@ -21,9 +21,8 @@ void TabletModeControllerTestApi::EnterTabletMode() {
   tablet_mode_controller_->AttemptEnterTabletMode();
 }
 
-void TabletModeControllerTestApi::LeaveTabletMode(
-    bool called_by_device_update) {
-  tablet_mode_controller_->AttemptLeaveTabletMode(called_by_device_update);
+void TabletModeControllerTestApi::LeaveTabletMode() {
+  tablet_mode_controller_->AttemptLeaveTabletMode();
 }
 
 void TabletModeControllerTestApi::AttachExternalMouse() {

ash/wm/tablet_mode/tablet_mode_controller_test_api.h

@@ -29,7 +29,7 @@ class TabletModeControllerTestApi {
   // Enters or exits tablet mode. Use these instead when stuff such as tray
   // visibilty depends on the event blocker instead of the actual tablet mode.
   void EnterTabletMode();
-  void LeaveTabletMode(bool called_by_device_update);
+  void LeaveTabletMode();
 
   // Called to attach an external mouse. If we're currently in tablet mode,
   // tablet mode will be ended because of this.

ash/wm/tablet_mode/tablet_mode_controller_unittest.cc

@@ -783,6 +783,119 @@ TEST_F(TabletModeControllerTest, ExternalMouseInLaptopMode) {
   EXPECT_FALSE(AreEventsBlocked());
 }
 
+// Test that the ui mode and input event blocker should be both correctly
+// updated when there is a change in external mouse and lid angle.
+TEST_F(TabletModeControllerTest, ExternalMouseWithLidAngleTest) {
+  // Set the current list of devices to empty so that they don't interfere
+  // with the test.
+  base::RunLoop().RunUntilIdle();
+  ws::InputDeviceClientTestApi().SetMouseDevices({});
+  base::RunLoop().RunUntilIdle();
+
+  // Start in laptop mode.
+  OpenLidToAngle(30.0f);
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+
+  // Attach external mouse doesn't change the mode.
+  ws::InputDeviceClientTestApi().SetMouseDevices(
+      {ui::InputDevice(3, ui::InputDeviceType::INPUT_DEVICE_USB, "mouse")});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+
+  // Now flip the device to tablet mode angle. The device should stay in
+  // clamshell mode because of the external mouse. But the internal input events
+  // should be blocked.
+  OpenLidToAngle(300.0f);
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_TRUE(AreEventsBlocked());
+
+  // Remove the external mouse should enter tablet mode now. The internal input
+  // events should still be blocked.
+  ws::InputDeviceClientTestApi().SetMouseDevices({});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(IsTabletModeStarted());
+  EXPECT_TRUE(AreEventsBlocked());
+
+  // Attach the mouse again should enter clamshell mode again.
+  ws::InputDeviceClientTestApi().SetMouseDevices(
+      {ui::InputDevice(3, ui::InputDeviceType::INPUT_DEVICE_USB, "mouse")});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_TRUE(AreEventsBlocked());
+
+  // Flip the device back to clamshell angle. The device should stay in
+  // clamshell mode and the internal input events should not be blocked.
+  OpenLidToAngle(30.0f);
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+
+  // Now remove the mouse. The device should stay in clamshell mode and the
+  // internal events should not be blocked.
+  ws::InputDeviceClientTestApi().SetMouseDevices({});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+}
+
+// Test that the ui mode and input event blocker should be both correctly
+// updated when there is a change in external mouse and tablet mode switch
+// value.
+TEST_F(TabletModeControllerTest, ExternalMouseWithTabletModeSwithTest) {
+  // Set the current list of devices to empty so that they don't interfere
+  // with the test.
+  base::RunLoop().RunUntilIdle();
+  ws::InputDeviceClientTestApi().SetMouseDevices({});
+  base::RunLoop().RunUntilIdle();
+
+  // Start in laptop mode.
+  SetTabletMode(false);
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+
+  // Attach external mouse doesn't change the mode.
+  ws::InputDeviceClientTestApi().SetMouseDevices(
+      {ui::InputDevice(3, ui::InputDeviceType::INPUT_DEVICE_USB, "mouse")});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+
+  // Now set tablet mode switch value to true. The device should stay in
+  // clamshell mode because of the external mouse. But the internal input events
+  // should be blocked.
+  SetTabletMode(true);
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_TRUE(AreEventsBlocked());
+
+  // Remove the external mouse should enter tablet mode now. The internal input
+  // events should still be blocked.
+  ws::InputDeviceClientTestApi().SetMouseDevices({});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_TRUE(IsTabletModeStarted());
+  EXPECT_TRUE(AreEventsBlocked());
+
+  // Attach the mouse again should enter clamshell mode again.
+  ws::InputDeviceClientTestApi().SetMouseDevices(
+      {ui::InputDevice(3, ui::InputDeviceType::INPUT_DEVICE_USB, "mouse")});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_TRUE(AreEventsBlocked());
+
+  // Set tablet mode switch value to false. The device should stay in
+  // clamshell mode and the internal input events should not be blocked.
+  SetTabletMode(false);
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+
+  // Now remove the mouse. The device should stay in clamshell mode and the
+  // internal events should not be blocked.
+  ws::InputDeviceClientTestApi().SetMouseDevices({});
+  base::RunLoop().RunUntilIdle();
+  EXPECT_FALSE(IsTabletModeStarted());
+  EXPECT_FALSE(AreEventsBlocked());
+}
+
 class TabletModeControllerForceTabletModeTest
     : public TabletModeControllerTest {
  public: