Merge webviewchromium.pak and resources.pak in Monochrome

Merge webviewchromium.pak and resources.pak in Monochrome

This CL adds the webview specific resources to the Android
version of resources.pak (adding roughly 2.1KB to it), and
lets Webview read its resources from resources.pak if
webviewchromium.pak doesn't exist.

The following downstream CL will remove webviewchromium.pak
from the Monochrome APk, reducing the size of this by 588KB.

BUG=634358
Committed: https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261
Cr-Commit-Position: refs/heads/master@{#411066}

[aberent, 2016-08-10 13:48:16]

aberent@chromium.org changed reviewers:
+ jbudorick@chromium.org, sky@chromium.org, torne@chromium.org

[aberent, 2016-08-10 13:48:16]

torne@chromium.org: Please review all changes, including webview changes as
owner

sky@chromium.org: Owner review of resource_bundle_android.[ch] changes please.

jbudorick@chromium.org: Owner review of BUILD.gn please.

[Torne, 2016-08-10 13:52:54]

lgtm

https://codereview.chromium.org/2235753002/diff/1/android_webview/browser/aw_...
File android_webview/browser/aw_browser_main_parts.cc (right):

https://codereview.chromium.org/2235753002/diff/1/android_webview/browser/aw_...
android_webview/browser/aw_browser_main_parts.cc:116: if
(!ui::LoadMainAndroidPackFile("assets/webviewchromium.pak",
We don't have to do this in this CL, but how difficult would it be to just
always name the pak file the same thing (i.e. always resources.pak)?

I guess the build targets need to have distinct names, but it could end up in
the APK with the same name. If this isn't too fiddly it might be worth doing it
later?

[jbudorick, 2016-08-10 14:44:08]

BUILD.gn lgmt w/ nit

https://codereview.chromium.org/2235753002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2235753002/diff/1/chrome/BUILD.gn#newcode1411
chrome/BUILD.gn:1411: if (is_android) {
nit: blank line before this if block, no blank line after

[jbudorick, 2016-08-10 14:44:54]

On 2016/08/10 14:44:08, jbudorick wrote:
> BUILD.gn lgmt w/ nit

when you try to do code reviews before coffee, you misspell lgtm

> 
> https://codereview.chromium.org/2235753002/diff/1/chrome/BUILD.gn
> File chrome/BUILD.gn (right):
> 
> https://codereview.chromium.org/2235753002/diff/1/chrome/BUILD.gn#newcode1411
> chrome/BUILD.gn:1411: if (is_android) {
> nit: blank line before this if block, no blank line after

[sky, 2016-08-10 15:14:51]

LGTM

[sky, 2016-08-10 15:15:10]

Also, we should really have better owners of the resource_bundle_android.

[aberent, 2016-08-10 15:31:14]

The CQ bit was checked by aberent@chromium.org

[aberent, 2016-08-10 15:31:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org, sky@chromium.org, torne@chromium.org
Link to the patchset: https://codereview.chromium.org/2235753002/#ps20001
(title: "Fix nit")

[commit-bot: I haz the power, 2016-08-10 15:31:35]

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

[aberent, 2016-08-10 15:34:57]

The CQ bit was unchecked by aberent@chromium.org

[aberent, 2016-08-10 16:04:20]

https://codereview.chromium.org/2235753002/diff/1/android_webview/browser/aw_...
File android_webview/browser/aw_browser_main_parts.cc (right):

https://codereview.chromium.org/2235753002/diff/1/android_webview/browser/aw_...
android_webview/browser/aw_browser_main_parts.cc:116: if
(!ui::LoadMainAndroidPackFile("assets/webviewchromium.pak",
On 2016/08/10 13:52:54, Torne wrote:
> We don't have to do this in this CL, but how difficult would it be to just
> always name the pak file the same thing (i.e. always resources.pak)?
> 
> I guess the build targets need to have distinct names, but it could end up in
> the APK with the same name. If this isn't too fiddly it might be worth doing
it
> later?

I don't know; I considered investigating and decided not to. I think, to avoid
file clashes when building all, one would have to change the file name (or the
directory) in the final step of building the APK, but I don't know how easy or
difficult this would be.

https://codereview.chromium.org/2235753002/diff/1/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2235753002/diff/1/chrome/BUILD.gn#newcode1411
chrome/BUILD.gn:1411: if (is_android) {
On 2016/08/10 14:44:07, jbudorick wrote:
> nit: blank line before this if block, no blank line after

Done.

[aberent, 2016-08-10 16:04:30]

The CQ bit was checked by aberent@chromium.org

[aberent, 2016-08-10 16:04:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org, sky@chromium.org, torne@chromium.org
Link to the patchset: https://codereview.chromium.org/2235753002/#ps40001
(title: "upstream monochrome asset build")

[commit-bot: I haz the power, 2016-08-10 16:04: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-08-10 16:08:14]

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

[commit-bot: I haz the power, 2016-08-10 16:08:15]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[aberent, 2016-08-10 16:34:27]

The CQ bit was checked by aberent@chromium.org

[commit-bot: I haz the power, 2016-08-10 16:34:50]

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-08-10 17:31:01]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-10 17:32:49]

Description was changed from

==========
Merge webviewchromium.pak and resources.pak in Monochrome

This CL adds the webview specific resources to the Android
version of resources.pak (adding roughly 2.1KB to it), and
lets Webview read its resources from resources.pak if
webviewchromium.pak doesn't exist.

The following downstream CL will remove webviewchromium.pak
from the Monochrome APk, reducing the size of this by 588KB.

BUG=634358
==========

to

==========
Merge webviewchromium.pak and resources.pak in Monochrome

This CL adds the webview specific resources to the Android
version of resources.pak (adding roughly 2.1KB to it), and
lets Webview read its resources from resources.pak if
webviewchromium.pak doesn't exist.

The following downstream CL will remove webviewchromium.pak
from the Monochrome APk, reducing the size of this by 588KB.

BUG=634358

Committed: https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261
Cr-Commit-Position: refs/heads/master@{#411066}
==========

[commit-bot: I haz the power, 2016-08-10 17:32:52]

Patchset 3 (id:??) landed as
https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261
Cr-Commit-Position: refs/heads/master@{#411066}

[michaelbai, 2016-08-22 22:29:37]

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

[michaelbai, 2016-08-22 22:29:38]

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

https://codereview.chromium.org/2235753002/diff/40001/chrome/BUILD.gn#newcode...
chrome/BUILD.gn:1415: }
Will this add aw_resources to standaline Chrome?

[Torne, 2016-08-26 13:56:37]

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

https://codereview.chromium.org/2235753002/diff/40001/chrome/BUILD.gn#newcode...
chrome/BUILD.gn:1415: }
On 2016/08/22 22:29:37, michaelbai wrote:
> Will this add aw_resources to standaline Chrome?

I think so, actually, yes.. oops. Maybe you could try to fix that?

I think there's only actually one resource in there, though?

[michaelbai, 2016-08-29 22:33:37]

On 2016/08/26 13:56:37, Torne wrote:
> https://codereview.chromium.org/2235753002/diff/40001/chrome/BUILD.gn
> File chrome/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2235753002/diff/40001/chrome/BUILD.gn#newcode...
> chrome/BUILD.gn:1415: }
> On 2016/08/22 22:29:37, michaelbai wrote:
> > Will this add aw_resources to standaline Chrome?
> 
> I think so, actually, yes.. oops. Maybe you could try to fix that?
> 
> I think there's only actually one resource in there, though?

Right, there is only one resource, sent you the fix

https://codereview.chromium.org/2295523002/