Support uncompress and page align shared libraries

Support uncompress and page align shared libraries

This is no corresponding gyp change, because the ant apkbuilder
doesn't support uncompress shared libraries, to only have GN
support this meets our requirement.

BUG=579610
Committed: https://crrev.com/e77d565e1b9a8d3a2822b54bbaee1d81f5a67198
Cr-Commit-Position: refs/heads/master@{#371384}

[michaelbai, 2016-01-21 07:27:02]

Description was changed from

==========
Support uncompress and page align shared libraries

This is no corresponding gyp change, because the ant apkbuilder
doesn't support uncompress shared libraries, to only have GN
support this meets our requirement.

BUG=579610
==========

to

==========
Support uncompress and page align shared libraries

This is no corresponding gyp change, because the ant apkbuilder
doesn't support uncompress shared libraries, to only have GN
support this meets our requirement.

BUG=579610
==========

[michaelbai, 2016-01-21 07:27:02]

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

[Yaron, 2016-01-21 20:22:45]

yfriedman@chromium.org changed reviewers:
+ jbudorick@chromium.org, yfriedman@chromium.org
- agrieve@chromium.org

[Yaron, 2016-01-21 20:22:46]

agrive is OOO through next week so took a look. +john to help :)

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
File build/android/gyp/apkbuilder.py (right):

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
build/android/gyp/apkbuilder.py:217: compress = not
(options.uncompress_shared_libraries and
I'm not sure this is exactly right. This is forcing the behaviour in all cases.
I think instead you want that if it's a shared library and your flag is set,
don't zip, otherwise pass none (to mimic previous behaviour)

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
File build/android/gyp/finalize_apk.py (right):

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
build/android/gyp/finalize_apk.py:65: align_cmd += ['-p']
Is there a reason we don't do this always?

https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:1139: forward_variables_from(invoker, [
"page_align_shared_libraries" ])
did you test this? incremental apks have a special way of loading shared
libraries as it is. No idea if is actually needed.

[michaelbai, 2016-01-21 21:58:03]

PTAL

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
File build/android/gyp/apkbuilder.py (right):

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
build/android/gyp/apkbuilder.py:217: compress = not
(options.uncompress_shared_libraries and
On 2016/01/21 20:22:46, Yaron wrote:
> I'm not sure this is exactly right. This is forcing the behaviour in all
cases.
> I think instead you want that if it's a shared library and your flag is set,
> don't zip, otherwise pass none (to mimic previous behaviour)

Thanks, I should keep previous behavior.

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
File build/android/gyp/finalize_apk.py (right):

https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
build/android/gyp/finalize_apk.py:65: align_cmd += ['-p']
On 2016/01/21 20:22:46, Yaron wrote:
> Is there a reason we don't do this always?

For the compressed shared libraries, there is no benefit to page align them, and
the apk size will be increased.

https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
File build/config/android/internal_rules.gni (right):

https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
build/config/android/internal_rules.gni:1139: forward_variables_from(invoker, [
"page_align_shared_libraries" ])
On 2016/01/21 20:22:46, Yaron wrote:
> did you test this? incremental apks have a special way of loading shared
> libraries as it is. No idea if is actually needed. 

No, I didn't test it, actually, I don't know how to test, but comparing to the
similar variable load_library_from_apk, it seemed we don't need this.

[Yaron, 2016-01-21 22:11:04]

On 2016/01/21 21:58:03, michaelbai wrote:
> PTAL
> 
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
> File build/android/gyp/apkbuilder.py (right):
> 
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
> build/android/gyp/apkbuilder.py:217: compress = not
> (options.uncompress_shared_libraries and
> On 2016/01/21 20:22:46, Yaron wrote:
> > I'm not sure this is exactly right. This is forcing the behaviour in all
> cases.
> > I think instead you want that if it's a shared library and your flag is set,
> > don't zip, otherwise pass none (to mimic previous behaviour)
> 
> Thanks, I should keep previous behavior.
> 
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
> File build/android/gyp/finalize_apk.py (right):
> 
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
> build/android/gyp/finalize_apk.py:65: align_cmd += ['-p']
> On 2016/01/21 20:22:46, Yaron wrote:
> > Is there a reason we don't do this always?
> 
> For the compressed shared libraries, there is no benefit to page align them,
and
> the apk size will be increased.
> 
>
https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
> File build/config/android/internal_rules.gni (right):
> 
>
https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
> build/config/android/internal_rules.gni:1139: forward_variables_from(invoker,
[
> "page_align_shared_libraries" ])
> On 2016/01/21 20:22:46, Yaron wrote:
> > did you test this? incremental apks have a special way of loading shared
> > libraries as it is. No idea if is actually needed. 
> 
> No, I didn't test it, actually, I don't know how to test, but comparing to the
> similar variable load_library_from_apk, it seemed we don't need this.

lgtm but please let John look too

[michaelbai, 2016-01-21 22:18:08]

On 2016/01/21 22:11:04, Yaron wrote:
> On 2016/01/21 21:58:03, michaelbai wrote:
> > PTAL
> > 
> >
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
> > File build/android/gyp/apkbuilder.py (right):
> > 
> >
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/apkbuilde...
> > build/android/gyp/apkbuilder.py:217: compress = not
> > (options.uncompress_shared_libraries and
> > On 2016/01/21 20:22:46, Yaron wrote:
> > > I'm not sure this is exactly right. This is forcing the behaviour in all
> > cases.
> > > I think instead you want that if it's a shared library and your flag is
set,
> > > don't zip, otherwise pass none (to mimic previous behaviour)
> > 
> > Thanks, I should keep previous behavior.
> > 
> >
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
> > File build/android/gyp/finalize_apk.py (right):
> > 
> >
>
https://codereview.chromium.org/1619553003/diff/1/build/android/gyp/finalize_...
> > build/android/gyp/finalize_apk.py:65: align_cmd += ['-p']
> > On 2016/01/21 20:22:46, Yaron wrote:
> > > Is there a reason we don't do this always?
> > 
> > For the compressed shared libraries, there is no benefit to page align them,
> and
> > the apk size will be increased.
> > 
> >
>
https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
> > File build/config/android/internal_rules.gni (right):
> > 
> >
>
https://codereview.chromium.org/1619553003/diff/1/build/config/android/intern...
> > build/config/android/internal_rules.gni:1139:
forward_variables_from(invoker,
> [
> > "page_align_shared_libraries" ])
> > On 2016/01/21 20:22:46, Yaron wrote:
> > > did you test this? incremental apks have a special way of loading shared
> > > libraries as it is. No idea if is actually needed. 
> > 
> > No, I didn't test it, actually, I don't know how to test, but comparing to
the
> > similar variable load_library_from_apk, it seemed we don't need this.
> 
> lgtm but please let John look too

Thanks for reviewing this!

[jbudorick, 2016-01-25 15:31:10]

https://codereview.chromium.org/1619553003/diff/20001/build/android/gyp/final...
File build/android/gyp/finalize_apk.py (right):

https://codereview.chromium.org/1619553003/diff/20001/build/android/gyp/final...
build/android/gyp/finalize_apk.py:58: def AlignApk(zipalign_path, package_align,
unaligned_path, final_path):
Please update
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/to...

[michaelbai, 2016-01-25 20:40:59]

Thanks, PTAL

https://codereview.chromium.org/1619553003/diff/20001/build/android/gyp/final...
File build/android/gyp/finalize_apk.py (right):

https://codereview.chromium.org/1619553003/diff/20001/build/android/gyp/final...
build/android/gyp/finalize_apk.py:58: def AlignApk(zipalign_path, package_align,
unaligned_path, final_path):
On 2016/01/25 15:31:10, jbudorick wrote:
> Please update
>
https://code.google.com/p/chromium/codesearch#chromium/src/android_webview/to...

thanks!

[jbudorick, 2016-01-25 22:07:16]

lgtm

[michaelbai, 2016-01-25 22:53:18]

The CQ bit was checked by michaelbai@chromium.org

[michaelbai, 2016-01-25 22:53:18]

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

[commit-bot: I haz the power, 2016-01-25 22:57:47]

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

[commit-bot: I haz the power, 2016-01-26 00:43:19]

Description was changed from

==========
Support uncompress and page align shared libraries

This is no corresponding gyp change, because the ant apkbuilder
doesn't support uncompress shared libraries, to only have GN
support this meets our requirement.

BUG=579610
==========

to

==========
Support uncompress and page align shared libraries

This is no corresponding gyp change, because the ant apkbuilder
doesn't support uncompress shared libraries, to only have GN
support this meets our requirement.

BUG=579610
==========

[commit-bot: I haz the power, 2016-01-26 00:43:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-01-26 00:44:15]

Description was changed from

==========
Support uncompress and page align shared libraries

This is no corresponding gyp change, because the ant apkbuilder
doesn't support uncompress shared libraries, to only have GN
support this meets our requirement.

BUG=579610
==========

to

==========
Support uncompress and page align shared libraries

This is no corresponding gyp change, because the ant apkbuilder
doesn't support uncompress shared libraries, to only have GN
support this meets our requirement.

BUG=579610

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

[commit-bot: I haz the power, 2016-01-26 00:44:16]

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