Description
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}
Patch Set 1 #
Total comments: 6
Patch Set 2 : Move variable to android_tools.gyp, and other tidy up. #
Patch Set 3 : Add GN build changes #
Patch Set 4 : Fix description #
Total comments: 2
Patch Set 5 : Fix nits #
Messages
Total messages: 24 (6 generated)
| aberent |
aberent@chromium.org changed reviewers: + cjhopman@chromium.org, nyquist@chromium.org |
5 years, 11 months ago (2015-01-21 23:51:18 UTC) #1 |
| aberent |
Will add owner reviewers once you have had a look at this. |
5 years, 11 months ago (2015-01-21 23:51:18 UTC) #2 |
| aberent |
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 ... |
5 years, 11 months ago (2015-01-21 23:56:27 UTC) #3 |
| nyquist |
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 ... |
5 years, 11 months ago (2015-01-22 01:21:43 UTC) #4 |
| nyquist |
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 ... |
5 years, 11 months ago (2015-01-22 01:23:39 UTC) #5 |
| aberent |
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 ... |
5 years, 11 months ago (2015-01-22 16:28:55 UTC) #6 |
| nyquist |
lgtm, but could you change 'clank' to something more descriptive? |
5 years, 11 months ago (2015-01-22 17:16:03 UTC) #7 |
| nyquist |
also, I believe this target already has GN, so you would need to update the ... |
5 years, 11 months ago (2015-01-22 17:17:02 UTC) #8 |
| aberent |
Patchset #3 (id:40001) has been deleted |
5 years, 10 months ago (2015-02-05 13:59:37 UTC) #9 |
| aberent |
On 2015/01/22 17:17:02, nyquist wrote: > also, I believe this target already has GN, so ... |
5 years, 10 months ago (2015-02-05 15:40:48 UTC) #10 |
| cjhopman | 5 years, 10 months ago (2015-02-05 21:29:39 UTC) #11 |
| nyquist |
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#newcode630 chrome/chrome.gyp:630: '../third_party/libaddressinput/libaddressinput.gyp:android_addressinput_widget', Is ... |
5 years, 10 months ago (2015-02-05 22:32:08 UTC) #12 |
| nyquist |
By the way, does this require a roll of https://chromium-review.googlesource.com/#/c/242390/ or has that already been ... |
5 years, 10 months ago (2015-02-05 23:26:49 UTC) #13 |
| aberent |
New patchsets have been uploaded after l-g-t-m from cjhopman@chromium.org,nyquist@chromium.org |
5 years, 10 months ago (2015-02-06 17:19:16 UTC) #14 |
| aberent |
On 2015/02/05 at 23:26:49, nyquist wrote: > By the way, does this require a roll ... |
5 years, 10 months ago (2015-02-06 17:43:11 UTC) #15 |
| aberent |
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#newcode630 chrome/chrome.gyp:630: '../third_party/libaddressinput/libaddressinput.gyp:android_addressinput_widget', On 2015/02/05 at 22:32:08, nyquist wrote: > Is ... |
5 years, 10 months ago (2015-02-06 20:17:58 UTC) #16 |
| aberent |
The CQ bit was checked by aberent@chromium.org |
5 years, 10 months ago (2015-02-09 11:56:16 UTC) #17 |
| commit-bot: I haz the power |
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/865733002/100001 |
5 years, 10 months ago (2015-02-09 11:57:22 UTC) #18 |
| commit-bot: I haz the power |
The CQ bit was unchecked by commit-bot@chromium.org |
5 years, 10 months ago (2015-02-09 13:24:56 UTC) #19 |
| commit-bot: I haz the power |
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_tests_recipe/builds/53522) |
5 years, 10 months ago (2015-02-09 13:25:00 UTC) #20 |
| aberent |
The CQ bit was checked by aberent@chromium.org |
5 years, 10 months ago (2015-02-10 11:16:59 UTC) #21 |
| commit-bot: I haz the power |
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/865733002/100001 |
5 years, 10 months ago (2015-02-10 11:17:46 UTC) #22 |
| commit-bot: I haz the power |
Committed patchset #5 (id:100001) |
5 years, 10 months ago (2015-02-10 11:18:17 UTC) #23 |
| commit-bot: I haz the power |
Patchset 5 (id:??) landed as https://crrev.com/c8db3ccbde213d09fac41060c0bf81d4a0020cda Cr-Commit-Position: refs/heads/master@{#315538} |
5 years, 10 months ago (2015-02-10 11:19:05 UTC) #24 |