Move ash/ime to ui/chromeos/ime

Move ash/ime to ui/chromeos/ime. This should make it easier
to compile Athena without compiling Ash

BUG=426561
TEST=None
R=oshima, shuchen, sky

Committed: https://crrev.com/347c774cda72db0e5a5d7f9c6b33c2ada93fc614
Cr-Commit-Position: refs/heads/master@{#304271}

[pkotwicz, 2014-11-06 20:12:12]

Patchset #1 (id:1) has been deleted

[pkotwicz, 2014-11-06 20:12:21]

Patchset #1 (id:20001) has been deleted

[pkotwicz, 2014-11-06 20:12:29]

Patchset #1 (id:40001) has been deleted

[pkotwicz, 2014-11-06 20:14:32]

Patchset #1 (id:60001) has been deleted

[pkotwicz, 2014-11-06 20:16:48]

pkotwicz@chromium.org changed reviewers:
+ shuchen@chromium.org

[pkotwicz, 2014-11-06 20:17:07]

shuchen@ PTAL

[pkotwicz, 2014-11-06 20:20:10]

Patchset #1 (id:80001) has been deleted

[Shu Chen, 2014-11-08 00:58:17]

https://codereview.chromium.org/680383008/diff/100001/chrome/browser/chromeos...
File chrome/browser/chromeos/input_method/input_method_engine.cc (right):

https://codereview.chromium.org/680383008/diff/100001/chrome/browser/chromeos...
chrome/browser/chromeos/input_method/input_method_engine.cc:40: #if
!defined(USE_ATHENA)
I think this should be #if defined(USE_ASH)

[Shu Chen, 2014-11-08 01:50:39]

Also, please fix the compile failures for athena and ozone.

[pkotwicz, 2014-11-10 23:18:40]

Patchset #2 (id:120001) has been deleted

[pkotwicz, 2014-11-11 02:17:43]

shuchen@ can you please take another look?

I have added a new test target: ui_chromeos_unittests
I will add the new test to the relevant bots in a separate CL

[Shu Chen, 2014-11-11 02:36:10]

lgtm

[pkotwicz, 2014-11-11 03:35:32]

pkotwicz@chromium.org changed reviewers:
+ oshima@chromium.org

[pkotwicz, 2014-11-11 03:35:33]

Oshima, can you please take a look?
The CL itself is boring. I am mostly looking for your approval on:
- moving ash/ime to ui/chromeos/ime
- adding a ui_chromeos_unittests target

[oshima, 2014-11-11 18:00:11]

Is the new test ready to run on CQ/try and main waterfall?

If not, it's better to move tests to a new test target in a separate CL and bug.
And please move other tests in ui/chromeos as well.

In the mean time, you can can temporarily add them to ui_base_unittests.

[pkotwicz, 2014-11-11 21:28:58]

This CL does refactoring only. I have not added any unit tests (apart for taking
existing tests from ash/ime and moving them to ui/chromeos)
I am putting the tests that I moved into a new test suite ui_chromeos_unittests

Are the unit tests ready to run on the CQ/try and main waterfall? Yes, they are
already doing so as part of ash_unittests. My understanding is that this CL will
make ui_chromeos_unittests CQ blocking but that I need some more changes to
enable ui_chromeos_unittests on the main waterfall

[pkotwicz, 2014-11-11 21:33:11]

I think I have misread your comment. I will move the tests to ui_base_unittests
for now

[pkotwicz, 2014-11-11 21:33:11]

I think I have misread your comment. I will move the tests to ui_base_unittests
for now

[pkotwicz, 2014-11-11 21:33:12]

I think I have misread your comment. I will move the tests to ui_base_unittests
for now

[pkotwicz, 2014-11-11 21:55:59]

Patchset #2 (id:130056) has been deleted

[oshima, 2014-11-11 21:59:35]

On 2014/11/11 21:28:58, pkotwicz wrote:
> This CL does refactoring only. I have not added any unit tests (apart for
taking
> existing tests from ash/ime and moving them to ui/chromeos)
> I am putting the tests that I moved into a new test suite
ui_chromeos_unittests

Yes that's what I was asking.

> Are the unit tests ready to run on the CQ/try and main waterfall? Yes, they
are
> already doing so as part of ash_unittests. My understanding is that this CL
will
> make ui_chromeos_unittests CQ blocking

I believe you need to add the test to the json script in testing/buildbot.
You may also need to add the new test to "all" target in build/all.gyp, although
I may be wrong.


> but that I need some more changes to
> enable ui_chromeos_unittests on the main waterfall

This CL basically removes the test from CQ/try/mainwaterfall. You need to make
sure you can enable the test as soon as possible.

[pkotwicz, 2014-11-11 22:05:59]

Oshima, can you please take another look? I have temporarily moved the tests to
ui_base_unittests

[oshima, 2014-11-11 23:18:05]

lgtm

https://codereview.chromium.org/680383008/diff/170001/ui/base/ui_base_tests.gypi
File ui/base/ui_base_tests.gypi (right):

https://codereview.chromium.org/680383008/diff/170001/ui/base/ui_base_tests.g...
ui/base/ui_base_tests.gypi:200: '../views/views.gyp:views_test_support',
can you file a bug and add TODO to move tests and remove this?

[tfarina, 2014-11-12 01:44:30]

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

[tfarina, 2014-11-12 01:44:31]

https://codereview.chromium.org/680383008/diff/170001/ui/base/ui_base_tests.gypi
File ui/base/ui_base_tests.gypi (right):

https://codereview.chromium.org/680383008/diff/170001/ui/base/ui_base_tests.g...
ui/base/ui_base_tests.gypi:200: '../views/views.gyp:views_test_support',
On 2014/11/11 23:18:05, oshima wrote:
> can you file a bug and add TODO to move tests and remove this?

Yes, please, do this. Also copy me on it!

Thanks!

[pkotwicz, 2014-11-12 16:34:35]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-12 16:35:13]

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

