athena: Add support for virtual keyboard.

athena: Add support for virtual keyboard.

The virtual keyboard used on Chrome OS (in ash) is provided by an
extension. For athena, we cannot yet use the extension version of the
keyboard. As a short-term workaround, this patch introduces a web-ui
version of the keyboard for initial use. This patch provides a
simplistic implementation of some of the virtualKeyboardPrivate API
(sendKeyEvent, hideKeyboard) over WebUI.

This patch installs a webui controller (VKWebUIController) for the
keyboard URL (chrome://keyboard), and a message-handler (VKMessageHandler)
to process the incoming messages from the keyboard. These are installed
in the browser process. In the renderer process, this patch installs a
RenderViewObserver implementation (VirtualKeyboardBindingsImpl) which
provides the necessary bindings for the virtualKeyboardPrivate API
(VKBindings), and generates the appropriate webui messages by calling
chrome.send(). This lets us use the same HTML/JS/Polymer version of the
keyboard as in Chrome OS under ash/chrome.

For the keyboard to be completely functional, an implementation of
onTextInputBoxFocused will also be necessary. That will be provided in a
follow up CL.

BUG=380125
R=ben@chromium.org, bshe@chromium.org, oshima@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278687

[bshe, 2014-06-18 17:33:43]

webui implementation lgtm

https://codereview.chromium.org/335793005/diff/60001/athena/athena.gyp
File athena/athena.gyp (right):

https://codereview.chromium.org/335793005/diff/60001/athena/athena.gyp#newcode47
athena/athena.gyp:47: 'input/accelerator_manager_impl.h',
nit: order is wrong.

[sadrul, 2014-06-18 19:35:08]

https://codereview.chromium.org/335793005/diff/60001/athena/athena.gyp
File athena/athena.gyp (right):

https://codereview.chromium.org/335793005/diff/60001/athena/athena.gyp#newcode47
athena/athena.gyp:47: 'input/accelerator_manager_impl.h',
On 2014/06/18 17:33:42, bshe wrote:
> nit: order is wrong.

Done.

[sadrul, 2014-06-18 19:36:21]

+ben@ for virtual_keyboard/DEPS changes

[oshima, 2014-06-18 20:14:36]

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
File athena/virtual_keyboard/virtual_keyboard_bindings_impl.cc (right):

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_bindings_impl.cc:36: namespace gin {
why using gin namespace here? I guess you have a reason, but I'm curious.

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
File athena/virtual_keyboard/virtual_keyboard_manager_impl.cc (right):

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:28: class
FillLayoutManager : public aura::LayoutManager {
can you move the FillLaout from screen/screen_manager_impl.cc to common/ and use
it?

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:107: instance = NULL;
don't you have to do any cleanup where?

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:118: new
BasicKeyboardControllerProxy(browser_context_, root_window_)));
can you simply use container_->GetRootWindow() instead?

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
File athena/virtual_keyboard/vk_webui_controller.cc (right):

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/vk_webui_controller.cc:91: return
Singleton<VKWebUIControllerFactory>::get();
I'm not fan of Singleton class/template but ok as this has no state

[Ben Goodger (Google), 2014-06-18 20:46:56]

rather than rolling your own bindings and having to depend on third_party/WebKit
directly, can you use Mojo for this webui as we have been able to do in some
other places (e.g. chrome:omnibox).

[sadrul, 2014-06-18 20:57:53]

On 2014/06/18 20:46:56, Ben Goodger (Google) wrote:
> rather than rolling your own bindings and having to depend on
third_party/WebKit
> directly, can you use Mojo for this webui as we have been able to do in some
> other places (e.g. chrome:omnibox).

I considered that at first. But my understanding is it would be necessary to
update the JS code accordingly. We are trying to avoid that, since using the
same keyboard as on regular Chrome OS would be more beneficial.

If it is actually possible to provide the bindings without changing the JS code,
let me know, and I will look into it farther (although I would like to land this
incarnation of the implementation so that we can start using this on device
while I mojo this up).

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
File athena/virtual_keyboard/virtual_keyboard_bindings_impl.cc (right):

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_bindings_impl.cc:36: namespace gin {
On 2014/06/18 20:14:35, oshima wrote:
> why using gin namespace here? I guess you have a reason, but I'm curious.

This is an implementation of the Converter template-class defined in gin
namespace, and the code that uses the converter expects it to be in gin
namespace. (see, for another example, mojo/apps/js/bindings/gl/context.cc)

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
File athena/virtual_keyboard/virtual_keyboard_manager_impl.cc (right):

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:28: class
FillLayoutManager : public aura::LayoutManager {
On 2014/06/18 20:14:35, oshima wrote:
> can you move the FillLaout from screen/screen_manager_impl.cc to common/ and
use
> it?

Done.

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:107: instance = NULL;
On 2014/06/18 20:14:35, oshima wrote:
> don't you have to do any cleanup where?

What kind of cleanup do you think could be useful here? The keyboard-controller
is destroyed from the destructor because it's a scoped_ptr<>. The
WebUIControllerFactory installed using ::RegisterFactory() cannot be
unregistered (there's only a ::UnregisterFactoryForTesting()).

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:118: new
BasicKeyboardControllerProxy(browser_context_, root_window_)));
On 2014/06/18 20:14:35, oshima wrote:
> can you simply use container_->GetRootWindow() instead?

Done.

[oshima, 2014-06-18 21:11:15]

lgtm with nits, if ben is ok with this non-mojo approach.

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
File athena/virtual_keyboard/virtual_keyboard_manager_impl.cc (right):

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:107: instance = NULL;
On 2014/06/18 20:57:52, sadrul wrote:
> On 2014/06/18 20:14:35, oshima wrote:
> > don't you have to do any cleanup where?
> 
> What kind of cleanup do you think could be useful here? The
keyboard-controller
> is destroyed from the destructor because it's a scoped_ptr<>. The
> WebUIControllerFactory installed using ::RegisterFactory() cannot be
> unregistered (there's only a ::UnregisterFactoryForTesting()).

Don't you have to do

keyboard::KeyboardController::ResetInstance(NULL)

? I'm not familiar with VK code, so I may be wrong, but ash/shell seems to be
doing this in dtor.

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
File athena/common/fill_layout_manager.cc (right):

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
athena/common/fill_layout_manager.cc:7: #include "base/logging.h"
do you need this?

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
File athena/common/fill_layout_manager.h (right):

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
athena/common/fill_layout_manager.h:13: class ATHENA_EXPORT FillLayoutManager :
public aura::LayoutManager {
I think you don't need export, but you may keep it if I'm wrong.

[sadrul, 2014-06-18 21:19:10]

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
File athena/virtual_keyboard/virtual_keyboard_manager_impl.cc (right):

https://codereview.chromium.org/335793005/diff/80001/athena/virtual_keyboard/...
athena/virtual_keyboard/virtual_keyboard_manager_impl.cc:107: instance = NULL;
On 2014/06/18 21:11:15, oshima wrote:
> On 2014/06/18 20:57:52, sadrul wrote:
> > On 2014/06/18 20:14:35, oshima wrote:
> > > don't you have to do any cleanup where?
> > 
> > What kind of cleanup do you think could be useful here? The
> keyboard-controller
> > is destroyed from the destructor because it's a scoped_ptr<>. The
> > WebUIControllerFactory installed using ::RegisterFactory() cannot be
> > unregistered (there's only a ::UnregisterFactoryForTesting()).
> 
> Don't you have to do
> 
> keyboard::KeyboardController::ResetInstance(NULL)
> 
> ? I'm not familiar with VK code, so I may be wrong, but ash/shell seems to be
> doing this in dtor.

Yes, good catch! Done. Thanks

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
File athena/common/fill_layout_manager.cc (right):

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
athena/common/fill_layout_manager.cc:7: #include "base/logging.h"
On 2014/06/18 21:11:15, oshima wrote:
> do you need this?

This is necessary for the DCHECK in line 14

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
File athena/common/fill_layout_manager.h (right):

https://codereview.chromium.org/335793005/diff/100001/athena/common/fill_layo...
athena/common/fill_layout_manager.h:13: class ATHENA_EXPORT FillLayoutManager :
public aura::LayoutManager {
On 2014/06/18 21:11:15, oshima wrote:
> I think you don't need export, but you may keep it if I'm wrong.

This is necessary since this is in the athena_lib component, but it is also used
from the athena_content_lib component (since the virtual-keyboard is in there
because of its content dependency).

[sadrul, 2014-06-19 22:16:20]

On 2014/06/18 20:57:53, sadrul wrote:
> On 2014/06/18 20:46:56, Ben Goodger (Google) wrote:
> > rather than rolling your own bindings and having to depend on
> third_party/WebKit
> > directly, can you use Mojo for this webui as we have been able to do in some
> > other places (e.g. chrome:omnibox).
> 
> I considered that at first. But my understanding is it would be necessary to
> update the JS code accordingly. We are trying to avoid that, since using the
> same keyboard as on regular Chrome OS would be more beneficial.
> 
> If it is actually possible to provide the bindings without changing the JS
code,
> let me know, and I will look into it farther (although I would like to land
this
> incarnation of the implementation so that we can start using this on device
> while I mojo this up).

I have a WIP CL to provide the bindings using Mojo here:
https://codereview.chromium.org/328303008/ There's opportunity for some
refactoring in there, and some missing pieces to be filled in. I expect it to be
a few more days before it's ready for review. Do you think we can go ahead with
landing this CL while I cleanup the mojo CL?

[Ben Goodger (Google), 2014-06-20 05:38:43]

yes lgtm

[sadrul, 2014-06-20 12:53:51]

On 2014/06/20 05:38:43, Ben Goodger (Google) wrote:
> yes lgtm

Cool. Thanks!

[sadrul, 2014-06-20 13:01:21]

Committed patchset #6 manually as r278687 (presubmit successful).