New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 775354 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: 2018-10-22
OS: Chrome
Pri: 3
Type: Bug


Participants' hotlists:
Fixing-touch


Sign in to add a comment

Unexpectedly small number of LOADING_EXTENSION_to_HIDDENs are recorded

Project Member Reported by oka@chromium.org, Oct 17 2017

Issue description

See
https://uma.googleplex.com/p/chrome/histograms/?endDate=20171015&dayCount=7&histograms=VirtualKeyboard.ControllerStateTransition&fixupData=true&showMax=true&filters=platform%2Ceq%2CC%2Cchannel%2Ceq%2C4%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

There are 300K INITIAL_to_LOADING_EXTENSION, but only 100K LOADING_EXTENSION_to_HIDDEN.

I'm not sure why they differs.
Possible explanation is KeyboardController is destructed in loading extension state for 200K times, but why?

 
Cc: yhanada@chromium.org
Labels: Pri-2 Type-Bug
Owner: oka@chromium.org
Status: Assigned (was: Untriaged)
is this still a problem?
Labels: -Pri-2 Pri-3
We still see this unexpected behavior.

Comment 3 by shend@chromium.org, Jun 29 2018

Owner: ----
Status: Available (was: Assigned)
Bulk edit: oka@ is no longer working on virtual keyboard.
Owner: shend@chromium.org
Status: Assigned (was: Available)
Issue is still persisting. It appears that, on all channels, INITIAL -> LOADING occurs 3 times more than LOADING -> HIDDEN.

I believe the problem is that we call ash::EnableKeyboard multiple times during initialization:
- RootWindowController::Init
- SessionController::SetUserSessionOrder
- Shell::OnSessionChanged

ash::EnableKeyboard will reload the virtual keyboard if it's already enabled. Thus, lots of LOADING states will not transition to HIDDEN because the keyboard is reinitialized.

IMO it's better if the numbers add up. I can make DisableKeyboard call ChangeState(INITIAL), so that INITIAL -> LOADING = LOADING -> HIDDEN + LOADING -> INITIAL.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/44f5e9c235afe4ab0cfaaec1c55d323e7f732a2f

commit 44f5e9c235afe4ab0cfaaec1c55d323e7f732a2f
Author: Darren Shen <shend@chromium.org>
Date: Mon Aug 06 07:01:58 2018

[VK] Change to INITIAL state when keyboard is being disabled.

Currently, when we disable the keyboard, we directly reset the state
back to UNKNOWN without logging or validation. This means that in the
UMA metrics for state transitions, the transitions going into a state
does not add up to the transitions going out of a state.

This patch explicitly changes the state back to INITIAL when the
keyboard is disabled. When the keyboard is about to be disabled,
the only valid states are INITIAL, LOADING and HIDDEN. We don't
change if the current state is INITIAL to avoid self-transitions.

Bug: 775354
Change-Id: Idab146e017d179a9086808481f3ed65b6f0dfbcf
Reviewed-on: https://chromium-review.googlesource.com/1163346
Reviewed-by: Yuichiro Hanada <yhanada@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580815}
[modify] https://crrev.com/44f5e9c235afe4ab0cfaaec1c55d323e7f732a2f/chrome/browser/ui/ash/keyboard_controller_browsertest.cc
[modify] https://crrev.com/44f5e9c235afe4ab0cfaaec1c55d323e7f732a2f/ui/keyboard/keyboard_controller.cc

NextAction: 2018-10-22
Status: Fixed (was: Assigned)
Should be fixed now, but hard to verify until M70 hits Beta or something. Will verify in a couple of months.
The NextAction date has arrived: 2018-10-22
Status: Assigned (was: Fixed)
Hmm still doesn't seem to add up...
Oh hmm, another possibility is that aggregation happens weekly so some state transitions are cut off.

We could probably filter by unique users, since each unique user can have at most two "cut off" transitions due to aggregation.

Sign in to add a comment