Add resource whitelisting to android_webview pak files.

Add resource whitelisting to android_webview pak files.

This saves only 5kb off of webview's .apk.

BUG=641032
Committed: https://crrev.com/425d550f2c0b578e8942ccd5fc257d3d35689125
Cr-Commit-Position: refs/heads/master@{#419622}

[agrieve, 2016-09-16 19:39:03]

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

[agrieve, 2016-09-16 19:39:04]

Not really saving much, but at least it's in place now. It should save much more
for monochrome.

[estevenson, 2016-09-17 19:01:55]

On 2016/09/16 19:39:04, agrieve wrote:
> Not really saving much, but at least it's in place now. It should save much
more
> for monochrome.

Is //tools/resources/generate_resource_whitelist.gni supposed to be included in
this CL? Thanks for the build refactor + per target whitelisting stuff btw, much
cleaner now!

[agrieve, 2016-09-19 14:03:09]

On 2016/09/17 19:01:55, estevenson wrote:
> On 2016/09/16 19:39:04, agrieve wrote:
> > Not really saving much, but at least it's in place now. It should save much
> more
> > for monochrome.
> 
> Is //tools/resources/generate_resource_whitelist.gni supposed to be included
in
> this CL? Thanks for the build refactor + per target whitelisting stuff btw,
much
> cleaner now!

Doh! Yes. Added now.

[agrieve, 2016-09-19 18:55:15]

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

[agrieve, 2016-09-19 18:55:16]

thakis@chromium.org: Please review changes in tools/

[michaelbai, 2016-09-19 19:06:14]

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

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:435: input =
"$root_out_dir/libwebviewchromium$shlib_extension.whitelist"
Sorry, I missed context here, could you show me where shlib_extension.whitelist
is defined and how lib_foo.$shlib_extension.whitelist is generated? I searched
code and can't figure it out.

[agrieve, 2016-09-19 19:10:33]

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

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:435: input =
"$root_out_dir/libwebviewchromium$shlib_extension.whitelist"
On 2016/09/19 19:06:13, michaelbai wrote:
> Sorry, I missed context here, could you show me where
shlib_extension.whitelist
> is defined and how lib_foo.$shlib_extension.whitelist is generated? I searched
> code and can't figure it out.

Searching for "enable_resource_whitelist_generation" is the key here.
This causes whitelists to be created as a part of compilation:
https://cs.chromium.org/chromium/src/build/toolchain/gcc_toolchain.gni?q=enab...

It doesn't solve the problem of excessive pragma warnings, but does at least
scope usages to specific .o files. Then, when linking the .o.whitelist files are
concatenated together.

This final generate_resource_whitelist() step actually just adds in 2 hard-coded
resource IDs on top of the .so.whitelist one.

[Nico, 2016-09-19 20:27:49]

lgtm, but:

https://codereview.chromium.org/2343083003/diff/40001/tools/resources/generat...
File tools/resources/generate_resource_whitelist.gni (right):

https://codereview.chromium.org/2343083003/diff/40001/tools/resources/generat...
tools/resources/generate_resource_whitelist.gni:4: 
can you explain some more what this actually does? pretend someone not familiar
with resource whitelisting or android looks at this file to understand what it
does

[michaelbai, 2016-09-19 21:02:06]

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

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:294: whitelist =
rebase_path("ui/grit_components_whitelist.txt")
Here, we manually add whitelist, would you mind to find out why ui directory has
this list? Should it be removed? what's relationship between this and your
change?

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:435: input =
"$root_out_dir/libwebviewchromium$shlib_extension.whitelist"
On 2016/09/19 19:10:32, agrieve wrote:
> On 2016/09/19 19:06:13, michaelbai wrote:
> > Sorry, I missed context here, could you show me where
> shlib_extension.whitelist
> > is defined and how lib_foo.$shlib_extension.whitelist is generated? I
searched
> > code and can't figure it out.
> 
> Searching for "enable_resource_whitelist_generation" is the key here.
> This causes whitelists to be created as a part of compilation:
>
https://cs.chromium.org/chromium/src/build/toolchain/gcc_toolchain.gni?q=enab...
> 
> It doesn't solve the problem of excessive pragma warnings, but does at least
> scope usages to specific .o files. Then, when linking the .o.whitelist files
are
> concatenated together.
> 
> This final generate_resource_whitelist() step actually just adds in 2
hard-coded
> resource IDs on top of the .so.whitelist one.

Thanks for your explanation, now I understand how it works, but still have one
question, is shlib_extension.whitelist defined anywhere?

[agrieve, 2016-09-20 00:48:34]

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

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:294: whitelist =
rebase_path("ui/grit_components_whitelist.txt")
On 2016/09/19 21:02:06, michaelbai wrote:
> Here, we manually add whitelist, would you mind to find out why ui directory
has
> this list? Should it be removed? what's relationship between this and your
> change?

Wow, didn't even notice this. Tested removing it, and it looks like it still
does a lot. This means that it strips out resources that are unreached, but
still seen by the compiler because they are not #ifdef'ed out. Added a comment.

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:435: input =
"$root_out_dir/libwebviewchromium$shlib_extension.whitelist"
On 2016/09/19 21:02:06, michaelbai wrote:
> On 2016/09/19 19:10:32, agrieve wrote:
> > On 2016/09/19 19:06:13, michaelbai wrote:
> > > Sorry, I missed context here, could you show me where
> > shlib_extension.whitelist
> > > is defined and how lib_foo.$shlib_extension.whitelist is generated? I
> searched
> > > code and can't figure it out.
> > 
> > Searching for "enable_resource_whitelist_generation" is the key here.
> > This causes whitelists to be created as a part of compilation:
> >
>
https://cs.chromium.org/chromium/src/build/toolchain/gcc_toolchain.gni?q=enab...
> > 
> > It doesn't solve the problem of excessive pragma warnings, but does at least
> > scope usages to specific .o files. Then, when linking the .o.whitelist files
> are
> > concatenated together.
> > 
> > This final generate_resource_whitelist() step actually just adds in 2
> hard-coded
> > resource IDs on top of the .so.whitelist one.
> 
> Thanks for your explanation, now I understand how it works, but still have one
> question, is shlib_extension.whitelist defined anywhere?

shlib_extension is defined here: build/toolchain/toolchain.gni
.whitelist isn't a part of the expression (it's the file extension)

