Skip to content

Commit

Permalink
Deduplicate Monochrome locale .paks
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
zpeng authored and Commit Bot committed Jul 11, 2017
1 parent e01ad95 commit 08ba6b7
Show file tree
Hide file tree
Showing 12 changed files with 259 additions and 57 deletions.
52 changes: 48 additions & 4 deletions chrome/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ app_hooks_impl = "java/src/org/chromium/chrome/browser/AppHooksImpl.java"
if (enable_resource_whitelist_generation) {
monochrome_resource_whitelist =
"$target_gen_dir/monochrome_resource_whitelist.txt"
monochrome_locale_whitelist =
"$target_gen_dir/monochrome_locale_whitelist.txt"
}

jinja_template("chrome_public_android_manifest") {
Expand Down Expand Up @@ -730,9 +732,38 @@ if (current_toolchain == default_toolchain) {
"/libmonochrome$shlib_extension.whitelist"
output = monochrome_resource_whitelist
}

action("monochrome_locale_whitelist") {
script = "//tools/resources/filter_resource_whitelist.py"

_system_webview_pak_whitelist =
"$root_gen_dir/android_webview/system_webview_pak_whitelist.txt"

inputs = [
monochrome_resource_whitelist,
_system_webview_pak_whitelist,
]

outputs = [
monochrome_locale_whitelist,
]

deps = [
":monochrome_resource_whitelist",
"//android_webview:system_webview_pak_whitelist",
]

args = [
"--input",
rebase_path(monochrome_resource_whitelist, root_build_dir),
"--filter",
rebase_path(_system_webview_pak_whitelist, root_build_dir),
"--output",
rebase_path(monochrome_locale_whitelist, root_build_dir),
]
}
}

# This target does not output locale paks.
chrome_paks("monochrome_paks") {
output_dir = "$target_gen_dir/$target_name"

Expand All @@ -741,13 +772,26 @@ if (current_toolchain == default_toolchain) {
"//android_webview:generate_aw_resources",
]

exclude_locale_paks = true

if (enable_resource_whitelist_generation) {
repack_whitelist = monochrome_resource_whitelist
deps += [ ":monochrome_resource_whitelist" ]
locale_whitelist = monochrome_locale_whitelist
deps += [ ":monochrome_locale_whitelist" ]
}
}

# This target is separate from monochrome_pak_assets because it does not
# disable compression.
android_assets("monochrome_locale_pak_assets") {
sources = []
foreach(_locale, locales - android_chrome_omitted_locales) {
sources += [ "$target_gen_dir/monochrome_paks/locales/$_locale.pak" ]
}

deps = [
":monochrome_paks",
]
}

