Android: Add java files from android_apk targets

Android: Add java files from android_apk targets

This allows editing android_webview_apk and android_webview_test_apk
files in android studio.

BUG=670044
Committed: https://crrev.com/c3bace8213461c2a3884b36a3b80aea0e651b15a
Cr-Commit-Position: refs/heads/master@{#437056}

[Peter Wen, 2016-12-01 15:12:23]

wnwen@chromium.org changed reviewers:
+ agrieve@chromium.org

[Peter Wen, 2016-12-01 15:12:23]

🎁

[agrieve, 2016-12-05 18:28:03]

On 2016/12/01 15:12:23, Peter Wen wrote:
> 🎁

Looked at this a bit, and I think the better way to solve this is to make an
apk's build_config aware of its existing java.sources file. It uses
"build_config_override" so that their internal java_library doesn't write its
own build_config. However, the build_config that the apk creates never sets the
java_sources_file variable.

https://cs.chromium.org/chromium/src/build/config/android/rules.gni?dr=C&sq=p...
https://cs.chromium.org/chromium/src/build/config/android/internal_rules.gni?...

I think the best way to fix this is to have java_library_impl assert:
assert(!defined(build_config_override) || _java_files == [] ||
defined(invoker.java_sources_file))

And then pass java_sources_file along rather than writing a new one.

[Peter Wen, 2016-12-06 19:24:08]

How's this? I wasn't sure how to avoid the __java.sources in the string. What
generates these sources?

[Peter Wen, 2016-12-06 20:15:43]

PTAL. :)

[Peter Wen, 2016-12-06 20:15:51]

The CQ bit was checked by wnwen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 20:16:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[agrieve, 2016-12-06 20:25:59]

lgtm /w nit

https://codereview.chromium.org/2545793002/diff/40001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/2545793002/diff/40001/build/config/android/ru...
build/config/android/rules.gni:1437: java_sources_file = "$base_path.sources"
nit: Please make this _java_sources_file and explicitly pass it to
write_build_config and java_libarary_impl(). Otherwise it's really hard to
figure out the data flow.

[commit-bot: I haz the power, 2016-12-06 21:25:22]

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

[commit-bot: I haz the power, 2016-12-06 21:25:23]

Dry run: This issue passed the CQ dry run.

[Peter Wen, 2016-12-06 22:05:45]

PTAL, do we prefer gating on the same condition "if
(defined(invoker.java_files))" or whether the exact variable exists? "if
(defined(_java_sources_file))"

https://codereview.chromium.org/2545793002/diff/40001/build/config/android/ru...
File build/config/android/rules.gni (right):

https://codereview.chromium.org/2545793002/diff/40001/build/config/android/ru...
build/config/android/rules.gni:1437: java_sources_file = "$base_path.sources"
On 2016/12/06 20:25:59, agrieve wrote:
> nit: Please make this _java_sources_file and explicitly pass it to
> write_build_config and java_libarary_impl(). Otherwise it's really hard to
> figure out the data flow.

Done

[agrieve, 2016-12-07 18:23:11]

On 2016/12/06 22:05:45, Peter Wen wrote:
> PTAL, do we prefer gating on the same condition "if
> (defined(invoker.java_files))" or whether the exact variable exists? "if
> (defined(_java_sources_file))"
> 
>
https://codereview.chromium.org/2545793002/diff/40001/build/config/android/ru...
> File build/config/android/rules.gni (right):
> 
>
https://codereview.chromium.org/2545793002/diff/40001/build/config/android/ru...
> build/config/android/rules.gni:1437: java_sources_file = "$base_path.sources"
> On 2016/12/06 20:25:59, agrieve wrote:
> > nit: Please make this _java_sources_file and explicitly pass it to
> > write_build_config and java_libarary_impl(). Otherwise it's really hard to
> > figure out the data flow.
> 
> Done

lgtm

[Peter Wen, 2016-12-07 18:34:32]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2016-12-07 18:34:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 20:16:59]

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

[commit-bot: I haz the power, 2016-12-07 20:17:00]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Peter Wen, 2016-12-07 20:20:04]

The CQ bit was checked by wnwen@chromium.org

[commit-bot: I haz the power, 2016-12-07 20:20:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 21:12:01]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481142004753870,
"parent_rev": "85ec054b35102b1b5b82677b483a3e51ecf615bb", "commit_rev":
"74d52b960d16eb94ac2388f8746a9ceb910dba5f"}

[commit-bot: I haz the power, 2016-12-07 21:12:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-07 21:15:14]

Description was changed from

==========
Android: Add java files from android_apk targets

This allows editing android_webview_apk and android_webview_test_apk
files in android studio.

BUG=670044
==========

to

==========
Android: Add java files from android_apk targets

This allows editing android_webview_apk and android_webview_test_apk
files in android studio.

BUG=670044

Committed: https://crrev.com/c3bace8213461c2a3884b36a3b80aea0e651b15a
Cr-Commit-Position: refs/heads/master@{#437056}
==========

[commit-bot: I haz the power, 2016-12-07 21:15:15]

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