Split ui/base/ime into a new component

Split ui/base/ime into a new component (reland with windows GN fix)

The IME code in ui/base/ime depends on ui/ozone to support keyboard
control (via the InputController interface). This dependency is
currently missing from the build.

Unfortunately, we cannot simply add this dependency, because ui/ozone
depends on code from ui/base/cursor and ui/base/resource
(for cursor bitmaps).

Break this cycle by componentizing ui/base/ime & add a dependency
from the new ui/base/ime component to ui/ozone.

This also helps clean things up a bit because iOS builds ui/base
but not ui/base/ime. So there is already a fault line here.

The handful of IME tests are left as part of ui_base_unittests.

BUG=445627
TEST=gn gen out_gn_ozone --args='os="chromeos" use_ozone=true is_component_build=true' &&
  ninja -C out_gn_ozone chrome ozone_unittests (along with other needed changes)
TBR=jam

Committed: https://crrev.com/2321ca1b99c1a8e9a90e48a9cd50f6446a7e5e82
Cr-Commit-Position: refs/heads/master@{#314815}

Committed: https://crrev.com/1c36facc53f4a0038f2e2e482c78a65438cc8ec4
Cr-Commit-Position: refs/heads/master@{#314860}

[spang, 2015-02-04 20:11:45]

spang@chromium.org changed reviewers:
+ sky@chromium.org, thakis@chromium.org

[Nico, 2015-02-04 20:29:36]

Thanks much for doing this. Mostly lgtm. I'm confused that you're adding a dep
on both ui/base and base/ui/ime in a few places – I would've expected that you
need to add deps on base/ui/ime in places that already depend on ui/base, but
adding both looks weird to me.

https://codereview.chromium.org/889323003/diff/40001/extensions/shell/BUILD.gn
File extensions/shell/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/extensions/shell/BUILD.g...
extensions/shell/BUILD.gn:53: "//ui/base",
why is this needed all of a sudden?

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn
File ui/base/ime/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn#new...
ui/base/ime/BUILD.gn:94: "text_input_type.h",
add ui_base_ime_export.h

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/chromeos/moc...
File ui/base/ime/chromeos/mock_ime_engine_handler.h (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/chromeos/moc...
ui/base/ime/chromeos/mock_ime_engine_handler.h:14: class UI_BASE_IME_EXPORT
MockIMEEngineHandler : public IMEEngineHandlerInterface {
nit: > 80 cols (run `git cl format` on your branch, it'll fix all these)

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/mock_input_m...
File ui/base/ime/mock_input_method.h (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/mock_input_m...
ui/base/ime/mock_input_method.h:26: class UI_BASE_IME_EXPORT MockInputMethod :
NON_EXPORTED_BASE(public InputMethod) {
same nit

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/ui_base_ime.gyp
File ui/base/ime/ui_base_ime.gyp (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/ui_base_ime....
ui/base/ime/ui_base_ime.gyp:115: 'text_input_type.h',
add new _export file here too

https://codereview.chromium.org/889323003/diff/40001/ui/keyboard/BUILD.gn
File ui/keyboard/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/ui/keyboard/BUILD.gn#new...
ui/keyboard/BUILD.gn:136: "//ui/base/ime",
this too looks surprising

[spang, 2015-02-04 21:14:07]

New patchsets have been uploaded after l-g-t-m from thakis@chromium.org

[spang, 2015-02-04 21:14:26]

https://codereview.chromium.org/889323003/diff/40001/extensions/shell/BUILD.gn
File extensions/shell/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/extensions/shell/BUILD.g...
extensions/shell/BUILD.gn:53: "//ui/base",
On 2015/02/04 20:29:35, Nico wrote:
> why is this needed all of a sudden?

There are direct dependencies:

git grep -l 'include.*ui/base' extensions/shell 
extensions/shell/app/shell_main_delegate.cc
extensions/shell/browser/shell_browser_main_parts.cc
extensions/shell/browser/shell_desktop_controller_aura.cc
extensions/shell/common/shell_content_client.cc

We must be getting it transitively.

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn
File ui/base/ime/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn#new...
ui/base/ime/BUILD.gn:94: "text_input_type.h",
On 2015/02/04 20:29:35, Nico wrote:
> add ui_base_ime_export.h

Done.

What effect does listing headers in sources have? The only thing I can think of
is that Xcode or Visual Studio use them.

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/chromeos/moc...
File ui/base/ime/chromeos/mock_ime_engine_handler.h (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/chromeos/moc...
ui/base/ime/chromeos/mock_ime_engine_handler.h:14: class UI_BASE_IME_EXPORT
MockIMEEngineHandler : public IMEEngineHandlerInterface {
On 2015/02/04 20:29:36, Nico wrote:
> nit: > 80 cols (run `git cl format` on your branch, it'll fix all these)

Done.

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/mock_input_m...
File ui/base/ime/mock_input_method.h (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/mock_input_m...
ui/base/ime/mock_input_method.h:26: class UI_BASE_IME_EXPORT MockInputMethod :
NON_EXPORTED_BASE(public InputMethod) {
On 2015/02/04 20:29:36, Nico wrote:
> same nit

Done.

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/ui_base_ime.gyp
File ui/base/ime/ui_base_ime.gyp (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/ui_base_ime....
ui/base/ime/ui_base_ime.gyp:115: 'text_input_type.h',
On 2015/02/04 20:29:36, Nico wrote:
> add new _export file here too

Done.

https://codereview.chromium.org/889323003/diff/40001/ui/keyboard/BUILD.gn
File ui/keyboard/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/ui/keyboard/BUILD.gn#new...
ui/keyboard/BUILD.gn:136: "//ui/base/ime",
On 2015/02/04 20:29:36, Nico wrote:
> this too looks surprising

Also seems to be missing:

git grep -l 'include.*ui/base' ui/keyboard                                      
                
ui/keyboard/keyboard.cc
ui/keyboard/keyboard_controller.cc
ui/keyboard/keyboard_controller.h
ui/keyboard/keyboard_controller_proxy.cc
ui/keyboard/keyboard_controller_proxy.h
ui/keyboard/keyboard_controller_unittest.cc
ui/keyboard/keyboard_util.cc
ui/keyboard/test/run_all_unittests.cc
ui/keyboard/webui/vk_mojo_handler.cc
ui/keyboard/webui/vk_mojo_handler.h

[spang, 2015-02-04 21:14:42]

On 2015/02/04 20:29:36, Nico wrote:
> Thanks much for doing this. Mostly lgtm. I'm confused that you're adding a dep
> on both ui/base and base/ui/ime in a few places – I would've expected that you
> need to add deps on base/ui/ime in places that already depend on ui/base, but
> adding both looks weird to me.

I added ui/base where there seemed to be a missing direct dependency.

> 
> https://codereview.chromium.org/889323003/diff/40001/extensions/shell/BUILD.gn
> File extensions/shell/BUILD.gn (right):
> 
>
https://codereview.chromium.org/889323003/diff/40001/extensions/shell/BUILD.g...
> extensions/shell/BUILD.gn:53: "//ui/base",
> why is this needed all of a sudden?
> 
> https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn
> File ui/base/ime/BUILD.gn (right):
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn#new...
> ui/base/ime/BUILD.gn:94: "text_input_type.h",
> add ui_base_ime_export.h
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/chromeos/moc...
> File ui/base/ime/chromeos/mock_ime_engine_handler.h (right):
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/chromeos/moc...
> ui/base/ime/chromeos/mock_ime_engine_handler.h:14: class UI_BASE_IME_EXPORT
> MockIMEEngineHandler : public IMEEngineHandlerInterface {
> nit: > 80 cols (run `git cl format` on your branch, it'll fix all these)
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/mock_input_m...
> File ui/base/ime/mock_input_method.h (right):
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/mock_input_m...
> ui/base/ime/mock_input_method.h:26: class UI_BASE_IME_EXPORT MockInputMethod :
> NON_EXPORTED_BASE(public InputMethod) {
> same nit
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/ui_base_ime.gyp
> File ui/base/ime/ui_base_ime.gyp (right):
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/ui_base_ime....
> ui/base/ime/ui_base_ime.gyp:115: 'text_input_type.h',
> add new _export file here too
> 
> https://codereview.chromium.org/889323003/diff/40001/ui/keyboard/BUILD.gn
> File ui/keyboard/BUILD.gn (right):
> 
>
https://codereview.chromium.org/889323003/diff/40001/ui/keyboard/BUILD.gn#new...
> ui/keyboard/BUILD.gn:136: "//ui/base/ime",
> this too looks surprising

[Nico, 2015-02-04 21:17:43]

lgtm!

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn
File ui/base/ime/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn#new...
ui/base/ime/BUILD.gn:94: "text_input_type.h",
On 2015/02/04 21:14:26, spang wrote:
> On 2015/02/04 20:29:35, Nico wrote:
> > add ui_base_ime_export.h
> 
> Done.
> 
> What effect does listing headers in sources have? The only thing I can think
of
> is that Xcode or Visual Studio use them.

The "analyze" step on the bots also uses this to figure out which targets to
rebuild after touching a header.

[tfarina, 2015-02-04 21:21:59]

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn
File ui/base/ime/BUILD.gn (right):

https://codereview.chromium.org/889323003/diff/40001/ui/base/ime/BUILD.gn#new...
ui/base/ime/BUILD.gn:94: "text_input_type.h",
On 2015/02/04 21:14:26, spang wrote:
> On 2015/02/04 20:29:35, Nico wrote:
> > add ui_base_ime_export.h
> 
> Done.
> 
> What effect does listing headers in sources have? The only thing I can think
of
> is that Xcode or Visual Studio use them.

As Nico said, it helps the 'analyze' step on the bots. Since this is a BUILD.gn
file it won't have any effect for Xcode or Visual Studio, as GN only generates
build files for Ninja.

[spang, 2015-02-04 21:29:09]

spang@chromium.org changed reviewers:
+ jam@chromium.org

[spang, 2015-02-04 21:29:10]

sky for ui/
+jam for extensions/ & content/

[Nico, 2015-02-04 21:35:05]

On 2015/02/04 21:29:10, spang wrote:
> sky for ui/

I'm in ui/OWNERS, you're good here.

> +jam for extensions/ & content/

[spang, 2015-02-04 21:59:46]

On 2015/02/04 21:35:05, Nico wrote:
> On 2015/02/04 21:29:10, spang wrote:
> > sky for ui/
> 
> I'm in ui/OWNERS, you're good here.

Oops, thanks. I did forget ash/, so actually:

sky: ash/
jam: extensions/ & content/

[sky, 2015-02-05 03:12:19]

ash LGTM

[Nico, 2015-02-05 03:27:35]

(I think this is simple enough that you can tbr content and extensions)
On Feb 4, 2015 7:12 PM, <sky@chromium.org> wrote:

> ash LGTM
>
> https://codereview.chromium.org/889323003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Yuki, 2015-02-05 05:19:56]

yukishiino@chromium.org changed reviewers:
+ yukishiino@chromium.org

[Yuki, 2015-02-05 05:19:57]

+cc: shuchen

Just FYI.  We may want to remove the dependency from ui/base/ime to ui/ozone in
future.  If ui/ozone can inject its ui::internal::InputMethodDelegate like
LinuxUI / Gtk2UI (ui::LinuxInputMethodContextFactory for example), we can get
rid of this dependency.

[spang, 2015-02-05 15:40:41]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2015-02-05 15:41:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/889323003/60001

[commit-bot: I haz the power, 2015-02-05 15:46:08]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-05 15:47:06]

Patchset 4 (id:??) landed as
https://crrev.com/2321ca1b99c1a8e9a90e48a9cd50f6446a7e5e82
Cr-Commit-Position: refs/heads/master@{#314815}

[ccameron, 2015-02-05 16:27:04]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/895363005/ by ccameron@chromium.org.

The reason for reverting is: Windows builds failing with

ERROR at //ui/base/ime/BUILD.gn:170:5: Undefined variable for +=.
    libs += [ "imm32.lib" ]
    ^---
I don't have something with this name in scope now.

http://build.chromium.org/p/chromium.win/builders/Win8%20GN/builds/4403/steps...

(perhaps missed a .gn update)?.

[spang, 2015-02-05 16:30:50]

On 2015/02/05 16:27:04, ccameron wrote:
> A revert of this CL (patchset #4 id:60001) has been created in
> https://codereview.chromium.org/895363005/ by mailto:ccameron@chromium.org.
> 
> The reason for reverting is: Windows builds failing with
> 
> ERROR at //ui/base/ime/BUILD.gn:170:5: Undefined variable for +=.
>     libs += [ "imm32.lib" ]
>     ^---
> I don't have something with this name in scope now.
> 
>
http://build.chromium.org/p/chromium.win/builders/Win8%20GN/builds/4403/steps...
> 
> (perhaps missed a .gn update)?.

Nope, it's just wrong. Doesn't look like the CQ ran a GN build for windows.

Sorry & thanks.

[tfarina, 2015-02-05 16:37:23]

On 2015/02/05 16:30:50, spang wrote:
> On 2015/02/05 16:27:04, ccameron wrote:
> > A revert of this CL (patchset #4 id:60001) has been created in
> > https://codereview.chromium.org/895363005/ by mailto:ccameron@chromium.org.
> > 
> > The reason for reverting is: Windows builds failing with
> > 
> > ERROR at //ui/base/ime/BUILD.gn:170:5: Undefined variable for +=.
> >     libs += [ "imm32.lib" ]
> >     ^---
> > I don't have something with this name in scope now.
> > 
> >
>
http://build.chromium.org/p/chromium.win/builders/Win8%20GN/builds/4403/steps...
> > 
> > (perhaps missed a .gn update)?.
> 
> Nope, it's just wrong. Doesn't look like the CQ ran a GN build for windows.
> 
> Sorry & thanks.

[tfarina, 2015-02-05 16:38:15]

On 2015/02/05 16:37:23, tfarina wrote:
> On 2015/02/05 16:30:50, spang wrote:
> > On 2015/02/05 16:27:04, ccameron wrote:
> > > A revert of this CL (patchset #4 id:60001) has been created in
> > > https://codereview.chromium.org/895363005/ by
mailto:ccameron@chromium.org.
> > > 
> > > The reason for reverting is: Windows builds failing with
> > > 
> > > ERROR at //ui/base/ime/BUILD.gn:170:5: Undefined variable for +=.
> > >     libs += [ "imm32.lib" ]
> > >     ^---
> > > I don't have something with this name in scope now.
> > > 
> > >
> >
>
http://build.chromium.org/p/chromium.win/builders/Win8%20GN/builds/4403/steps...
> > > 
> > > (perhaps missed a .gn update)?.
> > 
> > Nope, it's just wrong. Doesn't look like the CQ ran a GN build for windows.
> > 

crbug.com/439402

Though
https://chromium.googlesource.com/chromium/src/+/195f53b3677a9d58324b563e5adb...,
says it adds Win8 to CQ.

[spang, 2015-02-05 16:44:41]

New patchsets have been uploaded after l-g-t-m from
thakis@chromium.org,sky@chromium.org

[spang, 2015-02-05 19:49:24]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2015-02-05 19:50:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/889323003/100001

[commit-bot: I haz the power, 2015-02-05 19:55:42]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-05 19:57:26]

Patchset 6 (id:??) landed as
https://crrev.com/1c36facc53f4a0038f2e2e482c78a65438cc8ec4
Cr-Commit-Position: refs/heads/master@{#314860}