Commit a5749b7e authored by Andrew Grieve's avatar Andrew Grieve Committed by Commit Bot

Android: Move add_unwind_tables_in_apk logic into chrome_public_apk_tmpl.gni

Noticed the upstream and downstream logic had diverged. This should
allow deleting the downstream logic (since it's the same).

This was also causing the binary size trybot to include assets/unwind_cfi in
size reports, which it shouldn't do.

Bug: 977574
Change-Id: I24ac52bf806455866081c1bf7bf0d6643ecda12b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1670025
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Auto-Submit: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: default avatarTibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671295}
parent 3b74db86
......@@ -1552,12 +1552,6 @@ static_library("browser_test_support") {
]
}
# Unwind tables are added to only official builds (public_apk(s)) so that
# developer builds are not affected.
_add_unwind_tables_in_chrome_32bit_apk =
is_android && !is_component_build && is_official_build &&
(target_cpu == "arm" || target_cpu == "arm64")
# Defines a target that derives from the chrome public application. This
# can be either an APK or an app bundle module. This supports both the
# chrome_public_xxx targets (for Android J-K) and chrome_modern_public_xxx
......@@ -1601,11 +1595,6 @@ template("chrome_public_apk_or_module_tmpl") {
}
shared_libraries = [ ":libchrome" ]
add_unwind_tables_in_apk =
_add_unwind_tables_in_chrome_32bit_apk && target_cpu == "arm"
if (add_unwind_tables_in_apk) {
shared_library_for_unwind_asset = "chrome"
}
# Android supports webp transparent resources properly since API level 18,
# so this can only be activated for modern ones (which target API >= 21).
......@@ -1757,12 +1746,6 @@ template("monochrome_public_apk_or_module_tmpl") {
"//android_webview:platform_service_bridge_upstream_implementation_java",
]
}
_needs_arm32_lib = target_cpu == "arm" ||
(target_cpu == "arm64" && build_apk_secondary_abi)
add_unwind_tables_in_apk =
_add_unwind_tables_in_chrome_32bit_apk && _needs_arm32_lib &&
(!defined(use_trichrome_library) || !use_trichrome_library)
}
}
......@@ -1919,11 +1902,7 @@ template("chrome_test_apk_tmpl") {
testonly = true
target_type = "instrumentation_test_apk"
add_unwind_tables_in_apk =
_add_unwind_tables_in_chrome_32bit_apk && target_cpu == "arm"
if (add_unwind_tables_in_apk) {
shared_library_for_unwind_asset = "chromefortest"
}
shared_library_for_unwind_asset = "chromefortest"
deps = _chrome_public_shared_deps + invoker.deps + [
":chrome_apk_pak_assets",
......
......@@ -30,6 +30,14 @@ default_chrome_public_jinja_variables = [
"notouch_build=$notouch_build",
]
# Enable stack unwinding only on official build with specific channels. It is
# not enabled on non-official builds to not affect build time for developers.
# The unwind file is ~2MB in apk, which is fine for Canary.
_add_unwind_tables_in_chrome_32bit_apk =
is_official_build && (target_cpu == "arm" || target_cpu == "arm64") &&
(android_channel == "default" || android_channel == "canary" ||
android_channel == "dev")
# A template used to declare any target that will implement a full Chromium
# or Chrome application, either as an APK, or an app bundle module.
#
......@@ -64,33 +72,51 @@ default_chrome_public_jinja_variables = [
# android_app_bundle_module(), depending on the target type.
#
template("chrome_public_common_apk_or_module_tmpl") {
assert(defined(invoker.target_type), "target_type is required!")
assert(
invoker.target_type == "android_apk" ||
invoker.target_type == "android_app_bundle_module" ||
invoker.target_type == "instrumentation_test_apk",
"Invalid target_type definition, should be 'android_apk' or 'android_app_bundle_module'")
assert(!(defined(invoker.is_trichrome) && invoker.is_trichrome) ||
!(defined(invoker.is_monochrome) && invoker.is_monochrome),
_is_modern = defined(invoker.is_modern) && invoker.is_modern
_is_monochrome = defined(invoker.is_monochrome) && invoker.is_monochrome
_is_trichrome = defined(invoker.is_trichrome) && invoker.is_trichrome
assert(_is_modern || !_is_modern) # Mark as used.
assert(!(_is_monochrome && _is_trichrome),
"Cannot be both trichrome and monochrome!")
# Adds unwind table asset to the chrome apk for the given library target. This
# is not part of generic apk assets target since it depends on the main shared
# library of the apk, to extract unwind tables.
if (defined(invoker.add_unwind_tables_in_apk) &&
invoker.add_unwind_tables_in_apk) {
_unwind_asset = "${target_name}_unwind_assets"
unwind_table_asset(_unwind_asset) {
if (defined(invoker.add_unwind_tables_in_apk)) {
_add_unwind_tables = invoker.add_unwind_tables_in_apk
} else {
_needs_32bit_lib =
target_cpu == "arm" ||
(_is_monochrome && target_cpu == "arm64" && build_apk_secondary_abi)
_add_unwind_tables =
_needs_32bit_lib && _add_unwind_tables_in_chrome_32bit_apk
}
if (_add_unwind_tables) {
_unwind_asset_target = "${target_name}__unwind_assets"
unwind_table_asset(_unwind_asset_target) {
if (defined(invoker.testonly)) {
testonly = invoker.testonly
}
library_target = invoker.shared_library_for_unwind_asset
if (defined(invoker.shared_library_for_unwind_asset)) {
library_target = invoker.shared_library_for_unwind_asset
} else {
library_target = "chrome"
}
deps = invoker.shared_libraries
if (build_apk_secondary_abi && defined(android_secondary_abi_cpu)) {
deps += [ "//chrome/android:lib${library_target}($android_secondary_abi_toolchain)" ]
}
}
} else if (defined(invoker.shared_library_for_unwind_asset)) {
not_needed(invoker, [ "shared_library_for_unwind_asset" ])
}
if (!defined(invoker.target_type)) {
......@@ -99,13 +125,6 @@ template("chrome_public_common_apk_or_module_tmpl") {
_target_type = invoker.target_type
}
_is_modern = defined(invoker.is_modern) && invoker.is_modern
assert(_is_modern || !_is_modern) # Mark as used.
_is_monochrome = defined(invoker.is_monochrome) && invoker.is_monochrome
_is_trichrome = defined(invoker.is_trichrome) && invoker.is_trichrome
assert(_is_trichrome || !_is_trichrome) # Mark as used.
if (defined(invoker.enable_multidex)) {
_enable_multidex = invoker.enable_multidex
} else {
......@@ -251,9 +270,8 @@ template("chrome_public_common_apk_or_module_tmpl") {
"//chrome/android:product_version_resources"
}
if (defined(invoker.add_unwind_tables_in_apk) &&
invoker.add_unwind_tables_in_apk) {
deps += [ ":$_unwind_asset" ]
if (defined(_unwind_asset_target)) {
deps += [ ":$_unwind_asset_target" ]
}
deps += [ "//chrome/android:chrome_all_java" ]
......@@ -323,9 +341,7 @@ template("monochrome_public_common_apk_or_module_tmpl") {
} else {
shared_libraries = [ "//chrome/android:libmonochrome" ]
}
if (invoker.add_unwind_tables_in_apk) {
shared_library_for_unwind_asset = "monochrome"
}
shared_library_for_unwind_asset = "monochrome"
_deps += [
"//android_webview:monochrome_webview_assets",
......
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