Add an enum which explicitly designates the keyboard state to keyboard_controller, and stabilize the state machine |
||||||||||
Issue descriptionDesign doc: https://docs.google.com/document/d/1f8h6SNIx7szzzVg9d_Wog6eR6ZF1Jo4zHDPfC2s0UOE/edit# I’m proposing to add an enum which explicitly designates the keyboard state to keyboard_controller. Currently the state is implicit, but introducing and managing the current state explicitly makes the logic clear and the code easier to understand.
,
Jun 1 2017
- Add assertions that expected conditions are met on state transition. - Export UMA if the conditions not met.
,
Jun 1 2017
Let's export success (conditions met) too, because the failure ratio is interesting.
,
Jun 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2 commit 1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2 Author: Keigo Oka <oka@chromium.org> Date: Mon Jun 12 09:01:29 2017 Introduce explicit state to keyboard_controller. Bug: 719905 Change-Id: I14a1ee6b8e66c4f4c98ea4957dde7a7e0d75aad3 Reviewed-on: https://chromium-review.googlesource.com/516866 Commit-Queue: Keigo Oka <oka@chromium.org> Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Commit-Position: refs/heads/master@{#478568} [modify] https://crrev.com/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2/ash/BUILD.gn [modify] https://crrev.com/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2/chrome/browser/ui/ash/keyboard_controller_browsertest.cc [modify] https://crrev.com/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2/ui/keyboard/keyboard_controller.h [modify] https://crrev.com/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2/ui/keyboard/keyboard_controller_observer.h [modify] https://crrev.com/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2/ui/keyboard/keyboard_test_util.cc [modify] https://crrev.com/1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2/ui/keyboard/keyboard_test_util.h
,
Jun 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff commit a55a1b5dec7be5a660fd942f9f6946bf5ac425ff Author: Taiju Tsuiki <tzik@chromium.org> Date: Mon Jun 12 09:40:03 2017 Revert "Introduce explicit state to keyboard_controller." This reverts commit 1fd2f73ecc97f737a4d9b5beaf9e8bc8b39e2ee2. Reason for revert: This CL causes a compile failure. The failure log is: https://build.chromium.org/p/chromium/builders/Win/builds/56026 c:\b\c\b\win_archive\src\ui\keyboard\keyboard_test_util.cc(54): warning C4373: '`anonymous-namespace'::ControllerStateChangeWaiter::OnStateChanged': virtual function overrides 'keyboard::KeyboardControllerObserver::OnStateChanged', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers c:\b\c\b\win_archive\src\ui\keyboard\keyboard_controller_observer.h(34): note: see declaration of 'keyboard::KeyboardControllerObserver::OnStateChanged' Original change's description: > Introduce explicit state to keyboard_controller. > > Bug: 719905 > Change-Id: I14a1ee6b8e66c4f4c98ea4957dde7a7e0d75aad3 > Reviewed-on: https://chromium-review.googlesource.com/516866 > Commit-Queue: Keigo Oka <oka@chromium.org> > Reviewed-by: Mitsuru Oshima <oshima@chromium.org> > Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> > Cr-Commit-Position: refs/heads/master@{#478568} TBR=oshima@chromium.org,bshe@chromium.org,oka@chromium.org,yhanada@chromium.org No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 719905 Change-Id: I14143cde82fa85757be72ee9aed622577fd40dd7 Reviewed-on: https://chromium-review.googlesource.com/530732 Reviewed-by: Taiju Tsuiki <tzik@chromium.org> Commit-Queue: Taiju Tsuiki <tzik@chromium.org> Cr-Commit-Position: refs/heads/master@{#478572} [modify] https://crrev.com/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff/ash/BUILD.gn [modify] https://crrev.com/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff/chrome/browser/ui/ash/keyboard_controller_browsertest.cc [modify] https://crrev.com/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff/ui/keyboard/keyboard_controller.h [modify] https://crrev.com/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff/ui/keyboard/keyboard_controller_observer.h [modify] https://crrev.com/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff/ui/keyboard/keyboard_test_util.cc [modify] https://crrev.com/a55a1b5dec7be5a660fd942f9f6946bf5ac425ff/ui/keyboard/keyboard_test_util.h
,
Jun 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f7a65f731d638e5b904bda190184ed1625735789 commit f7a65f731d638e5b904bda190184ed1625735789 Author: Keigo Oka <oka@chromium.org> Date: Wed Jun 14 07:47:58 2017 Reland "Introduce explicit state to keyboard_controller." fixing build failure. Reland the change reverted with https://chromium-review.googlesource.com/c/530732/ . Fixed the mismatched declarations of OnStateChanged in keyboard_controller_observer and keyboard_test_util. Bug: 719905 Change-Id: I292c1712b046a55273ff23b04d7d75212d4cf978 Reviewed-on: https://chromium-review.googlesource.com/532693 Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Commit-Queue: Keigo Oka <oka@chromium.org> Cr-Commit-Position: refs/heads/master@{#479318} [modify] https://crrev.com/f7a65f731d638e5b904bda190184ed1625735789/ash/BUILD.gn [modify] https://crrev.com/f7a65f731d638e5b904bda190184ed1625735789/chrome/browser/ui/ash/keyboard_controller_browsertest.cc [modify] https://crrev.com/f7a65f731d638e5b904bda190184ed1625735789/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/f7a65f731d638e5b904bda190184ed1625735789/ui/keyboard/keyboard_controller.h [modify] https://crrev.com/f7a65f731d638e5b904bda190184ed1625735789/ui/keyboard/keyboard_controller_observer.h [modify] https://crrev.com/f7a65f731d638e5b904bda190184ed1625735789/ui/keyboard/keyboard_test_util.cc [modify] https://crrev.com/f7a65f731d638e5b904bda190184ed1625735789/ui/keyboard/keyboard_test_util.h
,
Jul 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a80c9a26e634925eb91f4b077e943ac315031aa1 commit a80c9a26e634925eb91f4b077e943ac315031aa1 Author: oka@chromium.org <oka@chromium.org> Date: Wed Jul 05 06:44:50 2017 Add UMAs for keyboard controller state transition. Bug: 719905 Test: Checked chrome:histogram. Change-Id: I1fbef3ae1fed28c3129960ae743c3751439f25b1 Reviewed-on: https://chromium-review.googlesource.com/557319 Commit-Queue: Keigo Oka <oka@chromium.org> Reviewed-by: Alexei Svitkine <asvitkine@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Commit-Position: refs/heads/master@{#484200} [modify] https://crrev.com/a80c9a26e634925eb91f4b077e943ac315031aa1/tools/metrics/histograms/enums.xml [modify] https://crrev.com/a80c9a26e634925eb91f4b077e943ac315031aa1/tools/metrics/histograms/histograms.xml [modify] https://crrev.com/a80c9a26e634925eb91f4b077e943ac315031aa1/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/a80c9a26e634925eb91f4b077e943ac315031aa1/ui/keyboard/keyboard_controller.h
,
Jul 5 2017
UMAs for state transition validity has been checked in. I'll check the UMA after the change hits canary.
,
Jul 18 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5325997f23e67dcda053d4e7b6b56fe6d20ae65d commit 5325997f23e67dcda053d4e7b6b56fe6d20ae65d Author: oka@chromium.org <oka@chromium.org> Date: Tue Jul 18 07:10:11 2017 Make unexpected VK state transition cause crash to help debug Bug: 719905 Test: try Change-Id: I58915c60e7ca582bae77a19e54b9ba5ac6fadf84 Reviewed-on: https://chromium-review.googlesource.com/558165 Commit-Queue: Keigo Oka <oka@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Commit-Position: refs/heads/master@{#487406} [modify] https://crrev.com/5325997f23e67dcda053d4e7b6b56fe6d20ae65d/ui/keyboard/keyboard_controller.cc
,
Jul 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/36f87e57654c712296d4ebdad6533f427be631e8 commit 36f87e57654c712296d4ebdad6533f427be631e8 Author: oka@chromium.org <oka@chromium.org> Date: Tue Jul 25 15:39:04 2017 Refactor and stabilize keyboard controller states machine (part 1/2). Keyboard controller state machine and expected conditions per state: https://docs.google.com/spreadsheets/d/1DnpW0s0TuulJr0IZRCuAhESwZc4ClUSY5GgxtFvNZ4g/edit#gid=0 Bug: 719905 Test: try Change-Id: I3319a49831f3f1c8e77f51c34ef4bdb374818080 Reviewed-on: https://chromium-review.googlesource.com/577434 Commit-Queue: Keigo Oka <oka@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Commit-Position: refs/heads/master@{#489312} [modify] https://crrev.com/36f87e57654c712296d4ebdad6533f427be631e8/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/36f87e57654c712296d4ebdad6533f427be631e8/ui/keyboard/keyboard_controller.h [modify] https://crrev.com/36f87e57654c712296d4ebdad6533f427be631e8/ui/keyboard/keyboard_controller_unittest.cc
,
Jul 26 2017
,
Jul 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5a5448aac9bdb1ccbd49768cd6f43dece1050c9d commit 5a5448aac9bdb1ccbd49768cd6f43dece1050c9d Author: oka@chromium.org <oka@chromium.org> Date: Wed Jul 26 13:42:10 2017 Refactor and stabilize keyboard controller state machine (part 2/2) Bug: 719905 Change-Id: I448ed4eeba37903596937ef4a6c08903f643fd5d Reviewed-on: https://chromium-review.googlesource.com/582709 Commit-Queue: Keigo Oka <oka@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Commit-Position: refs/heads/master@{#489627} [modify] https://crrev.com/5a5448aac9bdb1ccbd49768cd6f43dece1050c9d/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/5a5448aac9bdb1ccbd49768cd6f43dece1050c9d/ui/keyboard/keyboard_controller.h
,
Jul 26 2017
Finished stabilization of Keyboard controller state machine.
,
Jul 26 2017
,
Jul 27 2017
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 5 2017
ketakid@, could you kindly review merge request? Thank you. These CLs fix the issue that virtual keyboard enters in unexpected state.
,
Aug 10 2017
ketakid@ Friendly ping?
,
Aug 16 2017
Approving merge to M61 Chrome OS.
,
Aug 21 2017
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/171e6350011206e819480b02bb6e1d2b8c7d6048 commit 171e6350011206e819480b02bb6e1d2b8c7d6048 Author: oka@chromium.org <oka@chromium.org> Date: Mon Aug 21 15:56:19 2017 Refactor and stabilize keyboard controller states machine (part 1/2). Keyboard controller state machine and expected conditions per state: https://docs.google.com/spreadsheets/d/1DnpW0s0TuulJr0IZRCuAhESwZc4ClUSY5GgxtFvNZ4g/edit#gid=0 TBR=oka@chromium.org (cherry picked from commit 36f87e57654c712296d4ebdad6533f427be631e8) Bug: 719905 Test: try Change-Id: I3319a49831f3f1c8e77f51c34ef4bdb374818080 Reviewed-on: https://chromium-review.googlesource.com/577434 Commit-Queue: Keigo Oka <oka@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#489312} Reviewed-on: https://chromium-review.googlesource.com/624047 Reviewed-by: Keigo Oka <oka@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#698} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/171e6350011206e819480b02bb6e1d2b8c7d6048/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/171e6350011206e819480b02bb6e1d2b8c7d6048/ui/keyboard/keyboard_controller.h [modify] https://crrev.com/171e6350011206e819480b02bb6e1d2b8c7d6048/ui/keyboard/keyboard_controller_unittest.cc
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c6495c4c4868d50164db283bb41d93f8a53415c1 commit c6495c4c4868d50164db283bb41d93f8a53415c1 Author: oka@chromium.org <oka@chromium.org> Date: Mon Aug 21 16:16:13 2017 Refactor and stabilize keyboard controller state machine (part 2/2) TBR=oka@chromium.org (cherry picked from commit 5a5448aac9bdb1ccbd49768cd6f43dece1050c9d) Bug: 719905 Change-Id: I448ed4eeba37903596937ef4a6c08903f643fd5d Reviewed-on: https://chromium-review.googlesource.com/582709 Commit-Queue: Keigo Oka <oka@chromium.org> Reviewed-by: Yuichiro Hanada <yhanada@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#489627} Reviewed-on: https://chromium-review.googlesource.com/624134 Reviewed-by: Keigo Oka <oka@chromium.org> Cr-Commit-Position: refs/branch-heads/3163@{#699} Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528} [modify] https://crrev.com/c6495c4c4868d50164db283bb41d93f8a53415c1/ui/keyboard/keyboard_controller.cc [modify] https://crrev.com/c6495c4c4868d50164db283bb41d93f8a53415c1/ui/keyboard/keyboard_controller.h
,
Jan 22 2018
|
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by oka@chromium.org
, Jun 1 2017Labels: -Pri-3 Pri-1