Add virtualKeyboardPrivate.onKeyboardClosed API.

Add virtualKeyboardPrivate.onKeyboardClosed API

The added API will be fired when the virtual keyboard window has been closed, for example, this can happen when turning off on-screen keyboard or exiting tablet mode.

IME extensions rely on input view's visibility changing event to do some work such as hide candidate window. While, when exiting tablet mode, OS will close input view window immediately but doesn't fire 'unload' event, so the IME extensions won't receive the visibility changed event.

Thus, we add this API to notify the extensions the keyboard closed event.

BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard

Committed: https://crrev.com/169fcec510510b51122341c9471730f8a7b1c029
Cr-Commit-Position: refs/heads/master@{#418603}

[Azure Wei, 2016-07-05 12:04:27]

Description was changed from

==========
Add virtualKeyboardPrivate.onKeyboardDestroyed API.

BUG=619763
TEST=None
==========

to

==========
Add virtualKeyboardPrivate.onKeyboardDestroyed API.

BUG=619763
TEST=None
==========

[Azure Wei, 2016-07-05 12:04:27]

azurewei@chromium.org changed reviewers:
+ isherman@chromium.org, shuchen@chromium.org, sky@chromium.org

[Azure Wei, 2016-07-05 12:04:45]

Please review this cl. Thanks!

[sky, 2016-07-06 16:03:00]

https://codereview.chromium.org/2122873002/diff/20001/chrome/browser/ui/ash/c...
File chrome/browser/ui/ash/chrome_keyboard_ui.cc (right):

https://codereview.chromium.org/2122873002/diff/20001/chrome/browser/ui/ash/c...
chrome/browser/ui/ash/chrome_keyboard_ui.cc:120: std::move(args)));
nit: std::move(args)->base::WrapUnique(new base::ListValue())).

https://codereview.chromium.org/2122873002/diff/20001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/2122873002/diff/20001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.cc:188:
FOR_EACH_OBSERVER(KeyboardControllerObserver, observer_list_,
How come you're only calling OnKeyboardDestroyed() if there is a container_?
Also, why aren't you calling notifiers as the last thing in this function?

[Ilya Sherman, 2016-07-06 22:51:15]

histograms.xml lgtm

[Azure Wei, 2016-07-08 06:35:24]

https://codereview.chromium.org/2122873002/diff/20001/chrome/browser/ui/ash/c...
File chrome/browser/ui/ash/chrome_keyboard_ui.cc (right):

https://codereview.chromium.org/2122873002/diff/20001/chrome/browser/ui/ash/c...
chrome/browser/ui/ash/chrome_keyboard_ui.cc:120: std::move(args)));
On 2016/07/06 16:03:00, sky wrote:
> nit: std::move(args)->base::WrapUnique(new base::ListValue())).

Done.

https://codereview.chromium.org/2122873002/diff/20001/ui/keyboard/keyboard_co...
File ui/keyboard/keyboard_controller.cc (right):

https://codereview.chromium.org/2122873002/diff/20001/ui/keyboard/keyboard_co...
ui/keyboard/keyboard_controller.cc:188:
FOR_EACH_OBSERVER(KeyboardControllerObserver, observer_list_,
On 2016/07/06 16:03:00, sky wrote:
> How come you're only calling OnKeyboardDestroyed() if there is a container_?
> Also, why aren't you calling notifiers as the last thing in this function?

Want to make sure the container_ has been created (VK has ever been shown) to
trigger the event. While, it should not happen that KeyboardController is
created with no container_.
Moved it to the last of the function.

[sky, 2016-07-08 15:43:02]

LGTM

[Azure Wei, 2016-07-12 00:37:27]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-07-12 00:37:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2122873002/#ps40001
(title: "")

[commit-bot: I haz the power, 2016-07-12 00:38:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-12 00:52:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-12 00:52:30]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Azure Wei, 2016-07-12 01:04:21]

azurewei@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[Azure Wei, 2016-07-12 01:04:58]

R+ Devlin for owner of: common/api/virtual_keyboard_private.json

[Shu Chen, 2016-07-14 01:56:16]

lgtm

[Devlin, 2016-07-14 17:44:51]

https://codereview.chromium.org/2122873002/diff/40001/extensions/common/api/v...
File extensions/common/api/virtual_keyboard_private.json (right):

https://codereview.chromium.org/2122873002/diff/40001/extensions/common/api/v...
extensions/common/api/virtual_keyboard_private.json:247: "name":
"onKeyboardDestroyed",
"Destroyed" is kind of a weird name here.  What about just "Closed"?

