Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(510)

Issue 10913083: Remove {base,net}_java dependencies from GYP client targets on Android. (Closed)

Created:
8 years, 3 months ago by Philippe
Modified:
8 years, 3 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, jam, apatrick_chromium, joi+watch-content_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, erikwright+watch_chromium.org
Visibility:
Public.

Description

Remove {base,net}_java dependencies from GYP client targets on Android. On Android, some GYP client targets were explicitly depending on both the native and Java sides of a library. This removes the dependencies to {base,net}_java and make the native side ('base' and 'net) of these libraries depend on their Java counterpart. On Android it rarely makes sense to depend on a single side of a Java/C++ library. The {base,net}_java can now be considered as "private" targets although GYP does not support this concept unfortunately (AFAICT). Note that I made sure that the resulting APKs' size is unchanged. Additionally, this CL removes 'base_java' (i.e. does not replace it with 'base') from the targets including 'build/apk_test.gypi'. This dependency should not have been there (in the wrong layer) in the first place. It's needed by ChromeNativeTestActivity.java which clients should not know about. BUG=146323 TBR=lipalani,sky,willchan,brettw Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=155737

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : Address Peter's comment #

Total comments: 7

Patch Set 4 : Address Chris' comments #

Patch Set 5 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+33 lines, -25 lines) Patch
M base/base.gyp View 1 2 3 4 2 chunks +5 lines, -1 line 0 comments Download
M build/apk_test.gypi View 1 2 3 4 1 chunk +4 lines, -1 line 0 comments Download
M chrome/browser/component/web_contents_delegate_android/web_contents_delegate_android.gypi View 1 chunk +1 line, -1 line 0 comments Download
M chrome/chrome.gyp View 1 2 3 4 1 chunk +2 lines, -4 lines 0 comments Download
M content/content.gyp View 1 2 3 4 1 chunk +4 lines, -4 lines 0 comments Download
M content/content_tests.gypi View 1 2 3 4 2 chunks +1 line, -3 lines 0 comments Download
M gpu/gpu_common.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M ipc/ipc.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M media/media.gyp View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M net/net.gyp View 1 2 3 4 3 chunks +14 lines, -4 lines 0 comments Download
M sql/sql.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M sync/sync.gyp View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download
M ui/ui_unittests.gypi View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 16 (0 generated)
Philippe
I'm adding you guys first for a sanity check before I send that to other ...
8 years, 3 months ago (2012-09-05 14:08:54 UTC) #1
Philippe
Ping :)
8 years, 3 months ago (2012-09-06 09:02:03 UTC) #2
Peter Beverloo
LGTM, thank you Philippe! I'd like Marcus to have a look as well, though. There'd ...
8 years, 3 months ago (2012-09-06 10:09:09 UTC) #3
bulach
lgtm, with peter's comments.. I think your solution is better, but just some background.. :) ...
8 years, 3 months ago (2012-09-06 10:22:14 UTC) #4
Philippe
Thanks for the quick review guys and for the detailed explanation Marcus! I agree that ...
8 years, 3 months ago (2012-09-06 15:49:46 UTC) #5
Yaron
There's a minor downside in that there's less parallelism to the build, right? This makes ...
8 years, 3 months ago (2012-09-06 19:01:48 UTC) #6
cjhopman
I think that having input_jars_paths be a direct_dependent_setting and then everything having to export_dependent_setting that ...
8 years, 3 months ago (2012-09-06 20:29:24 UTC) #7
cjhopman
On 2012/09/06 19:01:48, Yaron wrote: > There's a minor downside in that there's less parallelism ...
8 years, 3 months ago (2012-09-06 20:41:38 UTC) #8
Philippe
I tried to make changes in base/android/java and re-build. I confirm that it only re-triggers ...
8 years, 3 months ago (2012-09-07 09:45:57 UTC) #9
cjhopman
lgtm https://chromiumcodereview.appspot.com/10913083/diff/1015/build/apk_test.gypi File build/apk_test.gypi (right): https://chromiumcodereview.appspot.com/10913083/diff/1015/build/apk_test.gypi#newcode27 build/apk_test.gypi:27: }, On 2012/09/07 09:45:57, Philippe wrote: > On ...
8 years, 3 months ago (2012-09-07 16:37:01 UTC) #10
Yaron
lgtm
8 years, 3 months ago (2012-09-07 20:41:09 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pliard@chromium.org/10913083/14002
8 years, 3 months ago (2012-09-10 09:45:45 UTC) #12
commit-bot: I haz the power
Try job failure for 10913083-14002 (retry) on android for steps "compile, build" (clobber build). It's ...
8 years, 3 months ago (2012-09-10 09:59:11 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pliard@chromium.org/10913083/14002
8 years, 3 months ago (2012-09-10 13:50:52 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/pliard@chromium.org/10913083/14002
8 years, 3 months ago (2012-09-10 14:00:54 UTC) #15
commit-bot: I haz the power
8 years, 3 months ago (2012-09-10 15:52:48 UTC) #16
Change committed as 155737

Powered by Google App Engine
This is Rietveld 408576698