GN WebView: make system_webview_apk work

GN WebView: make system_webview_apk work

This patch
- made shared_resources works
- not compress lpak
- move some code to gni files, so they could be reused in downstream.

BUG=532905
Committed: https://crrev.com/372730a1425415ada0fb77e8652357fc9bcb63a0
Cr-Commit-Position: refs/heads/master@{#360183}

[michaelbai, 2015-11-16 23:51:30]

Description was changed from

==========
GN WebView: make system_webview_apk work

This patch
- made shared_resources works
- not compress lpak
- move some code to gni files, so they could be reused in downstream.

BUG=532905
==========

to

==========
GN WebView: make system_webview_apk work

This patch
- made shared_resources works
- not compress lpak
- move some code to gni files, so they could be reused in downstream.

BUG=532905
==========

[michaelbai, 2015-11-16 23:51:31]

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

[agrieve, 2015-11-17 03:05:26]

If the downstream change that necessitates this is ready, it'd be helpful to see
it at the same time.

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
File android_webview/android_webview_common.gni (right):

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
android_webview/android_webview_common.gni:1: # Copyright 2015 The Chromium
Authors. All rights reserved.
There's already a directory called "android_webview/common, so calling this
android_webview_common.gni, and having it not live within common/ is a bit
confusing.

How about following chrome_public_apk_tmpl.gni, and calling this
"system_webview_apk_tmpl.gni?

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
android_webview/android_webview_common.gni:8: libwebviewchromium_path =
"$root_build_dir/libwebviewchromium.so"
Doesn't look like this is being used outside of this file. Maybe move it inside
the template or inline it.

Also - when used as a parameter to native_libs, the path is ignored, so no need
to specify $root_build_dir.

https://codereview.chromium.org/1452893002/diff/1/build/config/android/rules.gni
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1452893002/diff/1/build/config/android/rules....
build/config/android/rules.gni:1638: extensions_to_not_compress =
".dat,.bin,.pak,.lpak"
The default for this value in GYP is .dat,.bin,.pak, so I'd prefer to leave the
GN default like this as well

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

PTAL 

the downstream change is 
https://chrome-internal-review.googlesource.com/#/c/238800/

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
File android_webview/android_webview_common.gni (right):

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
android_webview/android_webview_common.gni:1: # Copyright 2015 The Chromium
Authors. All rights reserved.
On 2015/11/17 03:05:26, agrieve wrote:
> There's already a directory called "android_webview/common, so calling this
> android_webview_common.gni, and having it not live within common/ is a bit
> confusing.
> 
> How about following chrome_public_apk_tmpl.gni, and calling this
> "system_webview_apk_tmpl.gni?

This file had some variable shared between upstream and downstream, like
asset_dir, but there is no variable now, it should be named as
system_webview_apk_tmp.gni

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
android_webview/android_webview_common.gni:8: libwebviewchromium_path =
"$root_build_dir/libwebviewchromium.so"
On 2015/11/17 03:05:26, agrieve wrote:
> Doesn't look like this is being used outside of this file. Maybe move it
inside
> the template or inline it.
> 
> Also - when used as a parameter to native_libs, the path is ignored, so no
need
> to specify $root_build_dir.

Does this because the library is put in $root_build_dir? how about if I put the
library in other directory like $root_build_dir/lib/?

https://codereview.chromium.org/1452893002/diff/1/build/config/android/rules.gni
File build/config/android/rules.gni (right):

https://codereview.chromium.org/1452893002/diff/1/build/config/android/rules....
build/config/android/rules.gni:1638: extensions_to_not_compress =
".dat,.bin,.pak,.lpak"
On 2015/11/17 03:05:26, agrieve wrote:
> The default for this value in GYP is .dat,.bin,.pak, so I'd prefer to leave
the
> GN default like this as well

Done.

[agrieve, 2015-11-17 19:30:59]

lgtm

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
File android_webview/android_webview_common.gni (right):

https://codereview.chromium.org/1452893002/diff/1/android_webview/android_web...
android_webview/android_webview_common.gni:8: libwebviewchromium_path =
"$root_build_dir/libwebviewchromium.so"
On 2015/11/17 17:51:11, michaelbai wrote:
> On 2015/11/17 03:05:26, agrieve wrote:
> > Doesn't look like this is being used outside of this file. Maybe move it
> inside
> > the template or inline it.
> > 
> > Also - when used as a parameter to native_libs, the path is ignored, so no
> need
> > to specify $root_build_dir.
> 
> Does this because the library is put in $root_build_dir? how about if I put
the
> library in other directory like $root_build_dir/lib/?

Correct. It does not currently support having libs in non-default directories,
but if there's a need for it we could probably make it work. Relevant code here:
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro...

https://codereview.chromium.org/1452893002/diff/20001/android_webview/system_...
File android_webview/system_webview_apk_tmpl.gni (right):

https://codereview.chromium.org/1452893002/diff/20001/android_webview/system_...
android_webview/system_webview_apk_tmpl.gni:18: chromium_code = true
nit: this is the default. Can just remove it.

[michaelbai, 2015-11-17 20:23:29]

The CQ bit was checked by michaelbai@chromium.org

[michaelbai, 2015-11-17 20:23:29]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/1452893002/#ps40001
(title: "Remove .gn change")

[commit-bot: I haz the power, 2015-11-17 20:24:30]

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

[michaelbai, 2015-11-17 20:24:36]

The CQ bit was unchecked by michaelbai@chromium.org

[michaelbai, 2015-11-17 21:50:29]

The CQ bit was checked by michaelbai@chromium.org

[michaelbai, 2015-11-17 21:50:30]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/1452893002/#ps60001
(title: "Remove chromium_code")

[commit-bot: I haz the power, 2015-11-17 21:50:48]

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

[commit-bot: I haz the power, 2015-11-17 22:37:11]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-11-17 22:38:48]

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