# This target explicitly includes locale paks via deps.
android_assets("monochrome_pak_assets") {
Expand All @@ -758,7 +802,7 @@ if (current_toolchain == default_toolchain) {
disable_compression = true

deps = [
":chrome_public_locale_pak_assets",
":monochrome_locale_pak_assets",
":monochrome_paks",
"//android_webview:locale_pak_assets",
]
Expand Down
10 changes: 10 additions & 0 deletions chrome/app/chrome_main_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,16 @@ void ChromeMainDelegate::PreSandboxStartup() {
ResourceBundle::InitSharedInstanceWithPakFileRegion(base::File(pak_fd),
pak_region);

// Load secondary locale .pak file if it exists.
pak_fd = global_descriptors->MaybeGet(kAndroidSecondaryLocalePakDescriptor);
if (pak_fd != -1) {
pak_region = global_descriptors->GetRegion(
kAndroidSecondaryLocalePakDescriptor);
ResourceBundle::GetSharedInstance().
LoadSecondaryLocaleDataWithPakFileRegion(
base::File(pak_fd), pak_region);
}

int extra_pak_keys[] = {
kAndroidChrome100PercentPakDescriptor,
kAndroidUIResourcesPakDescriptor,
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/chrome_browser_main_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ int ChromeBrowserMainPartsAndroid::PreCreateThreads() {
crash_dump_dir, kAndroidMinidumpDescriptor));
}

// Auto-detect based on en-US whether secondary locale .pak files exist.
ui::SetLoadSecondaryLocalePaks(
!ui::GetPathForAndroidLocalePakWithinApk("en-US").empty());

return ChromeBrowserMainParts::PreCreateThreads();
}

Expand Down
6 changes: 6 additions & 0 deletions chrome/browser/chrome_content_browser_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2800,6 +2800,12 @@ void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
fd = ui::GetLocalePackFd(&region);
mappings->ShareWithRegion(kAndroidLocalePakDescriptor, fd, region);

// Optional secondary locale .pak file.
fd = ui::GetSecondaryLocalePackFd(&region);
if (fd != -1) {
mappings->ShareWithRegion(kAndroidSecondaryLocalePakDescriptor, fd, region);
}

breakpad::CrashDumpObserver::GetInstance()->BrowserChildProcessStarted(
child_process_id, mappings);

Expand Down
40 changes: 20 additions & 20 deletions chrome/chrome_paks.gni
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ template("chrome_extra_paks") {
# output_dir [required]: Directory to output .pak files. Locale .pak files
# will always be place in $output_dir/locales
# additional_extra_paks: List of extra .pak sources for resources.pak.
# exclude_locale_paks: if set to true, skip chrome_repack_locales.
# locale_whitelist: if set, override repack_whitelist for locale .pak files.
# copy_data_to_bundle:
# deps:
# output_dir:
Expand Down Expand Up @@ -234,24 +234,26 @@ template("chrome_paks") {
}
}

if (!defined(invoker.exclude_locale_paks) || !invoker.exclude_locale_paks) {
chrome_repack_locales("${target_name}_locales") {
forward_variables_from(invoker,
[
"copy_data_to_bundle",
"deps",
"repack_whitelist",
"visibility",
])
chrome_repack_locales("${target_name}_locales") {
forward_variables_from(invoker,
[
"copy_data_to_bundle",
"deps",
"visibility",
])
if (defined(invoker.locale_whitelist)) {
repack_whitelist = invoker.locale_whitelist
} else if (defined(invoker.repack_whitelist)) {
repack_whitelist = invoker.repack_whitelist
}

input_locales = locales
output_dir = "${invoker.output_dir}/locales"
input_locales = locales
output_dir = "${invoker.output_dir}/locales"

if (is_mac) {
output_locales = locales_as_mac_outputs
} else {
output_locales = locales
}
if (is_mac) {
output_locales = locales_as_mac_outputs
} else {
output_locales = locales
}
}

Expand All @@ -260,10 +262,8 @@ template("chrome_paks") {
public_deps = [
":${target_name}_100_percent",
":${target_name}_extra",
":${target_name}_locales",
]
if (!defined(invoker.exclude_locale_paks) || !invoker.exclude_locale_paks) {
public_deps += [ ":${target_name}_locales" ]
}
if (enable_hidpi) {
public_deps += [ ":${target_name}_200_percent" ]
}
Expand Down
1 change: 1 addition & 0 deletions chrome/common/descriptors_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
enum {
#if defined(OS_ANDROID)
kAndroidLocalePakDescriptor = kContentIPCDescriptorMax + 1,
kAndroidSecondaryLocalePakDescriptor,
kAndroidChrome100PercentPakDescriptor,
kAndroidUIResourcesPakDescriptor,
kAndroidMinidumpDescriptor,
Expand Down
2 changes: 2 additions & 0 deletions tools/resources/OWNERS
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
per-file generate_resource_whitelist.*=agrieve@chromium.org
per-file generate_resource_whitelist.*=estevenson@chromium.org
per-file filter_resource_whitelist.*=agrieve@chromium.org
per-file filter_resource_whitelist.*=zpeng@chromium.org
50 changes: 50 additions & 0 deletions tools/resources/filter_resource_whitelist.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#!/usr/bin/env python
# Copyright 2017 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

__doc__ = """filter_resource_whitelist.py [-h] [--input INPUT]
[--filter FILTER] [--output OUTPUT]
INPUT specifies a resource whitelist file containing resource IDs that should
be whitelisted, where each line of INPUT contains a single resource ID.
FILTER specifies a resource whitelist file containing resource IDs that should
not be whitelisted, where each line of FILTER contains a single resource ID.
Filters a resource whitelist by removing resource IDs that are contained in a
another resource whitelist.
This script is used to generate Monochrome's locale paks.
"""

import argparse
import sys


def main():
parser = argparse.ArgumentParser(usage=__doc__)
parser.add_argument(
'--input', type=argparse.FileType('r'), required=True,
help='A resource whitelist where each line contains one resource ID. '
'These IDs, excluding the ones in FILTER, are to be included.')
parser.add_argument(
'--filter', type=argparse.FileType('r'), required=True,
help='A resource whitelist where each line contains one resource ID. '
'These IDs are to be excluded.')
parser.add_argument(
'--output', type=argparse.FileType('w'), default=sys.stdout,
help='The resource list path to write (default stdout)')

args = parser.parse_args()

input_resources = list(int(resource_id) for resource_id in args.input)
filter_resources = set(int(resource_id) for resource_id in args.filter)
output_resources = [resource_id for resource_id in input_resources
if resource_id not in filter_resources]

for resource_id in sorted(output_resources):
args.output.write('%d\n' % resource_id)

if __name__ == '__main__':
main()
45 changes: 36 additions & 9 deletions ui/base/resource/resource_bundle.cc
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ void ResourceBundle::InitSharedInstanceWithPakFileRegion(
base::File pak_file,
const base::MemoryMappedFile::Region& region) {
InitSharedInstance(NULL);
std::unique_ptr<DataPack> data_pack(new DataPack(SCALE_FACTOR_100P));
auto data_pack = base::MakeUnique<DataPack>(SCALE_FACTOR_100P);
if (!data_pack->LoadFromFileRegion(std::move(pak_file), region)) {
NOTREACHED() << "failed to load pak file";
return;
Expand Down Expand Up @@ -248,6 +248,17 @@ ResourceBundle& ResourceBundle::GetSharedInstance() {
return *g_shared_instance_;
}

void ResourceBundle::LoadSecondaryLocaleDataWithPakFileRegion(
base::File pak_file,
const base::MemoryMappedFile::Region& region) {
auto data_pack = base::MakeUnique<DataPack>(SCALE_FACTOR_100P);
if (!data_pack->LoadFromFileRegion(std::move(pak_file), region)) {
NOTREACHED() << "failed to load secondary pak file";
return;
}
secondary_locale_resources_data_ = std::move(data_pack);
}

#if !defined(OS_ANDROID)
bool ResourceBundle::LocaleDataPakExists(const std::string& locale) {
return !GetLocaleFilePath(locale, true).empty();
Expand Down Expand Up @@ -389,6 +400,7 @@ void ResourceBundle::LoadTestResources(const base::FilePath& path,

void ResourceBundle::UnloadLocaleResources() {
locale_resources_data_.reset();
secondary_locale_resources_data_.reset();
}

void ResourceBundle::OverrideLocalePakForTest(const base::FilePath& pak_path) {
Expand Down Expand Up @@ -551,20 +563,27 @@ base::string16 ResourceBundle::GetLocalizedString(int message_id) {
}

base::StringPiece data;
ResourceHandle::TextEncodingType encoding =
locale_resources_data_->GetTextEncodingType();
if (!locale_resources_data_->GetStringPiece(static_cast<uint16_t>(message_id),
&data)) {
// Fall back on the main data pack (shouldn't be any strings here except in
// unittests).
data = GetRawDataResource(message_id);
if (data.empty()) {
NOTREACHED() << "unable to find resource: " << message_id;
return base::string16();
if (secondary_locale_resources_data_.get() &&
secondary_locale_resources_data_->GetStringPiece(
static_cast<uint16_t>(message_id), &data)) {
// Fall back on the secondary locale pak if it exists.
encoding = secondary_locale_resources_data_->GetTextEncodingType();
} else {
// Fall back on the main data pack (shouldn't be any strings here except
// in unittests).
data = GetRawDataResource(message_id);
if (data.empty()) {
NOTREACHED() << "unable to find resource: " << message_id;
return base::string16();
}
}
}

// Strings should not be loaded from a data pack that contains binary data.
ResourceHandle::TextEncodingType encoding =
locale_resources_data_->GetTextEncodingType();
DCHECK(encoding == ResourceHandle::UTF16 || encoding == ResourceHandle::UTF8)
<< "requested localized string from binary pack file";

Expand All @@ -584,12 +603,20 @@ base::RefCountedMemory* ResourceBundle::LoadLocalizedResourceBytes(
{
base::AutoLock lock_scope(*locale_resources_data_lock_);
base::StringPiece data;

if (locale_resources_data_.get() &&
locale_resources_data_->GetStringPiece(
static_cast<uint16_t>(resource_id), &data) &&
!data.empty()) {
return new base::RefCountedStaticMemory(data.data(), data.length());
}

if (secondary_locale_resources_data_.get() &&
secondary_locale_resources_data_->GetStringPiece(
static_cast<uint16_t>(resource_id), &data) &&
!data.empty()) {
return new base::RefCountedStaticMemory(data.data(), data.length());
}
}
// Release lock_scope and fall back to main data pack.
return LoadDataResourceBytes(resource_id);
Expand Down
6 changes: 6 additions & 0 deletions ui/base/resource/resource_bundle.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ class UI_BASE_EXPORT ResourceBundle {
// Return the global resource loader instance.
static ResourceBundle& GetSharedInstance();

// Loads a secondary locale data pack using the given file region.
void LoadSecondaryLocaleDataWithPakFileRegion(
base::File pak_file,
const base::MemoryMappedFile::Region& region);

// Check if the .pak for the given locale exists.
bool LocaleDataPakExists(const std::string& locale);

Expand Down Expand Up @@ -399,6 +404,7 @@ class UI_BASE_EXPORT ResourceBundle {

// Handles for data sources.
std::unique_ptr<ResourceHandle> locale_resources_data_;
std::unique_ptr<ResourceHandle> secondary_locale_resources_data_;
std::vector<std::unique_ptr<ResourceHandle>> data_packs_;

// The maximum scale factor currently loaded.
Expand Down
Loading

0 comments on commit 08ba6b7

Please sign in to comment.