Create Monochrome-specific repack() targets

Create Monochrome-specific repack() targets

This allows Monochrome.apk's .pak files to properly contain the
union of Chrome and WebView's .pak files. It also allows it to use
a target-specific resource whitelist.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032, 634358
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a5d84d504167c0fb7452dec22c7f024f8fcde072
Cr-Commit-Position: refs/heads/master@{#419960}

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

Description was changed from

==========
Create Monochrome-specific repack() targets

While it should generally be true that Monochrome's resources are the
same as ChromePublic's, if it ever becomes the case that WebView uses a
resource that Chrome doesn't, Monochrome would fail to have the resource
at runtime because it's using ChromePublic's resource whitelist rather
than its own.

This change defines repack() targets that are monochrome-specific in
order to ensure that resource whitelists never cause a resource to be
stripped that Monochrome actually uses.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

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

to

==========
Create Monochrome-specific repack() targets

While it should generally be true that Monochrome's resources are the
same as ChromePublic's, if it ever becomes the case that WebView uses a
resource that Chrome doesn't, Monochrome would fail to have the resource
at runtime because it's using ChromePublic's resource whitelist rather
than its own.

This change defines repack() targets that are monochrome-specific in
order to ensure that resource whitelists never cause a resource to be
stripped that Monochrome actually uses.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[agrieve, 2016-09-20 01:32:34]

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

[agrieve, 2016-09-20 01:33:31]

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

[commit-bot: I haz the power, 2016-09-20 01:33:36]

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

[agrieve, 2016-09-20 02:01:49]

On 2016/09/20 01:33:32, agrieve wrote:

Also note: I verified that the resources.pak and chrome_100_percent.pak file
sizes are the same before & after this change. This means that as of now,
webview does not use any resources from these pak files that chrome does not use
as well.

[michaelbai, 2016-09-20 02:09:38]

I am sorry to say your assumption is not true, see my comment, it is probably
critical for your patch.

Another issue is the monochrome specific change in android_webview/BUILD.gn
should be in chrome/android directory, because Monochrome is more like Chrome
than WebView.

https://codereview.chromium.org/2354803002/diff/1/chrome/chrome_paks.gni
File chrome/chrome_paks.gni (left):

https://codereview.chromium.org/2354803002/diff/1/chrome/chrome_paks.gni#oldc...
chrome/chrome_paks.gni:189: deps += [ "//android_webview:generate_aw_resources"
]
Here is bug see https://crbug.com/634358 , basically, aw_resources.pak is
WebView specific and should be part of Chrome resources pack. Someone put here
wrongly during my vacation, I were working on the fix, but didn't get time to
continue, Do you think if you can move this out of chrome resources.pak in your
patch?

this is main difference between chrome and monochrome resources.

[commit-bot: I haz the power, 2016-09-20 02:09:59]

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

[commit-bot: I haz the power, 2016-09-20 02:10:00]

Dry run: This issue passed the CQ dry run.

[agrieve, 2016-09-20 15:59:52]

Description was changed from

==========
Create Monochrome-specific repack() targets

While it should generally be true that Monochrome's resources are the
same as ChromePublic's, if it ever becomes the case that WebView uses a
resource that Chrome doesn't, Monochrome would fail to have the resource
at runtime because it's using ChromePublic's resource whitelist rather
than its own.

This change defines repack() targets that are monochrome-specific in
order to ensure that resource whitelists never cause a resource to be
stripped that Monochrome actually uses.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Create Monochrome-specific repack() targets

This allows Monochrome.apk's .pak files to properly contain the
union of Chrome and WebView's .pak files. It also allows it to use
a target-specific resource whitelist.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032,634358
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[agrieve, 2016-09-20 16:03:22]

Thanks for the insights! Make the change more useful :P.
I've moved to chrome/android and removed aw_resources.pak as you suggested.

https://codereview.chromium.org/2354803002/diff/1/chrome/chrome_paks.gni
File chrome/chrome_paks.gni (left):

https://codereview.chromium.org/2354803002/diff/1/chrome/chrome_paks.gni#oldc...
chrome/chrome_paks.gni:189: deps += [ "//android_webview:generate_aw_resources"
]
On 2016/09/20 02:09:38, michaelbai wrote:
> Here is bug see https://crbug.com/634358 , basically, aw_resources.pak is
> WebView specific and should be part of Chrome resources pack. Someone put here
> wrongly during my vacation, I were working on the fix, but didn't get time to
> continue, Do you think if you can move this out of chrome resources.pak in
your
> patch?
> 
> this is main difference between chrome and monochrome resources.

Done.

[michaelbai, 2016-09-20 18:49:43]

LGTM with 2 comments

https://codereview.chromium.org/2354803002/diff/20001/android_webview/BUILD.gn
File android_webview/BUILD.gn (left):

https://codereview.chromium.org/2354803002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:9:
import("//tools/resources/generate_resource_whitelist.gni")
Is this one still needed by Standalone webview?

https://codereview.chromium.org/2354803002/diff/20001/chrome/chrome_paks.gni
File chrome/chrome_paks.gni (right):

https://codereview.chromium.org/2354803002/diff/20001/chrome/chrome_paks.gni#...
chrome/chrome_paks.gni:219: chrome_repack_locales("${target_name}_locales") {
Monochrome has its own set of locales, currently implemented by
alternative_locale_resource, it could be implemented in this way, but don't need
to be done in this patch.

[agrieve, 2016-09-20 19:11:11]

https://codereview.chromium.org/2354803002/diff/20001/android_webview/BUILD.gn
File android_webview/BUILD.gn (left):

https://codereview.chromium.org/2354803002/diff/20001/android_webview/BUILD.g...
android_webview/BUILD.gn:9:
import("//tools/resources/generate_resource_whitelist.gni")
On 2016/09/20 18:49:42, michaelbai wrote:
> Is this one still needed by Standalone webview?

Done.

https://codereview.chromium.org/2354803002/diff/20001/chrome/chrome_paks.gni
File chrome/chrome_paks.gni (right):

https://codereview.chromium.org/2354803002/diff/20001/chrome/chrome_paks.gni#...
chrome/chrome_paks.gni:219: chrome_repack_locales("${target_name}_locales") {
On 2016/09/20 18:49:43, michaelbai wrote:
> Monochrome has its own set of locales, currently implemented by
> alternative_locale_resource, it could be implemented in this way, but don't
need
> to be done in this patch.

Acknowledged. I'll look at doing this in a follow-up.

[agrieve, 2016-09-20 19:11:44]

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

[agrieve, 2016-09-20 19:11:44]

brettw@ for other resource changes.

[brettw, 2016-09-20 19:21:32]

lgtm

https://codereview.chromium.org/2354803002/diff/40001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2354803002/diff/40001/chrome/BUILD.gn#newcode...
chrome/BUILD.gn:1351: # GYP version:
chrome/chrome_resources.gyp:browser_tests_pak
We shouldn't be adding these GYP annotations any more (I've been trying to
delete them).

[agrieve, 2016-09-20 19:23:52]

https://codereview.chromium.org/2354803002/diff/40001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2354803002/diff/40001/chrome/BUILD.gn#newcode...
chrome/BUILD.gn:1351: # GYP version:
chrome/chrome_resources.gyp:browser_tests_pak
On 2016/09/20 19:21:32, brettw (ping on IM after 24h) wrote:
> We shouldn't be adding these GYP annotations any more (I've been trying to
> delete them).

Done.

[agrieve, 2016-09-20 19:47:26]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-09-20 19:47:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelbai@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2354803002/#ps60001
(title: "remove GYP comment")

[commit-bot: I haz the power, 2016-09-20 19:48:07]

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 20:19:39]

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

