Commit ffd91e52 authored by Mohamed Heikal's avatar Mohamed Heikal Committed by Commit Bot

Fix bug where chrome's app icon is missing in AGSA

AGSA queries an apps icon by name and thus fails on collapsed names. The
fix is to not collapse mipmaps (application icons).

This also fixes an old layering bug where chrome only resources were
listed under android_webview by allowing multiple config files for use
by Monochrome and Trichrome. Additionally the directive no_obfuscate has
been deprecated with no_collapse as its replacement.

TBR=boliu@chromium.org # noop change to webview config file.

Bug: b/161564466
Change-Id: Idb735cd07e9e95ab572297c8dc4848451d8d52bf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2343755Reviewed-by: default avatarMohamed Heikal <mheikal@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Commit-Queue: Mohamed Heikal <mheikal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796449}
parent 3bb4055f
string/license_activity_title#no_obfuscate string/license_activity_title#no_collapse
drawable/shortcut_incognito#no_obfuscate
...@@ -174,7 +174,7 @@ id/toolbar#no_obfuscate ...@@ -174,7 +174,7 @@ id/toolbar#no_obfuscate
``` ```
The aapt2 config file is passed to the ninja target through the The aapt2 config file is passed to the ninja target through the
`resources_config_path` variable. To add a resource to the whitelist, check `resources_config_paths` variable. To add a resource to the whitelist, check
where the config is for your target and add a new line for your resource. If where the config is for your target and add a new line for your resource. If
none exist, create a new config file and pass its path in your target. none exist, create a new config file and pass its path in your target.
......
...@@ -197,7 +197,9 @@ def _ParseArgs(args): ...@@ -197,7 +197,9 @@ def _ParseArgs(args):
'--optimized-proto-path', '--optimized-proto-path',
help='Output for `aapt2 optimize` for proto format (enables the step).') help='Output for `aapt2 optimize` for proto format (enables the step).')
input_opts.add_argument( input_opts.add_argument(
'--resources-config-path', help='Path to aapt2 resources config file.') '--resources-config-paths',
default='[]',
help='GN list of paths to aapt2 resources config files.')
output_opts.add_argument( output_opts.add_argument(
'--info-path', help='Path to output info file for the partial apk.') '--info-path', help='Path to output info file for the partial apk.')
...@@ -244,6 +246,8 @@ def _ParseArgs(args): ...@@ -244,6 +246,8 @@ def _ParseArgs(args):
options.values_filter_rules) options.values_filter_rules)
options.extra_main_r_text_files = build_utils.ParseGnList( options.extra_main_r_text_files = build_utils.ParseGnList(
options.extra_main_r_text_files) options.extra_main_r_text_files)
options.resources_config_paths = build_utils.ParseGnList(
options.resources_config_paths)
if options.optimized_proto_path and not options.proto_path: if options.optimized_proto_path and not options.proto_path:
# We could write to a temp file, but it's simpler to require it. # We could write to a temp file, but it's simpler to require it.
...@@ -879,6 +883,14 @@ def _PackageApk(options, build): ...@@ -879,6 +883,14 @@ def _PackageApk(options, build):
return desired_manifest_package_name return desired_manifest_package_name
def _CombineResourceConfigs(resources_config_paths, out_config_path):
with open(out_config_path, 'w') as out_config:
for config_path in resources_config_paths:
with open(config_path) as config:
out_config.write(config.read())
out_config.write('\n')
def _OptimizeApk(output, options, temp_dir, unoptimized_path, r_txt_path): def _OptimizeApk(output, options, temp_dir, unoptimized_path, r_txt_path):
"""Optimize intermediate .ap_ file with aapt2. """Optimize intermediate .ap_ file with aapt2.
...@@ -900,17 +912,13 @@ def _OptimizeApk(output, options, temp_dir, unoptimized_path, r_txt_path): ...@@ -900,17 +912,13 @@ def _OptimizeApk(output, options, temp_dir, unoptimized_path, r_txt_path):
# Optimize the resources.arsc file by obfuscating resource names and only # Optimize the resources.arsc file by obfuscating resource names and only
# allow usage via R.java constant. # allow usage via R.java constant.
if options.strip_resource_names: if options.strip_resource_names:
# Resources of type ID are references to UI elements/views. They are used by no_collapse_resources = _ExtractNonCollapsableResources(r_txt_path)
# UI automation testing frameworks. They are kept in so that they dont break
# tests, even though they may not actually be used during runtime. See
# https://crbug.com/900993
id_resources = _ExtractIdResources(r_txt_path)
gen_config_path = os.path.join(temp_dir, 'aapt2.config') gen_config_path = os.path.join(temp_dir, 'aapt2.config')
if options.resources_config_path: if options.resources_config_paths:
shutil.copyfile(options.resources_config_path, gen_config_path) _CombineResourceConfigs(options.resources_config_paths, gen_config_path)
with open(gen_config_path, 'a+') as config: with open(gen_config_path, 'a') as config:
for resource in id_resources: for resource in no_collapse_resources:
config.write('{}#no_obfuscate\n'.format(resource)) config.write('{}#no_collapse\n'.format(resource))
optimize_command += [ optimize_command += [
'--collapse-resource-names', '--collapse-resource-names',
...@@ -930,21 +938,30 @@ def _OptimizeApk(output, options, temp_dir, unoptimized_path, r_txt_path): ...@@ -930,21 +938,30 @@ def _OptimizeApk(output, options, temp_dir, unoptimized_path, r_txt_path):
optimize_command, print_stdout=False, print_stderr=False) optimize_command, print_stdout=False, print_stderr=False)
def _ExtractIdResources(rtxt_path): def _ExtractNonCollapsableResources(rtxt_path):
"""Extract resources of type ID from the R.txt file """Extract resources that should not be collapsed from the R.txt file
Resources of type ID are references to UI elements/views. They are used by
UI automation testing frameworks. They are kept in so that they don't break
tests, even though they may not actually be used during runtime. See
https://crbug.com/900993
App icons (aka mipmaps) are sometimes referenced by other apps by name so must
be keps as well. See https://b/161564466
Args: Args:
rtxt_path: Path to R.txt file with all the resources rtxt_path: Path to R.txt file with all the resources
Returns: Returns:
List of id resources in the form of id/<resource_name> List of resources in the form of <resource_type>/<resource_name>
""" """
id_resources = [] resources = []
_NO_COLLAPSE_TYPES = ['id', 'mipmap']
with open(rtxt_path) as rtxt: with open(rtxt_path) as rtxt:
for line in rtxt: for line in rtxt:
if ' id ' in line: for resource_type in _NO_COLLAPSE_TYPES:
resource_name = line.split()[2] if ' {} '.format(resource_type) in line:
id_resources.append('id/{}'.format(resource_name)) resource_name = line.split()[2]
return id_resources resources.append('{}/{}'.format(resource_type, resource_name))
return resources
@contextlib.contextmanager @contextlib.contextmanager
...@@ -1098,12 +1115,11 @@ def main(args): ...@@ -1098,12 +1115,11 @@ def main(args):
depfile_deps = (options.dependencies_res_zips + depfile_deps = (options.dependencies_res_zips +
options.extra_main_r_text_files + options.include_resources) options.extra_main_r_text_files + options.include_resources)
possible_input_paths = depfile_deps + [ possible_input_paths = depfile_deps + options.resources_config_paths + [
options.aapt2_path, options.aapt2_path,
options.android_manifest, options.android_manifest,
options.expected_file, options.expected_file,
options.expected_file_base, options.expected_file_base,
options.resources_config_path,
options.shared_resources_allowlist, options.shared_resources_allowlist,
options.use_resource_ids_path, options.use_resource_ids_path,
options.webp_binary, options.webp_binary,
......
...@@ -2456,12 +2456,11 @@ if (enable_java_templates) { ...@@ -2456,12 +2456,11 @@ if (enable_java_templates) {
rebase_path(invoker.optimized_proto_output, root_build_dir), rebase_path(invoker.optimized_proto_output, root_build_dir),
] ]
} }
if (defined(invoker.resources_config_path)) { if (defined(invoker.resources_config_paths)) {
_inputs += [ invoker.resources_config_path ] _inputs += invoker.resources_config_paths
_args += [ _rebased_resource_configs =
"--resources-config-path", rebase_path(invoker.resources_config_paths, root_build_dir)
rebase_path(invoker.resources_config_path, root_build_dir), _args += [ "--resources-config-paths=${_rebased_resource_configs}" ]
]
} }
if (defined(invoker.short_resource_paths) && invoker.short_resource_paths) { if (defined(invoker.short_resource_paths) && invoker.short_resource_paths) {
_args += [ "--short-resource-paths" ] _args += [ "--short-resource-paths" ]
......
...@@ -2088,8 +2088,8 @@ if (enable_java_templates) { ...@@ -2088,8 +2088,8 @@ if (enable_java_templates) {
# resources.arsc file in the apk or module. # resources.arsc file in the apk or module.
# short_resource_paths: True if resource paths should be shortened in the # short_resource_paths: True if resource paths should be shortened in the
# apk or module. # apk or module.
# resources_config_path: Path to the aapt2 optimize config file that tags # resources_config_paths: List of paths to the aapt2 optimize config files
# resources with acceptable/non-acceptable optimizations. # that tags resources with acceptable/non-acceptable optimizations.
# expected_android_manifest: Enables verification of expected merged # expected_android_manifest: Enables verification of expected merged
# manifest based on a golden file. # manifest based on a golden file.
# resource_ids_provider_dep: If passed, this target will use the resource # resource_ids_provider_dep: If passed, this target will use the resource
...@@ -2482,7 +2482,7 @@ if (enable_java_templates) { ...@@ -2482,7 +2482,7 @@ if (enable_java_templates) {
"resource_exclusion_exceptions", "resource_exclusion_exceptions",
"resource_exclusion_regex", "resource_exclusion_regex",
"resource_values_filter_rules", "resource_values_filter_rules",
"resources_config_path", "resources_config_paths",
"shared_resources", "shared_resources",
"shared_resources_allowlist_locales", "shared_resources_allowlist_locales",
"support_zh_hk", "support_zh_hk",
...@@ -3483,7 +3483,7 @@ if (enable_java_templates) { ...@@ -3483,7 +3483,7 @@ if (enable_java_templates) {
"resource_exclusion_regex", "resource_exclusion_regex",
"resource_ids_provider_dep", "resource_ids_provider_dep",
"resource_values_filter_rules", "resource_values_filter_rules",
"resources_config_path", "resources_config_paths",
"require_native_mocks", "require_native_mocks",
"secondary_abi_loadable_modules", "secondary_abi_loadable_modules",
"secondary_abi_shared_libraries", "secondary_abi_shared_libraries",
...@@ -3610,7 +3610,7 @@ if (enable_java_templates) { ...@@ -3610,7 +3610,7 @@ if (enable_java_templates) {
"resource_exclusion_regex", "resource_exclusion_regex",
"resource_ids_provider_dep", "resource_ids_provider_dep",
"resource_values_filter_rules", "resource_values_filter_rules",
"resources_config_path", "resources_config_paths",
"secondary_abi_loadable_modules", "secondary_abi_loadable_modules",
"secondary_abi_shared_libraries", "secondary_abi_shared_libraries",
"secondary_native_lib_placeholders", "secondary_native_lib_placeholders",
......
drawable/shortcut_incognito#no_collapse
...@@ -609,10 +609,12 @@ template("monochrome_public_common_apk_or_module_tmpl") { ...@@ -609,10 +609,12 @@ template("monochrome_public_common_apk_or_module_tmpl") {
} }
# Resources config for blocklisting resource names from obfuscation # Resources config for blocklisting resource names from obfuscation
if (defined(invoker.resources_config_path)) { resources_config_paths = [
resources_config_path = invoker.resources_config_path "//android_webview/aapt2.config",
} else { "//chrome/android/aapt2.config",
resources_config_path = "//android_webview/aapt2.config" ]
if (defined(invoker.resources_config_paths)) {
resources_config_paths += invoker.resources_config_paths
} }
if (defined(invoker.never_incremental)) { if (defined(invoker.never_incremental)) {
...@@ -702,7 +704,7 @@ template("monochrome_public_common_apk_or_module_tmpl") { ...@@ -702,7 +704,7 @@ template("monochrome_public_common_apk_or_module_tmpl") {
"no_xml_namespaces", "no_xml_namespaces",
"product_config_java_packages", "product_config_java_packages",
"proguard_configs", "proguard_configs",
"resources_config_path", "resources_config_paths",
"secondary_abi_loadable_modules", "secondary_abi_loadable_modules",
"secondary_abi_shared_libraries", "secondary_abi_shared_libraries",
"secondary_native_lib_placeholders", "secondary_native_lib_placeholders",
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment