Make Chrome for Android dependent on Google Play Services

Make Chrome for Android dependent on Google Play Services

Various bits of Chrome for Android that are being upstreamed
(in particular Cast support) require Google Play Services.
This adds Google Play Services to the Android chrome_java
target.

Note that this uses a variable so that one can use an
alternative version of Google Play Services. This is
required for Chrome for Android development.

TBR=sky@chromium.org
BUG=450675
Committed: https://crrev.com/c8db3ccbde213d09fac41060c0bf81d4a0020cda
Cr-Commit-Position: refs/heads/master@{#315538}

[aberent, 2015-01-21 23:51:18]

aberent@chromium.org changed reviewers:
+ cjhopman@chromium.org, nyquist@chromium.org

[aberent, 2015-01-21 23:51:18]

Will add owner reviewers once you have had a look at this.

[aberent, 2015-01-21 23:56:27]

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp#newcode627
chrome/chrome.gyp:627:
'../third_party/android_tools/android_tools.gyp:android_support_v7_mediarouter_javalib',
Noticed this after uploading. Accidental leak from my next task (upstreaming
Cast). I will remove it in the next patch for this CL.

[nyquist, 2015-01-22 01:21:43]

I think the last two numbers in the BUG= line are flipped.

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp#newcode609
chrome/chrome.gyp:609: '<(google_play_services_library_target)',
I would have liked it better if this was a proxy target instead of a variable
referring to a specific name. How it is now makes it confusing for how to add a
dependency on Google Play Services.

[nyquist, 2015-01-22 01:23:39]

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp#newcode688
chrome/chrome.gyp:688: 'google_play_services_library_target%':
'../third_party/android_tools/android_tools.gyp:google_play_services_javalib',
I believe that the extras folder is only downloaded automatically on the bots.
Should this CL also come with instructions if the library can not be found?
I believe the plan was to make that part of install-build-deps-android.sh and/or
add it to the documentation.

[aberent, 2015-01-22 16:28:55]

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp#newcode609
chrome/chrome.gyp:609: '<(google_play_services_library_target)',
On 2015/01/22 01:21:43, nyquist wrote:
> I would have liked it better if this was a proxy target instead of a variable
> referring to a specific name. How it is now makes it confusing for how to add
a
> dependency on Google Play Services.

Done.

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp#newcode627
chrome/chrome.gyp:627:
'../third_party/android_tools/android_tools.gyp:android_support_v7_mediarouter_javalib',
On 2015/01/21 23:56:27, aberent wrote:
> Noticed this after uploading. Accidental leak from my next task (upstreaming
> Cast). I will remove it in the next patch for this CL.

Done.

https://codereview.chromium.org/865733002/diff/1/chrome/chrome.gyp#newcode688
chrome/chrome.gyp:688: 'google_play_services_library_target%':
'../third_party/android_tools/android_tools.gyp:google_play_services_javalib',
On 2015/01/22 01:23:38, nyquist wrote:
> I believe that the extras folder is only downloaded automatically on the bots.
> Should this CL also come with instructions if the library can not be found?
> I believe the plan was to make that part of install-build-deps-android.sh
and/or
> add it to the documentation.

It has been added to install-build-deps-android.sh. I have included this
information in
a comment in android_tools.gyp; do you think it is needed here as well?

[nyquist, 2015-01-22 17:16:03]

lgtm, but could you change 'clank' to something more descriptive?

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

also, I believe this target already has GN, so you would need to update the GN
target as well.

[aberent, 2015-02-05 13:59:37]

Patchset #3 (id:40001) has been deleted

[aberent, 2015-02-05 15:40:48]

On 2015/01/22 17:17:02, nyquist wrote:
> also, I believe this target already has GN, so you would need to update the GN
> target as well.

PTAL

[cjhopman, 2015-02-05 21:29:39]

lgtm

[nyquist, 2015-02-05 22:32:08]

lgtm % one small comment about libaddress

https://codereview.chromium.org/865733002/diff/80001/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/865733002/diff/80001/chrome/chrome.gyp#newcod...
chrome/chrome.gyp:630:
'../third_party/libaddressinput/libaddressinput.gyp:android_addressinput_widget',
Is this related to this change or is it here by accident?

[nyquist, 2015-02-05 23:26:49]

By the way, does this require a roll of
https://chromium-review.googlesource.com/#/c/242390/ or has that already been
done?

[aberent, 2015-02-06 17:19:16]

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

[aberent, 2015-02-06 17:43:11]

On 2015/02/05 at 23:26:49, nyquist wrote:
> By the way, does this require a roll of
https://chromium-review.googlesource.com/#/c/242390/ or has that already been
done?

The roll is https://codereview.chromium.org/900273003, unfortunately just
reverted because of a hidden Chromecast bot that hadn't been updated.

[aberent, 2015-02-06 20:17:58]

https://codereview.chromium.org/865733002/diff/80001/chrome/chrome.gyp
File chrome/chrome.gyp (right):

https://codereview.chromium.org/865733002/diff/80001/chrome/chrome.gyp#newcod...
chrome/chrome.gyp:630:
'../third_party/libaddressinput/libaddressinput.gyp:android_addressinput_widget',
On 2015/02/05 at 22:32:08, nyquist wrote:
> Is this related to this change or is it here by accident?

Done. It was there by accident (probably a bad merge).

[aberent, 2015-02-09 11:56:16]

The CQ bit was checked by aberent@chromium.org

[commit-bot: I haz the power, 2015-02-09 11:57:22]

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

[commit-bot: I haz the power, 2015-02-09 13:24:56]

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

[commit-bot: I haz the power, 2015-02-09 13:25:00]

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

[aberent, 2015-02-10 11:16:59]

The CQ bit was checked by aberent@chromium.org

[commit-bot: I haz the power, 2015-02-10 11:17:46]

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

[commit-bot: I haz the power, 2015-02-10 11:18:17]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2015-02-10 11:19:05]

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