New issue
Advanced search Search tips

Issue 634358 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Feature

Blocking:
issue 604330



Sign in to add a comment

In monochrome merge webviewchromium.pak and resources.pak

Project Member Reported by aber...@chromium.org, Aug 4 2016

Issue description

The monochrome APK contains both webviewchromium.pak and resources.pak. All but one of the resources in webviewchromium.pak are also in resources.pak, so this is wasting roughly 640KB in the APK.

When Webview is provided by Monochrome it should use the resources from resources.pak.
 

Comment 1 by torne@chromium.org, Aug 4 2016

Cc: michaelbai@chromium.org
Status: Available (was: Untriaged)
We definitely want this; I assumed it was already this way :)
In discussion with Torne we thought that there might be clashing resource ids; however looking at //tools/gritsettings/resource_ids this should not be a problem. aw_resources.grd uses the same range as ash_strings.grd and ios_strings.grd, but neither of these should exist on Android.

Comment 3 by ti...@chromium.org, Aug 8 2016

Cc: -michaelbai@chromium.org
Owner: michaelbai@chromium.org
Status: Assigned (was: Available)
Blocking: 604330
This is blocking 604330 (Web Restrictions in WebView), since to try to avoid significantly increasing the size of webview's resources when we know we are using Monochrome, we are attempting to share Chrome's resources.


Project Member

Comment 5 by bugdroid1@chromium.org, Aug 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9ad131f6da209e1dd6ee719ceee60b10c61a0261

commit 9ad131f6da209e1dd6ee719ceee60b10c61a0261
Author: aberent <aberent@chromium.org>
Date: Wed Aug 10 17:30:43 2016

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 