[commit-bot: I haz the power, 2014-11-12 17:34:29]

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

[commit-bot: I haz the power, 2014-11-12 17:34:30]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pkotwicz, 2014-11-13 15:49:31]

Patchset #5 (id:210001) has been deleted

[pkotwicz, 2014-11-13 15:49:52]

Patchset #4 (id:190001) has been deleted

[pkotwicz, 2014-11-13 15:52:40]

Patchset #4 (id:230001) has been deleted

[pkotwicz, 2014-11-13 15:55:30]

sky@ can you please take a look at the changes in ui/base? There were slightly
more changes than I initially expected.
In a separate CL I will add a ui_chromeos_unittests target

[pkotwicz, 2014-11-13 15:55:35]

sky@ can you please take a look at the changes in ui/base? There were slightly
more changes than I initially expected.
In a separate CL I will add a ui_chromeos_unittests target

[pkotwicz, 2014-11-13 15:56:07]

pkotwicz@chromium.org changed reviewers:
+ sky@chromium.org

[pkotwicz, 2014-11-14 15:21:49]

sky@ ping?

[sky, 2014-11-14 17:56:04]

https://codereview.chromium.org/680383008/diff/250001/ui/base/BUILD.gn
File ui/base/BUILD.gn (right):

https://codereview.chromium.org/680383008/diff/250001/ui/base/BUILD.gn#newcod...
ui/base/BUILD.gn:918: "//ui/views",
This is wrong and means there is a cycle between views and ui_base.

[oshima, 2014-11-14 19:52:11]

https://codereview.chromium.org/680383008/diff/250001/ui/base/BUILD.gn
File ui/base/BUILD.gn (right):

https://codereview.chromium.org/680383008/diff/250001/ui/base/BUILD.gn#newcod...
ui/base/BUILD.gn:918: "//ui/views",
On 2014/11/14 17:56:04, sky wrote:
> This is wrong and means there is a cycle between views and ui_base.

This is for the test target, so it won't introduce the cycle.

That's being said, I agree that even test target shouldn't depend no views, but
this target actually already depends on views (and aura) indirectly.
(ui_base_tests.gypi depends on ui_chrmoeos, which depends on views, and aura).

The plan is Peter will move these test to separate ui_chromeos_unittest and
remove these dependency.

[sky, 2014-11-14 20:29:22]

*SIGH* LGTM

[pkotwicz, 2014-11-14 20:46:29]

The CQ bit was checked by pkotwicz@chromium.org

[commit-bot: I haz the power, 2014-11-14 20:47:10]

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

[commit-bot: I haz the power, 2014-11-14 21:42:55]

Committed patchset #4 (id:250001)

[commit-bot: I haz the power, 2014-11-14 21:44:11]

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