TabletModeController refactoring tracking bug |
||||
Issue descriptionOS: Chrome This is a tracking bug for the refactoring of TabletModeController. The logic in TabletModeController is very complicated, which makes it hard to understand and bug-prone. I'm trying to clean it up a little bit and fix potential bugs (hopefully) along the way.
,
Sep 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/43b3f48dfdf7c77f1b75285bff576341286fedf0 commit 43b3f48dfdf7c77f1b75285bff576341286fedf0 Author: Xiaoqian Dai <xdai@chromium.org> Date: Mon Sep 24 20:56:43 2018 TabletModeController refactoring II. 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 laptop mode but the lid is flipped over, i.e., we are in laptop mode because of an attached external mouse. Bug: 887042 Change-Id: I8be03c684265d79ccc3499362a7154eb44c4dec0 Reviewed-on: https://chromium-review.googlesource.com/1237407 Commit-Queue: Xiaoqian Dai <xdai@chromium.org> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Cr-Commit-Position: refs/heads/master@{#593671} [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/BUILD.gn [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/keyboard/virtual_keyboard_controller_unittest.cc [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/wm/tablet_mode/tablet_mode_controller.cc [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/wm/tablet_mode/tablet_mode_controller.h [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/wm/tablet_mode/tablet_mode_controller_test_api.cc [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/wm/tablet_mode/tablet_mode_controller_test_api.h [modify] https://crrev.com/43b3f48dfdf7c77f1b75285bff576341286fedf0/ash/wm/tablet_mode/tablet_mode_controller_unittest.cc
,
Sep 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95f93d44d3caf29694533328e2162c360ed795c2 commit 95f93d44d3caf29694533328e2162c360ed795c2 Author: Xiaoqian Dai <xdai@chromium.org> Date: Wed Sep 26 18:39:30 2018 TabletModeController refactoring III. Changes in this CL: - Remove have_seen_accelerometer_data_ variable. As this variable is not set if the device doesn't have kAshEnableTabletMode flag. - Only observe the display/lid/input device events if the ui mode is allowed to be changed. No functionality is changed in this CL. Bug: 887042 Change-Id: Ic42d4cc0c25667e7aa67efcf26f65032082db126 Reviewed-on: https://chromium-review.googlesource.com/1244289 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Xiaoqian Dai <xdai@chromium.org> Cr-Commit-Position: refs/heads/master@{#594407} [modify] https://crrev.com/95f93d44d3caf29694533328e2162c360ed795c2/ash/public/cpp/ash_switches.cc [modify] https://crrev.com/95f93d44d3caf29694533328e2162c360ed795c2/ash/wm/tablet_mode/tablet_mode_controller.cc [modify] https://crrev.com/95f93d44d3caf29694533328e2162c360ed795c2/ash/wm/tablet_mode/tablet_mode_controller.h [modify] https://crrev.com/95f93d44d3caf29694533328e2162c360ed795c2/ash/wm/tablet_mode/tablet_mode_controller_unittest.cc
,
Sep 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b0a138d4ce2b8890dd858ca67cdb61878752c42 commit 5b0a138d4ce2b8890dd858ca67cdb61878752c42 Author: Xiaoqian Dai <xdai@chromium.org> Date: Wed Sep 26 22:53:56 2018 Fix the regression caused by the refactoring. I accidentally delete one line code that should not be deleted. Bring it back. Bug: 887042 Change-Id: I9a6c4131eb2003888e0edd987ae7362ad847c568 Reviewed-on: https://chromium-review.googlesource.com/1247182 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Xiaoqian Dai <xdai@chromium.org> Cr-Commit-Position: refs/heads/master@{#594508} [modify] https://crrev.com/5b0a138d4ce2b8890dd858ca67cdb61878752c42/ash/wm/tablet_mode/tablet_mode_controller.cc
,
Oct 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e commit 8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e Author: Xiaoqian Dai <xdai@chromium.org> Date: Fri Oct 05 23:07:15 2018 Revert TabletModeController refactoring cls. The refacotring CLs exposed an underlying issue that caused b/117075672. I need to revert all the refactoring for now, and fix the underlying issue first and then reland these refactoring cls. Basically the assumption of "--enable-touchview" USE flag is not longer true, thus Chrome can't reply on this flag to decide whether a device is capable of entering tablet mode. We need to find other ways to do so. Since it's a pure revert, TBR xiyuan@. Revert "TabletModeController refactoring II." This reverts commit 43b3f48dfdf7c77f1b75285bff576341286fedf0. Revert "TabletModeController refactoring III." This reverts commit 95f93d44d3caf29694533328e2162c360ed795c2. Revert "Fix the regression caused by the refactoring." This reverts commit 5b0a138d4ce2b8890dd858ca67cdb61878752c42. Bug:887042, b/117075672 TBR=xiyuan@chromium.org Change-Id: I494a8d05be1fb07bea404cd714b0b1d9689e46d4 Reviewed-on: https://chromium-review.googlesource.com/c/1265996 Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Commit-Queue: Xiaoqian Dai <xdai@chromium.org> Cr-Commit-Position: refs/heads/master@{#597356} [modify] https://crrev.com/8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e/ash/keyboard/virtual_keyboard_controller_unittest.cc [modify] https://crrev.com/8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e/ash/wm/tablet_mode/tablet_mode_controller.cc [modify] https://crrev.com/8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e/ash/wm/tablet_mode/tablet_mode_controller.h [modify] https://crrev.com/8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e/ash/wm/tablet_mode/tablet_mode_controller_test_api.cc [modify] https://crrev.com/8242c9ca8df2ed946fdf1e7f8c8d47a5997e513e/ash/wm/tablet_mode/tablet_mode_controller_test_api.h
,
Oct 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/20a76ff5b7298d2e97f9dcb9faee7915677dda57 commit 20a76ff5b7298d2e97f9dcb9faee7915677dda57 Author: Xiaoqian Dai <xdai@chromium.org> Date: Wed Oct 24 16:48:26 2018 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/1297577 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Xiaoqian Dai <xdai@chromium.org> Cr-Commit-Position: refs/heads/master@{#602370} [modify] https://crrev.com/20a76ff5b7298d2e97f9dcb9faee7915677dda57/ash/keyboard/virtual_keyboard_controller_unittest.cc [modify] https://crrev.com/20a76ff5b7298d2e97f9dcb9faee7915677dda57/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/20a76ff5b7298d2e97f9dcb9faee7915677dda57/ash/wm/tablet_mode/tablet_mode_controller.cc [modify] https://crrev.com/20a76ff5b7298d2e97f9dcb9faee7915677dda57/ash/wm/tablet_mode/tablet_mode_controller.h [modify] https://crrev.com/20a76ff5b7298d2e97f9dcb9faee7915677dda57/ash/wm/tablet_mode/tablet_mode_controller_test_api.cc [modify] https://crrev.com/20a76ff5b7298d2e97f9dcb9faee7915677dda57/ash/wm/tablet_mode/tablet_mode_controller_test_api.h [modify] https://crrev.com/20a76ff5b7298d2e97f9dcb9faee7915677dda57/ash/wm/tablet_mode/tablet_mode_controller_unittest.cc
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68bd491df494b88a7b8a81fc3b4a470bfc432aa9 commit 68bd491df494b88a7b8a81fc3b4a470bfc432aa9 Author: Xiaoqian Dai <xdai@chromium.org> Date: Thu Oct 25 18:14:34 2018 [Merge to M71] 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 TBR=xiyuan@chromium.org (cherry picked from commit 20a76ff5b7298d2e97f9dcb9faee7915677dda57) Change-Id: I93d963cfab2ae14bd1cad19675f0ed4fed88c1b0 Reviewed-on: https://chromium-review.googlesource.com/c/1297577 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Xiaoqian Dai <xdai@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602370} Reviewed-on: https://chromium-review.googlesource.com/c/1299777 Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#328} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} [modify] https://crrev.com/68bd491df494b88a7b8a81fc3b4a470bfc432aa9/ash/keyboard/virtual_keyboard_controller_unittest.cc [modify] https://crrev.com/68bd491df494b88a7b8a81fc3b4a470bfc432aa9/ash/system/overview/overview_button_tray_unittest.cc [modify] https://crrev.com/68bd491df494b88a7b8a81fc3b4a470bfc432aa9/ash/wm/tablet_mode/tablet_mode_controller.cc [modify] https://crrev.com/68bd491df494b88a7b8a81fc3b4a470bfc432aa9/ash/wm/tablet_mode/tablet_mode_controller.h [modify] https://crrev.com/68bd491df494b88a7b8a81fc3b4a470bfc432aa9/ash/wm/tablet_mode/tablet_mode_controller_test_api.cc [modify] https://crrev.com/68bd491df494b88a7b8a81fc3b4a470bfc432aa9/ash/wm/tablet_mode/tablet_mode_controller_test_api.h [modify] https://crrev.com/68bd491df494b88a7b8a81fc3b4a470bfc432aa9/ash/wm/tablet_mode/tablet_mode_controller_unittest.cc
,
Oct 25
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision 68bd491df494b88a7b8a81fc3b4a470bfc432aa9 was merged to refs/branch-heads/3578 branch with no merge approval from a TPM! Please explain why this change was merged to the branch! - AcknowledgeMerge: Notification Required --
,
Oct 25
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68bd491df494b88a7b8a81fc3b4a470bfc432aa9 Commit: 68bd491df494b88a7b8a81fc3b4a470bfc432aa9 Author: xdai@chromium.org Commiter: xdai@chromium.org Date: 2018-10-25 18:14:34 +0000 UTC [Merge to M71] 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 TBR=xiyuan@chromium.org (cherry picked from commit 20a76ff5b7298d2e97f9dcb9faee7915677dda57) Change-Id: I93d963cfab2ae14bd1cad19675f0ed4fed88c1b0 Reviewed-on: https://chromium-review.googlesource.com/c/1297577 Reviewed-by: Xiyuan Xia <xiyuan@chromium.org> Commit-Queue: Xiaoqian Dai <xdai@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#602370} Reviewed-on: https://chromium-review.googlesource.com/c/1299777 Reviewed-by: Xiaoqian Dai <xdai@chromium.org> Cr-Commit-Position: refs/branch-heads/3578@{#328} Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034} |
||||
►
Sign in to add a comment |
||||
Comment 1 by x...@chromium.org
, Sep 20