Remove dependency from ui/base to cc on non-Android platforms

Remove dependency from ui/base to cc on non-Android platforms

This is needed to resolve a dependency cycle when use_ozone=1:

  gn gen out_gn_ozone --args='os="chromeos" use_ozone=true'

  ERROR Dependency cycle:
    //ui/base:base ->
    //cc:cc ->
    //gpu:gpu ->
    //gpu/command_buffer/service:service ->
    //ui/gl:gl ->
    //ui/ozone:ozone ->
    //ui/ozone/platform/test:test ->
    //ui/base:base

The ui/base -> cc dependency is very small (2 files) and only used for
Android builds. We can fix the issue by restricting this dependency to
ui/base/android. Two functions in common code needed moving to drop the
dependency from non-Android builds.

Also add the cc dependency to the gyp build as it's missing there.

BUG=445627
TEST=built android chrome_shell_apk (both gyp & gn)
  gn gen out_gn_ozone --args='os="chromeos" use_ozone=true is_component_build=true' &&
  ninja -C out_gn_ozone chrome ozone_unittests (last one!)

Committed: https://crrev.com/ce336f674e224d9cbde5a5acc97f104fc888ae30
Cr-Commit-Position: refs/heads/master@{#316687}

[spang, 2015-02-10 02:38:27]

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

[spang, 2015-02-10 02:38:27]

This is the last change needed to make our GN build work again.

thakis: ui/
aelias: content/browser/renderer_host/

[Nico, 2015-02-10 02:48:31]

thakis@chromium.org changed reviewers:
+ danakj@chromium.org

[Nico, 2015-02-10 02:48:32]

Looks like the dep from ui/base on cc comes from
https://codereview.chromium.org/281003002

Dana, can we make ui/base not depend on cc on android too?

This change lgtm, but maybe Dana wants to comment on it too.

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

https://codereview.chromium.org/908033003/diff/1/ui/base/BUILD.gn#newcode531
ui/base/BUILD.gn:531: "//cc",
It's a bit wild that this dep is required on android but not permitted with
ozone.

[spang, 2015-02-10 02:54:00]

On 2015/02/10 02:48:32, Nico wrote:
> Looks like the dep from ui/base on cc comes from
> https://codereview.chromium.org/281003002
> 
> Dana, can we make ui/base not depend on cc on android too?
> 
> This change lgtm, but maybe Dana wants to comment on it too.
> 
> https://codereview.chromium.org/908033003/diff/1/ui/base/BUILD.gn
> File ui/base/BUILD.gn (right):
> 
> https://codereview.chromium.org/908033003/diff/1/ui/base/BUILD.gn#newcode531
> ui/base/BUILD.gn:531: "//cc",
> It's a bit wild that this dep is required on android but not permitted with
> ozone.

Yeah.. I agree with removing it altogether, it just wasn't immediately obvious
to me how to do it. There's only one thing left: usage of cc::CopyOutputRequest
in WindowAndroidCompositor.

[danakj, 2015-02-10 17:59:03]

It would be fine with me for sure. We just need to move some code around or
something. Maybe window_android_compositor.h doesn't belong in that dir?


On Mon, Feb 9, 2015 at 6:54 PM, <spang@chromium.org> wrote:
>
> On 2015/02/10 02:48:32, Nico wrote:
>>
>> Looks like the dep from ui/base on cc comes from
>> https://codereview.chromium.org/281003002
>
>
>> Dana, can we make ui/base not depend on cc on android too?
>
>
>> This change lgtm, but maybe Dana wants to comment on it too.
>
>
>> https://codereview.chromium.org/908033003/diff/1/ui/base/BUILD.gn
>> File ui/base/BUILD.gn (right):
>
>
>>
https://codereview.chromium.org/908033003/diff/1/ui/base/BUILD.gn#newcode531
>> ui/base/BUILD.gn:531: "//cc",
>> It's a bit wild that this dep is required on android but not permitted
with
>> ozone.
>
>
> Yeah.. I agree with removing it altogether, it just wasn't immediately
obvious
> to me how to do it. There's only one thing left: usage of
cc::CopyOutputRequest
> in WindowAndroidCompositor.
>
> https://codereview.chromium.org/908033003/

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

[aelias_OOO_until_Jul13, 2015-02-10 20:39:10]

aelias@chromium.org changed reviewers:
+ jaekyun@chromium.org

[aelias_OOO_until_Jul13, 2015-02-10 20:40:08]

I don't have any real strong opinion about what directory that code should live
in either.  It does seem that if it's causing DEPS craziness, it shouldn't live
in ui/base after all though.  Would moving the files to ui/android work?

[Jaekyun Seok (inactive), 2015-02-10 21:33:09]

jaekyun@chromium.org changed reviewers:
+ dtrainor@chromium.org, jdduke@chromium.org

[Jaekyun Seok (inactive), 2015-02-10 21:33:09]

It seems good to move the files to ui/android, but maybe David and Jared want to
comment on it too.

[jdduke (slow), 2015-02-10 22:07:27]

This looks fine. At some point other platforms will require the cc -> ui
conversion for selection bounds, but we can cross that bridge when we get there.

[David Trainor- moved to gerrit, 2015-02-11 01:00:07]

On 2015/02/10 22:07:27, jdduke wrote:
> This looks fine. At some point other platforms will require the cc -> ui
> conversion for selection bounds, but we can cross that bridge when we get
there.

Looks fine to me as well.

[spang, 2015-02-12 21:15:21]

On 2015/02/10 20:40:08, aelias wrote:
> I don't have any real strong opinion about what directory that code should
live
> in either.  It does seem that if it's causing DEPS craziness, it shouldn't
live
> in ui/base after all though.  Would moving the files to ui/android work?

It doesn't look completely straightforward.

We probably have to slice up ui_base_jni_registrar.cc - ui_base_unittests is
using this and it references WindowAndroid, which references
WindowAndroidCompositor, which references cc::CopyOutputRequest.

The dep isn't causing the Android just any real problems, just the CrOS ozone
one. Since they are mutually exclusive, I don't think this is an urgent problem
to fix.

[spang, 2015-02-15 20:31:26]

On 2015/02/12 21:15:21, spang wrote:
> On 2015/02/10 20:40:08, aelias wrote:
> > I don't have any real strong opinion about what directory that code should
> live
> > in either.  It does seem that if it's causing DEPS craziness, it shouldn't
> live
> > in ui/base after all though.  Would moving the files to ui/android work?
> 
> It doesn't look completely straightforward.
> 
> We probably have to slice up ui_base_jni_registrar.cc - ui_base_unittests is
> using this and it references WindowAndroid, which references
> WindowAndroidCompositor, which references cc::CopyOutputRequest.
> 
> The dep isn't causing the Android just any real problems, just the CrOS ozone
> one. Since they are mutually exclusive, I don't think this is an urgent
problem
> to fix.

aelias: ping

[Nico, 2015-02-15 22:11:57]

I think this can land as is, but I'd really like ui_base to not depend on cc on
android medium-term. Having different layering orders on different platforms
(ui_base -> cc on android, the other way round elsewhere) doesn't sound like a
good spot to be in.

[aelias_OOO_until_Jul13, 2015-02-16 02:26:24]

OK, lgtm.  I filed http://crbug.com/458889 so we don't forget about it.

[spang, 2015-02-17 20:02:27]

The CQ bit was checked by spang@chromium.org

[commit-bot: I haz the power, 2015-02-17 20:03:09]

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

[commit-bot: I haz the power, 2015-02-17 21:44:43]

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

[commit-bot: I haz the power, 2015-02-17 21:44:44]

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

[Nico, 2015-02-17 22:01:02]

The CQ bit was checked by thakis@chromium.org

[commit-bot: I haz the power, 2015-02-17 22:02:29]

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

[commit-bot: I haz the power, 2015-02-17 22:53:45]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-02-17 22:54:18]

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