New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 724110 link

Starred by 1 user
Status: Fixed
Owner:
█ zpeng@chromium.org
Last visit > 30 days ago
Closed: Jul 2017
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: ----

Blocked on:
issue 742388


Sign in to add a comment

Remove duplicates locales from monochrome

Project Member Reported by zpeng@chromium.org, May 18 2017 
Now that monochrome contains both webview locales and chrome locales, duplicate locales exist, though webview's locales are uncompressed and chrome's locales are compressed.

It's found that 225 of the 231 IDs in webview's en-US.pak exist in Chrome's whitelist. And because Webview's locale paks are 535KB uncompressed and 215KB compressed, it's estimated that removing duplicates would save 200KB.


forked from  crbug.com/687797

Comment 1 by zpeng@chromium.org, May 24 2017

Owner: zpeng@chromium.org

Comment 2 by zpeng@chromium.org, May 31 2017 

The plan:
1. Add new build target monochrome_locale_whitelist, which is chrome_resource_whitelist excluding resource IDs from system_webview_pak_whitelist.
2. (This step should be guarded by flag: enable_resource_whitelist_generation) Call chrome_repack_locales on the new whitelist, and replace chrome_public_locale_pak_assets with the new locale target for monochrome.
3. Implement fallback resource loading.

Comment 3 by wnwen@chromium.org, Jun 5 2017

Labels: -Pri-3 M-61 Pri-2
Status: Assigned (was: Untriaged)
Targeting M-61 for Chrome Go.

Comment 4 Deleted

Comment 5 by zpeng@chromium.org, Jun 12 2017

Blockedon: 732414

Comment 6 by zpeng@chromium.org, Jun 28 2017

Blockedon: -732414
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 11 2017 

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

commit 08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6
Author: zpeng <zpeng@chromium.org>
Date: Tue Jul 11 15:35:08 2017

Deduplicate Monochrome locale .paks

This CL removes WebView strings from Chrome locale .paks in Monochrome
using a generated resource whitelist. While WebView's locale resource
logic remains the same, Chrome in Monochrome now loads a secondary
locale pack file as a fallback when a string cannot be found in the
primary locale pack file.

This CL reduces binary size by over 400KB.

BUG= 724110 

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

[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/chrome/android/BUILD.gn
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/chrome/app/chrome_main_delegate.cc
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/chrome/browser/chrome_browser_main_android.cc
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/chrome/chrome_paks.gni
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/chrome/common/descriptors_android.h
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/tools/resources/OWNERS
[add] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/tools/resources/filter_resource_whitelist.py
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/ui/base/resource/resource_bundle.cc
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/ui/base/resource/resource_bundle.h
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/ui/base/resource/resource_bundle_android.cc
[modify] https://crrev.com/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6/ui/base/resource/resource_bundle_android.h

Comment 8 by wnwen@chromium.org, Jul 11 2017 

Reduced binary size by 409KiB: https://chromeperf.appspot.com/group_report?rev=485635

Comment 9 by wnwen@chromium.org, Jul 11 2017

Summary: Remove duplicates locales from monochrome (was: Remove duplicates locales from monochrome (estimated 200KB))

Comment 10 by zpeng@chromium.org, Jul 11 2017

Status: Fixed (was: Assigned)
Project Member

Comment 11 by bugdroid1@chromium.org, Jul 13 2017 

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

commit df9f33ffee10b4ffbd6469697bc916a137097860
Author: zpeng <zpeng@chromium.org>
Date: Thu Jul 13 15:58:53 2017

Revert of Deduplicate Monochrome locale .paks (patchset #4 id:180001 of https://codereview.chromium.org/2933343002/ )

Reason for revert:
Chrome is now missing 5 locale strings. The reason is that these strings are believed to be in WebView locales because they're whitelisted, but since WebView uses multiple whitelists to pack its locale strings, these strings are actually not contained by WebView

Original issue's description:
> Deduplicate Monochrome locale .paks
>
> This CL removes WebView strings from Chrome locale .paks in Monochrome
> using a generated resource whitelist. While WebView's locale resource
> logic remains the same, Chrome in Monochrome now loads a secondary
> locale pack file as a fallback when a string cannot be found in the
> primary locale pack file.
>
> This CL reduces binary size by over 400KB.
>
> BUG= 724110 
>
> Review-Url: https://codereview.chromium.org/2933343002
> Cr-Commit-Position: refs/heads/master@{#485635}
> Committed: https://chromium.googlesource.com/chromium/src/+/08ba6b7c7e7edb3f7c8851eb7ddc1d2ea64497e6

TBR=agrieve@chromium.org,dpranke@chromium.org,thestig@chromium.org,sadrul@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 724110 , 741798

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

[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/chrome/android/BUILD.gn
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/chrome/app/chrome_main_delegate.cc
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/chrome/browser/chrome_browser_main_android.cc
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/chrome/chrome_paks.gni
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/chrome/common/descriptors_android.h
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/tools/resources/OWNERS
[delete] https://crrev.com/c0ce31467ec5d9f79c3e1e069f785d211a03d655/tools/resources/filter_resource_whitelist.py
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/ui/base/resource/resource_bundle.cc
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/ui/base/resource/resource_bundle.h
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/ui/base/resource/resource_bundle_android.cc
[modify] https://crrev.com/df9f33ffee10b4ffbd6469697bc916a137097860/ui/base/resource/resource_bundle_android.h

Comment 12 by zpeng@chromium.org, Jul 13 2017

Blockedon: 742388
Status: Started (was: Fixed)
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 17 2017 

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

commit 368afac85d2ca5e22efcbb2271c0d379b2fc22ab
Author: zpeng <zpeng@chromium.org>
Date: Mon Jul 17 18:47:45 2017

Reland of Deduplicate Monochrome locale .paks

Instead of using system webview's resource whitelist, now uses a
generated list of resource IDs that are actually packed into
Webview's locale paks. This fixes the missing strings issue.

Original issue:
https://codereview.chromium.org/2980773002/

TBR=agrieve@chromium.org,dpranke@chromium.org,thestig@chromium.org,sadrul@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days
ago.
BUG= 724110 ,  742388 

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

[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/chrome/android/BUILD.gn
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/chrome/app/chrome_main_delegate.cc
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/chrome/browser/chrome_browser_main_android.cc
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/chrome/chrome_paks.gni
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/chrome/common/descriptors_android.h
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/tools/grit/pak_util.py
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/tools/resources/OWNERS
[add] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/tools/resources/filter_resource_whitelist.py
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/ui/base/resource/resource_bundle.cc
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/ui/base/resource/resource_bundle.h
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/ui/base/resource/resource_bundle_android.cc
[modify] https://crrev.com/368afac85d2ca5e22efcbb2271c0d379b2fc22ab/ui/base/resource/resource_bundle_android.h

Comment 14 by zpeng@chromium.org, Jul 18 2017

Status: Fixed (was: Started)
►

Sign in to add a comment