https://codereview.chromium.org/2343083003/diff/40001/tools/resources/generat...
File tools/resources/generate_resource_whitelist.gni (right):

https://codereview.chromium.org/2343083003/diff/40001/tools/resources/generat...
tools/resources/generate_resource_whitelist.gni:4: 
On 2016/09/19 20:27:49, Nico wrote:
> can you explain some more what this actually does? pretend someone not
familiar
> with resource whitelisting or android looks at this file to understand what it
> does

Done.

[michaelbai, 2016-09-20 01:01:01]

lgtm

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

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:294: whitelist =
rebase_path("ui/grit_components_whitelist.txt")
On 2016/09/20 00:48:34, agrieve wrote:
> On 2016/09/19 21:02:06, michaelbai wrote:
> > Here, we manually add whitelist, would you mind to find out why ui directory
> has
> > this list? Should it be removed? what's relationship between this and your
> > change?
> 
> Wow, didn't even notice this. Tested removing it, and it looks like it still
> does a lot. This means that it strips out resources that are unreached, but
> still seen by the compiler because they are not #ifdef'ed out. Added a
comment.


It probably means we could do more to remove the unused resources? Will file a
bug on this.

[agrieve, 2016-09-20 01:08:58]

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

https://codereview.chromium.org/2343083003/diff/40001/android_webview/BUILD.g...
android_webview/BUILD.gn:294: whitelist =
rebase_path("ui/grit_components_whitelist.txt")
On 2016/09/20 01:01:01, michaelbai wrote:
> On 2016/09/20 00:48:34, agrieve wrote:
> > On 2016/09/19 21:02:06, michaelbai wrote:
> > > Here, we manually add whitelist, would you mind to find out why ui
directory
> > has
> > > this list? Should it be removed? what's relationship between this and your
> > > change?
> > 
> > Wow, didn't even notice this. Tested removing it, and it looks like it still
> > does a lot. This means that it strips out resources that are unreached, but
> > still seen by the compiler because they are not #ifdef'ed out. Added a
> comment.
> 
> 
> It probably means we could do more to remove the unused resources? Will file a
> bug on this.

It's certainly possible. Right now if the compiler sees an ID at all, it marks
it as used. Even if it's in dead code.

[agrieve, 2016-09-20 01:09:04]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-09-20 01:09:04]

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

[commit-bot: I haz the power, 2016-09-20 01:10:06]

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-09-20 01:21:51]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-20 01:23:34]

Description was changed from

==========
Add resource whitelisting to android_webview pak files.

This saves only 5kb off of webview's .apk.

BUG=641032
==========

to

==========
Add resource whitelisting to android_webview pak files.

This saves only 5kb off of webview's .apk.

BUG=641032

Committed: https://crrev.com/425d550f2c0b578e8942ccd5fc257d3d35689125
Cr-Commit-Position: refs/heads/master@{#419622}
==========

[commit-bot: I haz the power, 2016-09-20 01:23:35]

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