https://codereview.chromium.org/2122873002/diff/40001/extensions/common/api/v...
extensions/common/api/virtual_keyboard_private.json:250: "parameters": [
unneeded if empty.

[Azure Wei, 2016-07-15 06:08:33]

The CQ bit was checked by azurewei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 06:08:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 07:23:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 07:23:01]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Azure Wei, 2016-07-15 08:38:39]

The CQ bit was checked by azurewei@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-15 08:39:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 09:27:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-15 09:27:36]

Dry run: This issue passed the CQ dry run.

[Azure Wei, 2016-07-15 15:52:31]

https://codereview.chromium.org/2122873002/diff/40001/extensions/common/api/v...
File extensions/common/api/virtual_keyboard_private.json (right):

https://codereview.chromium.org/2122873002/diff/40001/extensions/common/api/v...
extensions/common/api/virtual_keyboard_private.json:247: "name":
"onKeyboardDestroyed",
On 2016/07/14 17:44:51, Devlin wrote:
> "Destroyed" is kind of a weird name here.  What about just "Closed"?

Done.

https://codereview.chromium.org/2122873002/diff/40001/extensions/common/api/v...
extensions/common/api/virtual_keyboard_private.json:250: "parameters": [
On 2016/07/14 17:44:51, Devlin wrote:
> unneeded if empty.

Done.

[Devlin, 2016-07-15 16:00:17]

extensions lgtm with nit.

https://codereview.chromium.org/2122873002/diff/80001/extensions/common/api/v...
File extensions/common/api/virtual_keyboard_private.json (right):

https://codereview.chromium.org/2122873002/diff/80001/extensions/common/api/v...
extensions/common/api/virtual_keyboard_private.json:249: "description": "Fired
when the virtual keyboard window has been closed, for example, turning off
on-screen keyboard, or exiting tablet mode."
nit: make this two sentences
Fired when the virtual keyboard window has been closed. For example, this can
happen when turning off on-screen keyboard or exiting tablet mode.

[Azure Wei, 2016-07-18 02:37:02]

azurewei@chromium.org changed reviewers:
+ jochen@chromium.org

[Azure Wei, 2016-07-18 02:37:30]

+jochen@ for owner of components/arc/ime/arc_ime_service.h

[jochen (gone - plz use gerrit), 2016-07-18 11:32:02]

The CL description doesn't match the actual implementation name.

Please add a test.

https://codereview.chromium.org/2122873002/diff/100001/components/arc/ime/arc...
File components/arc/ime/arc_ime_service.h (right):

https://codereview.chromium.org/2122873002/diff/100001/components/arc/ime/arc...
components/arc/ime/arc_ime_service.h:73: void OnKeyboardClosed() override {}
please move the implementation to the .cc file here and everywhere.

[Azure Wei, 2016-09-14 07:07:52]

Description was changed from

==========
Add virtualKeyboardPrivate.onKeyboardDestroyed API.

BUG=619763
TEST=None
==========

to

==========
Add virtualKeyboardPrivate.onKeyboardClosed API, which will be fired when the
virtual keyboard window has been closed. For example, this can happen when
turning off on-screen keyboard or exiting tablet mode."


BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard
==========

[Azure Wei, 2016-09-14 07:09:12]

On 2016/07/18 11:32:02, jochen wrote:
> The CL description doesn't match the actual implementation name.
> 
> Please add a test.
> 
>
https://codereview.chromium.org/2122873002/diff/100001/components/arc/ime/arc...
> File components/arc/ime/arc_ime_service.h (right):
> 
>
https://codereview.chromium.org/2122873002/diff/100001/components/arc/ime/arc...
> components/arc/ime/arc_ime_service.h:73: void OnKeyboardClosed() override {}
> please move the implementation to the .cc file here and everywhere.

Updated the CL description and added test: KeyboardControllerTest.CloseKeyboard.
PTAL.

[Azure Wei, 2016-09-14 07:09:27]

https://codereview.chromium.org/2122873002/diff/100001/components/arc/ime/arc...
File components/arc/ime/arc_ime_service.h (right):

https://codereview.chromium.org/2122873002/diff/100001/components/arc/ime/arc...
components/arc/ime/arc_ime_service.h:73: void OnKeyboardClosed() override {}
On 2016/07/18 11:32:02, jochen wrote:
> please move the implementation to the .cc file here and everywhere.

Done.

[jochen (gone - plz use gerrit), 2016-09-14 08:54:33]

please reformat the CL description according to
chromium.org/developers/contributing-code#Writing_change_list_descriptions

lgtm with that

[Azure Wei, 2016-09-14 13:08:25]

Description was changed from

==========
Add virtualKeyboardPrivate.onKeyboardClosed API, which will be fired when the
virtual keyboard window has been closed. For example, this can happen when
turning off on-screen keyboard or exiting tablet mode."


BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard
==========

to

==========
Add virtualKeyboardPrivate.onKeyboardClosed API

The added API will be fired when the virtual keyboard window has been closed,
for example, this can happen when turning off on-screen keyboard or exiting
tablet mode.

IME extensions rely on input view's visibility changing event to do some work
such as hide candidate window. While, when exiting tablet mode, OS will close
input view window immediately but doesn't fire 'unload' event, so the IME
extensions won't receive the visibility changed event. 

Thus, we add this API to notify the extensions the keyboard closed event.

BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard
==========

[Azure Wei, 2016-09-14 13:08:36]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-09-14 13:08:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
shuchen@chromium.org, isherman@chromium.org, sky@chromium.org,
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2122873002/#ps120001
(title: "Add test")

[commit-bot: I haz the power, 2016-09-14 13:08:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 13:59:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-14 13:59:47]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Azure Wei, 2016-09-14 15:55:48]