Review-Url: https://codereview.chromium.org/2235753002
Cr-Commit-Position: refs/heads/master@{#411066}

[modify] https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261/android_webview/BUILD.gn
[modify] https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261/android_webview/browser/aw_browser_main_parts.cc
[modify] https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261/chrome/BUILD.gn
[modify] https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261/ui/base/resource/resource_bundle_android.cc
[modify] https://crrev.com/9ad131f6da209e1dd6ee719ceee60b10c61a0261/ui/base/resource/resource_bundle_android.h

Project Member

Comment 6 by bugdroid1@chromium.org, Aug 11 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/a08ece2cdc2addb5d9ff14d28eace65f8e5c49a2

commit a08ece2cdc2addb5d9ff14d28eace65f8e5c49a2
Author: Anthony Berent <aberent@chromium.org>
Date: Wed Aug 10 15:56:44 2016

Project Member

Comment 7 by bugdroid1@chromium.org, Aug 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3bcea370cbe5f65605d2211f1145280e455af1eb

commit 3bcea370cbe5f65605d2211f1145280e455af1eb
Author: aberent <aberent@chromium.org>
Date: Tue Aug 16 12:12:53 2016

Fix missing monochrome resources.

As a result of https://codereview.chromium.org/2235753002
monochrome was failing to find some resources. Instead of
looking in two places for the PAK files this CL changes
the organization of the webview PAK files to match those
of Chrome, hence allowing identical behaviour for
Monochrome and isolated Webview.

In doing so it reverts the changes to resource_bundle_android.*
that were made in the previous CL.

BUG= 634358 

Review-Url: https://codereview.chromium.org/2248743002
Cr-Commit-Position: refs/heads/master@{#412218}

[modify] https://crrev.com/3bcea370cbe5f65605d2211f1145280e455af1eb/android_webview/BUILD.gn
[modify] https://crrev.com/3bcea370cbe5f65605d2211f1145280e455af1eb/android_webview/browser/aw_browser_main_parts.cc
[modify] https://crrev.com/3bcea370cbe5f65605d2211f1145280e455af1eb/ui/base/resource/resource_bundle_android.cc
[modify] https://crrev.com/3bcea370cbe5f65605d2211f1145280e455af1eb/ui/base/resource/resource_bundle_android.h

Status: Fixed (was: Assigned)

Comment 9 by torne@chromium.org, Aug 30 2016

Status: Assigned (was: Fixed)
Michael is going to clean up the way the pak files get included.
Cc: agrieve@chromium.org
Project Member

Comment 12 by bugdroid1@chromium.org, Sep 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a5d84d504167c0fb7452dec22c7f024f8fcde072

commit a5d84d504167c0fb7452dec22c7f024f8fcde072
Author: agrieve <agrieve@chromium.org>
Date: Wed Sep 21 03:07:03 2016

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

Review-Url: https://codereview.chromium.org/2354803002
Cr-Commit-Position: refs/heads/master@{#419960}

[modify] https://crrev.com/a5d84d504167c0fb7452dec22c7f024f8fcde072/build/config/android/internal_rules.gni
[modify] https://crrev.com/a5d84d504167c0fb7452dec22c7f024f8fcde072/chrome/BUILD.gn
[modify] https://crrev.com/a5d84d504167c0fb7452dec22c7f024f8fcde072/chrome/android/BUILD.gn
[modify] https://crrev.com/a5d84d504167c0fb7452dec22c7f024f8fcde072/chrome/browser/resources/chromeos/chromevox/BUILD.gn
[modify] https://crrev.com/a5d84d504167c0fb7452dec22c7f024f8fcde072/chrome/chrome_paks.gni
[modify] https://crrev.com/a5d84d504167c0fb7452dec22c7f024f8fcde072/chrome/test/BUILD.gn

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb8045c8f3889b84f3d36e3280763018328a5f17

commit cb8045c8f3889b84f3d36e3280763018328a5f17
Author: lizeb <lizeb@chromium.org>
Date: Wed Sep 21 09:27:45 2016

Revert of Create Monochrome-specific repack() targets (patchset #5 id:80001 of https://codereview.chromium.org/2354803002/ )

Reason for revert:
Breaks ARM64 downstream bot (crbug.com/648878).

Original issue's description:
> 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}

TBR=michaelbai@chromium.org,brettw@chromium.org,agrieve@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=641032, 634358 ,648878

Review-Url: https://codereview.chromium.org/2353553004
Cr-Commit-Position: refs/heads/master@{#420021}

[modify] https://crrev.com/cb8045c8f3889b84f3d36e3280763018328a5f17/build/config/android/internal_rules.gni
[modify] https://crrev.com/cb8045c8f3889b84f3d36e3280763018328a5f17/chrome/BUILD.gn
[modify] https://crrev.com/cb8045c8f3889b84f3d36e3280763018328a5f17/chrome/android/BUILD.gn
[modify] https://crrev.com/cb8045c8f3889b84f3d36e3280763018328a5f17/chrome/browser/resources/chromeos/chromevox/BUILD.gn
[modify] https://crrev.com/cb8045c8f3889b84f3d36e3280763018328a5f17/chrome/chrome_paks.gni
[modify] https://crrev.com/cb8045c8f3889b84f3d36e3280763018328a5f17/chrome/test/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5fce4ef2072549d268a77e23210c4ec673a1c4c6

commit 5fce4ef2072549d268a77e23210c4ec673a1c4c6
Author: agrieve <agrieve@chromium.org>
Date: Wed Sep 21 17:21:59 2016

Reland of Create Monochrome-specific repack() targets

Reverted in:
https://codereview.chromium.org/2353553004/

Reason for reland:
Now tested with arm64

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.

TBR=michaelbai@chromium.org,brettw@chromium.org,lizeb@chromium.org
BUG=641032, 634358 ,648878
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2357973002
Cr-Commit-Position: refs/heads/master@{#420090}

[modify] https://crrev.com/5fce4ef2072549d268a77e23210c4ec673a1c4c6/build/config/android/internal_rules.gni
[modify] https://crrev.com/5fce4ef2072549d268a77e23210c4ec673a1c4c6/chrome/BUILD.gn
[modify] https://crrev.com/5fce4ef2072549d268a77e23210c4ec673a1c4c6/chrome/android/BUILD.gn
[modify] https://crrev.com/5fce4ef2072549d268a77e23210c4ec673a1c4c6/chrome/browser/resources/chromeos/chromevox/BUILD.gn
[modify] https://crrev.com/5fce4ef2072549d268a77e23210c4ec673a1c4c6/chrome/chrome_paks.gni
[modify] https://crrev.com/5fce4ef2072549d268a77e23210c4ec673a1c4c6/chrome/test/BUILD.gn

Status: Fixed (was: Assigned)
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a0a347d16cb6ad810e1b2e488a3f58923769565e

commit a0a347d16cb6ad810e1b2e488a3f58923769565e
Author: timav <timav@chromium.org>
Date: Tue Oct 04 16:14:18 2016

Propagate common resources to WebView child process

WebView uses the same resources file scheme as Chrome
since https://codereview.chromium.org/2248743002.
In case of multiprocess WebView we need to pass the common
resources pak (chrome_100_percent.pak) to the renderer process.

BUG= 634358 , 638344

Review-Url: https://codereview.chromium.org/2384063005
Cr-Commit-Position: refs/heads/master@{#422822}

[modify] https://crrev.com/a0a347d16cb6ad810e1b2e488a3f58923769565e/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/a0a347d16cb6ad810e1b2e488a3f58923769565e/android_webview/common/aw_descriptors.h
[modify] https://crrev.com/a0a347d16cb6ad810e1b2e488a3f58923769565e/android_webview/lib/main/aw_main_delegate.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Oct 4 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1568079e94cbd3a8cb1f6cb0031181399c09b9fa

commit 1568079e94cbd3a8cb1f6cb0031181399c09b9fa
Author: Tima Vaisburd <timav@chromium.org>
Date: Tue Oct 04 17:08:23 2016

[Merge M54] Propagate common resources to WebView child process

WebView uses the same resources file scheme as Chrome
since https://codereview.chromium.org/2248743002.
In case of multiprocess WebView we need to pass the common
resources pak (chrome_100_percent.pak) to the renderer process.

BUG= 634358 , 638344

> Review-Url: https://codereview.chromium.org/2384063005
> Cr-Commit-Position: refs/heads/master@{#422822}
(cherry picked from commit a0a347d16cb6ad810e1b2e488a3f58923769565e)

Review URL: https://codereview.chromium.org/2391763004 .

Cr-Commit-Position: refs/branch-heads/2840@{#637}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/1568079e94cbd3a8cb1f6cb0031181399c09b9fa/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/1568079e94cbd3a8cb1f6cb0031181399c09b9fa/android_webview/common/aw_descriptors.h
[modify] https://crrev.com/1568079e94cbd3a8cb1f6cb0031181399c09b9fa/android_webview/lib/main/aw_main_delegate.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Oct 27 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1568079e94cbd3a8cb1f6cb0031181399c09b9fa

commit 1568079e94cbd3a8cb1f6cb0031181399c09b9fa
Author: Tima Vaisburd <timav@chromium.org>
Date: Tue Oct 04 17:08:23 2016

[Merge M54] Propagate common resources to WebView child process

WebView uses the same resources file scheme as Chrome
since https://codereview.chromium.org/2248743002.
In case of multiprocess WebView we need to pass the common
resources pak (chrome_100_percent.pak) to the renderer process.

BUG= 634358 , 638344

> Review-Url: https://codereview.chromium.org/2384063005
> Cr-Commit-Position: refs/heads/master@{#422822}
(cherry picked from commit a0a347d16cb6ad810e1b2e488a3f58923769565e)

Review URL: https://codereview.chromium.org/2391763004 .

Cr-Commit-Position: refs/branch-heads/2840@{#637}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/1568079e94cbd3a8cb1f6cb0031181399c09b9fa/android_webview/browser/aw_content_browser_client.cc
[modify] https://crrev.com/1568079e94cbd3a8cb1f6cb0031181399c09b9fa/android_webview/common/aw_descriptors.h
[modify] https://crrev.com/1568079e94cbd3a8cb1f6cb0031181399c09b9fa/android_webview/lib/main/aw_main_delegate.cc

Sign in to add a comment