[commit-bot: I haz the power, 2016-09-20 20:19:40]

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

[agrieve, 2016-09-21 00:24:18]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-09-21 00:24:45]

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-21 00:29:39]

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

[commit-bot: I haz the power, 2016-09-21 00:29:40]

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

[agrieve, 2016-09-21 01:10:20]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-09-21 01:10:21]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelbai@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2354803002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-09-21 01:11:24]

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-21 03:07:22]

Description was changed from

==========
Create Monochrome-specific repack() targets

This allows Monochrome.apk's .pak files to properly contain the
union of Chrome and WebView's .pak files. It also allows it to use
a target-specific resource whitelist.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032,634358
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Create Monochrome-specific repack() targets

This allows Monochrome.apk's .pak files to properly contain the
union of Chrome and WebView's .pak files. It also allows it to use
a target-specific resource whitelist.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032,634358
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-09-21 03:07:23]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-09-21 03:09:34]

Description was changed from

==========
Create Monochrome-specific repack() targets

This allows Monochrome.apk's .pak files to properly contain the
union of Chrome and WebView's .pak files. It also allows it to use
a target-specific resource whitelist.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032,634358
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Create Monochrome-specific repack() targets

This allows Monochrome.apk's .pak files to properly contain the
union of Chrome and WebView's .pak files. It also allows it to use
a target-specific resource whitelist.

This change applies to resources.pak and chrome_100_percent.pak. A
change to language .pak files will come as a follow-up.

BUG=641032,634358
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-09-21 03:09:36]

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

[Benoit L, 2016-09-21 09:11:00]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/2353553004/ by lizeb@chromium.org.

The reason for reverting is: Breaks ARM64 downstream bot (crbug.com/648878)..