The CQ bit was checked by azurewei@chromium.org

[Azure Wei, 2016-09-14 15:55:49]

The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org,
rdevlin.cronin@chromium.org, isherman@chromium.org, shuchen@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2122873002/#ps140001
(title: "Fix test error.")

[commit-bot: I haz the power, 2016-09-14 15:56:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 17:31:34]

Description was changed from

==========
Add virtualKeyboardPrivate.onKeyboardClosed API

The added API will be fired when the virtual keyboard window has been closed,
for example, this can happen when turning off on-screen keyboard or exiting
tablet mode.

IME extensions rely on input view's visibility changing event to do some work
such as hide candidate window. While, when exiting tablet mode, OS will close
input view window immediately but doesn't fire 'unload' event, so the IME
extensions won't receive the visibility changed event. 

Thus, we add this API to notify the extensions the keyboard closed event.

BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard
==========

to

==========
Add virtualKeyboardPrivate.onKeyboardClosed API

The added API will be fired when the virtual keyboard window has been closed,
for example, this can happen when turning off on-screen keyboard or exiting
tablet mode.

IME extensions rely on input view's visibility changing event to do some work
such as hide candidate window. While, when exiting tablet mode, OS will close
input view window immediately but doesn't fire 'unload' event, so the IME
extensions won't receive the visibility changed event. 

Thus, we add this API to notify the extensions the keyboard closed event.

BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard
==========

[commit-bot: I haz the power, 2016-09-14 17:31:36]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-09-14 17:34:48]

Description was changed from

==========
Add virtualKeyboardPrivate.onKeyboardClosed API

The added API will be fired when the virtual keyboard window has been closed,
for example, this can happen when turning off on-screen keyboard or exiting
tablet mode.

IME extensions rely on input view's visibility changing event to do some work
such as hide candidate window. While, when exiting tablet mode, OS will close
input view window immediately but doesn't fire 'unload' event, so the IME
extensions won't receive the visibility changed event. 

Thus, we add this API to notify the extensions the keyboard closed event.

BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard
==========

to

==========
Add virtualKeyboardPrivate.onKeyboardClosed API

The added API will be fired when the virtual keyboard window has been closed,
for example, this can happen when turning off on-screen keyboard or exiting
tablet mode.

IME extensions rely on input view's visibility changing event to do some work
such as hide candidate window. While, when exiting tablet mode, OS will close
input view window immediately but doesn't fire 'unload' event, so the IME
extensions won't receive the visibility changed event.

Thus, we add this API to notify the extensions the keyboard closed event.

BUG=619763
TEST=KeyboardControllerTest.CloseKeyboard

Committed: https://crrev.com/169fcec510510b51122341c9471730f8a7b1c029
Cr-Commit-Position: refs/heads/master@{#418603}
==========

[commit-bot: I haz the power, 2016-09-14 17:34:50]

Patchset 8 (id:??) landed as
https://crrev.com/169fcec510510b51122341c9471730f8a7b1c029
Cr-Commit-Position: refs/heads/master@{#418603}