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

Issue 826391 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocking:
issue 678705



Sign in to add a comment

mash: Remove virtual keyboard code from ChromeLauncherController

Project Member Reported by jamescook@chromium.org, Mar 27 2018

Issue description

See //chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc

This code supports the ash shelf. It has nothing to do with virtual keyboard. I think the reason the code is there is because it observes active user changes.

This code should move elsewhere. It also should stop using ash::Shell to create and destroy the keyboard.

 
Cc: blakeo@chromium.org yhanada@chromium.org
Labels: -Proj-Mustash-Mash
jamescook@ who can we assign this to?
Cc: shend@chromium.org
Someone on keyboard team, maybe yhanada or shend can help you find the right person.

Comment 5 by blakeo@chromium.org, Jun 11 2018

Owner: blakeo@chromium.org
Status: Assigned (was: Untriaged)

Comment 6 by blakeo@chromium.org, Jun 18 2018

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 24 2018

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

commit e419ff3de4a1ec2b0572ea8c79cd9d7175b8cc54
Author: Blake O'Hare <blakeo@chromium.org>
Date: Sun Jun 24 14:11:40 2018

Remove an ash::Shell access from ChromeLauncherController

ChromeLauncherController is resetting the virtual keyboard when the
active user changes. However, the SessionController (which is already
in ash) can have an observer that can listen to the same event. By
making the VirtualKeyboardController a SessionObserver, we can move
the keyboard reset to the other process to make it mustash-friendly.

Bug:  826391 
Change-Id: I70b622105c3bf6dd415b1eee5f0521a38b6d1bf1
Reviewed-on: https://chromium-review.googlesource.com/1100672
Commit-Queue: Blake O'Hare <blakeo@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569926}
[modify] https://crrev.com/e419ff3de4a1ec2b0572ea8c79cd9d7175b8cc54/ash/keyboard/virtual_keyboard_controller.cc
[modify] https://crrev.com/e419ff3de4a1ec2b0572ea8c79cd9d7175b8cc54/ash/keyboard/virtual_keyboard_controller.h
[modify] https://crrev.com/e419ff3de4a1ec2b0572ea8c79cd9d7175b8cc54/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc

Labels: Proj-Mustash
Labels: -Proj-Mustash Proj-Mash-MultiProcess
Owner: shend@chromium.org
Owner: steve...@chromium.org
Status: Fixed (was: Started)
I think Steven's recent vk mash work makes this obsolete?
Status: Started (was: Fixed)
Perhaps ChromeLauncherController::SetVirtualKeyboardBehaviorFromPrefs() needs further attention?
https://cs.chromium.org/chromium/src/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc?rcl=d17bbb3a7efcc84ff02285f2d51484cd081245f1&l=994
Ah sorry, I assumed that the original motivation was to remove dependency on //ui/keyboard or something (for mash). If it's to remove all dependencies on "virtual keyboard", then yeah we gotta move that function.
Status: Fixed (was: Started)
Project Member

Comment 15 by bugdroid1@chromium.org, Dec 5

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

commit 14e2ef4bd1a4b9c599cbbf1b9264612caf547423
Author: Steven Bennetts <stevenjb@chromium.org>
Date: Wed Dec 05 23:08:10 2018

Keyboard: move pref observer to ChromeKeyboardControllerClient

The keyboard pref observer does not have any particular reason to be
associated with the launcher controller; we should move it.

This moves SetVirtualKeyboardBehaviorFromPrefs from
ChromeLauncherController to ChromeKeyboardControllerClient

Bug:  826391 
Change-Id: I1b8680ea7c29e902763c3fce76590c55b1042928
Reviewed-on: https://chromium-review.googlesource.com/c/1361643
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614163}
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/ui/ash/chrome_keyboard_controller_client.cc
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/ui/ash/chrome_keyboard_controller_client.h
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/ui/ash/chrome_keyboard_controller_client_test_helper.cc
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/ui/ash/chrome_keyboard_ui_unittest.cc
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/ui/ash/chrome_keyboard_web_contents_unittest.cc
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc
[modify] https://crrev.com/14e2ef4bd1a4b9c599cbbf1b9264612caf547423/chrome/browser/ui/ash/launcher/chrome_launcher_controller.h

Sign in to add a comment