Start adding gn build for android_webview

Start adding gn build for android_webview

This patch added targets
//android_webview:string_grd
//android_webview:android_webview_java
//android_webview/native:native

BUG=532905
Committed: https://crrev.com/6da58b3abac8f1ad73cd9207be0667d8d2516833
Cr-Commit-Position: refs/heads/master@{#354869}

[michaelbai, 2015-10-17 00:37:41]

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

[agrieve, 2015-10-17 01:05:59]

lgtm. Looks great!

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:11: group("system_webview_apk") {
The convention for these types of groups would be to give it the same name as
the directory: "android_webview".

This will allow referring to it without a colon: //android_webview

[michaelbai, 2015-10-19 15:55:46]

michaelbai@chromium.org changed reviewers:
+ qinmin@chromium.org

[michaelbai, 2015-10-19 15:56:30]

qinmin for components

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:11: group("system_webview_apk") {
On 2015/10/17 01:05:59, agrieve wrote:
> The convention for these types of groups would be to give it the same name as
> the directory: "android_webview".
> 
> This will allow referring to it without a colon: //android_webview

Acknowledged.

[qinmin, 2015-10-19 17:34:41]

lgtm

[qinmin, 2015-10-19 17:34:44]

lgtm

[michaelbai, 2015-10-19 17:38:50]

The CQ bit was checked by michaelbai@chromium.org

[commit-bot: I haz the power, 2015-10-19 17:39:00]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410203002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410203002/20001

[commit-bot: I haz the power, 2015-10-19 17:49:12]

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

[commit-bot: I haz the power, 2015-10-19 17:49:13]

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

[michaelbai, 2015-10-19 17:51:11]

michaelbai@chromium.org changed reviewers:
+ brettw@chromium.org

[michaelbai, 2015-10-19 17:51:54]

michaelbai@chromium.org changed reviewers:
+ dpranke@google.com
- brettw@chromium.org

[michaelbai, 2015-10-19 17:51:54]

dpranke@ would you like help to review BUILD.gn?

[Dirk Pranke, 2015-10-19 19:18:04]

dpranke@chromium.org changed reviewers:
+ dpranke@chromium.org

[Dirk Pranke, 2015-10-19 19:18:04]

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:15: ":android_webview_java",
targets defined elsewhere in the file should be sorted before targets in other
files. Did you run `gn format` on this file? If it didn't automatically do that,
would you mind filing a bug for this?

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:31: srcjar_deps = [
"//android_webview/native:aw_permission_request_resource" ]
generally we leave blank lines in between multi-line blocks like deps, sources,
java_files, etc. It makes it a little easier to read.

I would ask the same comment about `gn format` catching that ...

https://codereview.chromium.org/1410203002/diff/20001/android_webview/native/...
File android_webview/native/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/android_webview/native/...
android_webview/native/BUILD.gn:31: "../..",
"../.." should be included automatically (and hence not needed here).

Please use src-relative paths like "//skia/config" instead of relative paths
where possible for paths that are not subdirectories of the current directory.

https://codereview.chromium.org/1410203002/diff/20001/components/external_vid...
File components/external_video_surface/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/components/external_vid...
components/external_video_surface/BUILD.gn:9:
static_library("external_video_surface") {
I realize this is an unrelated thing, but creating empty static_libraries() is
problematic, and we generally prefer source_sets() to static libraries
anyway.

Can you change this to a source_set()?

[michaelbai, 2015-10-19 19:56:48]

PTAL

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:15: ":android_webview_java",
On 2015/10/19 19:18:04, Dirk Pranke wrote:
> targets defined elsewhere in the file should be sorted before targets in other
> files. Did you run `gn format` on this file? If it didn't automatically do
that,
> would you mind filing a bug for this?

All BUILD.gn pass gn format, file bug crbug.com/545128

https://codereview.chromium.org/1410203002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:31: srcjar_deps = [
"//android_webview/native:aw_permission_request_resource" ]
On 2015/10/19 19:18:04, Dirk Pranke wrote:
> generally we leave blank lines in between multi-line blocks like deps,
sources,
> java_files, etc. It makes it a little easier to read.
> 
> I would ask the same comment about `gn format` catching that ...

file bug crbug.com/545129

https://codereview.chromium.org/1410203002/diff/20001/android_webview/native/...
File android_webview/native/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/android_webview/native/...
android_webview/native/BUILD.gn:31: "../..",
On 2015/10/19 19:18:04, Dirk Pranke wrote:
> "../.." should be included automatically (and hence not needed here).
> 
> Please use src-relative paths like "//skia/config" instead of relative paths
> where possible for paths that are not subdirectories of the current directory.

Done.

https://codereview.chromium.org/1410203002/diff/20001/components/external_vid...
File components/external_video_surface/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/20001/components/external_vid...
components/external_video_surface/BUILD.gn:9:
static_library("external_video_surface") {
On 2015/10/19 19:18:04, Dirk Pranke wrote:
> I realize this is an unrelated thing, but creating empty static_libraries() is
> problematic, and we generally prefer source_sets() to static libraries
> anyway.
> 
> Can you change this to a source_set()?

Done.

[Dirk Pranke, 2015-10-19 19:59:12]

lgtm w/ the comments below addressed, thanks!

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:11: group("system_webview_apk") {
Did you forget to rename this?

https://codereview.chromium.org/1410203002/diff/40001/android_webview/native/...
File android_webview/native/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/40001/android_webview/native/...
android_webview/native/BUILD.gn:30: include_dirs = [ "//skia/config" ]
nit: add blank lines?

[michaelbai, 2015-10-19 20:35:58]

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:11: group("system_webview_apk") {
On 2015/10/19 19:59:12, Dirk Pranke wrote:
> Did you forget to rename this?

I want to keep this, as I will have system_webview_apk target finally, and don't
want to change src/BUILD.gn again

https://codereview.chromium.org/1410203002/diff/40001/android_webview/native/...
File android_webview/native/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/40001/android_webview/native/...
android_webview/native/BUILD.gn:30: include_dirs = [ "//skia/config" ]
On 2015/10/19 19:59:12, Dirk Pranke wrote:
> nit: add blank lines?

Done.

[Dirk Pranke, 2015-10-19 20:42:16]

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:11: group("system_webview_apk") {
On 2015/10/19 20:35:57, michaelbai wrote:
> On 2015/10/19 19:59:12, Dirk Pranke wrote:
> > Did you forget to rename this?
> 
> I want to keep this, as I will have system_webview_apk target finally, and
don't
> want to change src/BUILD.gn again

I see, you're saying eventually this will be android_apk("system_webview_apk"),
but since that doesn't build yet, for now you're declaring this to be a group
... ?

[michaelbai, 2015-10-19 20:43:26]

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.gn
File android_webview/BUILD.gn (right):

https://codereview.chromium.org/1410203002/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:11: group("system_webview_apk") {
On 2015/10/19 20:42:16, Dirk Pranke wrote:
> On 2015/10/19 20:35:57, michaelbai wrote:
> > On 2015/10/19 19:59:12, Dirk Pranke wrote:
> > > Did you forget to rename this?
> > 
> > I want to keep this, as I will have system_webview_apk target finally, and
> don't
> > want to change src/BUILD.gn again
> 
> I see, you're saying eventually this will be
android_apk("system_webview_apk"),
> but since that doesn't build yet, for now you're declaring this to be a group
> ... ?

Yes, did you see any issue?

[Dirk Pranke, 2015-10-19 20:53:40]

I wouldn't have done it this way, but I can live with it :). lgtm.

[michaelbai, 2015-10-19 20:54:37]

On 2015/10/19 20:53:40, Dirk Pranke wrote:
> I wouldn't have done it this way, but I can live with it :). lgtm.

Thanks, land it

[michaelbai, 2015-10-19 20:54:42]

The CQ bit was checked by michaelbai@chromium.org

[michaelbai, 2015-10-19 20:54:43]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org,
agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/1410203002/#ps60001
(title: "address comment")

[commit-bot: I haz the power, 2015-10-19 20:55:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1410203002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1410203002/60001

[commit-bot: I haz the power, 2015-10-19 21:36:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-19 21:36:48]

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