Obfuscate android resource names using aapt2 |
|
Issue descriptionAndroid resources are stored in the resources.arsc with their names, ids and path in the apk. Accessing resources from java uses just the id (R.id.<resource_name>) rather than the string name. The actual names are only used in case of accessing via Resources.getIdentifier which takes the name of the resource and returns its id. Since resources are rarely accessed by anything other than their ids, we should remove those names from the apk/resources.arsc to reduce binary size. aapt2 optimize command has a flag (--enable-resource-obfuscation) that enables resource name obfuscation. It also can be passed a config file (--resources-config-path) that allows a subset of resources to be whitelisted from obfuscation so that they can still be accessed via name alone. The most prevalent reason a resource would need to be whitelisted from obfuscation is due to it being referenced from an AndroidManifest.xml. This is because of incremental install where the original compiled apk (with resources obfuscated) is passed to aapt and used to compile another AndroidManifest.xml to be used in the creation of the incremental install apk. If the AndroidManifest.xml references resources that have been obfuscated in the original apk, aapt would fail to compile the manifest and the build would fail. Thus all such resources should be added to the config file and set the no_obfuscate option. The format of the resources.config file is very simple. Each line looks like "<resource type>/<resource name>#<comma separated list of directives>". Currently the only two directives are 'delete' (for removing the resource from the apk) and 'no_obfuscate' which specifies that the resource name should not be obfuscated.
,
Oct 26
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8625939271762113637e56173b8d5a6c6ea6c765 commit 8625939271762113637e56173b8d5a6c6ea6c765 Author: Mohamed Heikal <mheikal@chromium.org> Date: Fri Oct 26 22:46:11 2018 [Android] Remove most resource names in resources.arsc resource names are stored in the resources.arsc file to allow for getIdentifier with the string name of the resource to work. This cl obfuscates all of those resources to one name (and thus stored only once) for all resources that are not accessed via getIdentifier. This improves binary size by about 200KB. Bug: 894208 Change-Id: I28c440c22b90cd045f53017073fdb88c7410d530 Reviewed-on: https://chromium-review.googlesource.com/c/1265897 Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Reviewed-by: Richard Coles <torne@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Reviewed-by: Ted Choc <tedchoc@chromium.org> Cr-Commit-Position: refs/heads/master@{#603229} [add] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/android_webview/aapt2.config [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/android_webview/apk/java/src/com/android/webview/chromium/LicenseActivity.java [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/build/android/gyp/apkbuilder.py [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/build/android/gyp/compile_resources.py [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/build/android/gyp/create_app_bundle.py [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/build/android/gyp/write_build_config.py [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/build/config/android/internal_rules.gni [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/build/config/android/rules.gni [modify] https://crrev.com/8625939271762113637e56173b8d5a6c6ea6c765/chrome/android/chrome_public_apk_tmpl.gni
,
Oct 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa commit 2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa Author: Mohamed Heikal <mheikal@chromium.org> Date: Mon Oct 29 23:57:26 2018 Revert "[Android] Remove most resource names in resources.arsc" This reverts commit 8625939271762113637e56173b8d5a6c6ea6c765. Reason for revert: Breaks webview https://crbug.com/899656 Original change's description: > [Android] Remove most resource names in resources.arsc > > resource names are stored in the resources.arsc file to allow for > getIdentifier with the string name of the resource to work. This cl > obfuscates all of those resources to one name (and thus stored only > once) for all resources that are not accessed via getIdentifier. > > This improves binary size by about 200KB. > > Bug: 894208 > > Change-Id: I28c440c22b90cd045f53017073fdb88c7410d530 > Reviewed-on: https://chromium-review.googlesource.com/c/1265897 > Commit-Queue: Mohamed Heikal <mheikal@chromium.org> > Reviewed-by: Richard Coles <torne@chromium.org> > Reviewed-by: agrieve <agrieve@chromium.org> > Reviewed-by: Ted Choc <tedchoc@chromium.org> > Cr-Commit-Position: refs/heads/master@{#603229} TBR=torne@chromium.org,tedchoc@chromium.org,agrieve@chromium.org,mheikal@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Binary-Size: This is a revert. A reland is coming soon (TM). Bug: 894208 Change-Id: I29e9dc851fcd45e4ad657d055ac2db0514a9563d Reviewed-on: https://chromium-review.googlesource.com/c/1305693 Commit-Queue: agrieve <agrieve@chromium.org> Reviewed-by: Mohamed Heikal <mheikal@chromium.org> Cr-Commit-Position: refs/heads/master@{#603694} [delete] https://crrev.com/adf536f12069e19379095f762fff81d9254ebd5d/android_webview/aapt2.config [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/android_webview/apk/java/src/com/android/webview/chromium/LicenseActivity.java [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/build/android/gyp/apkbuilder.py [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/build/android/gyp/compile_resources.py [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/build/android/gyp/create_app_bundle.py [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/build/android/gyp/write_build_config.py [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/build/config/android/internal_rules.gni [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/build/config/android/rules.gni [modify] https://crrev.com/2063ba8e30cb32e88feda0f5ebcb22cac4fbd1aa/chrome/android/chrome_public_apk_tmpl.gni
,
Oct 30
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1b52ff6a17671e9dcfc91fcdc6d86398730473e0 commit 1b52ff6a17671e9dcfc91fcdc6d86398730473e0 Author: Mohamed Heikal <mheikal@chromium.org> Date: Tue Oct 30 17:19:59 2018 [Resources] Output R.txt files to out/*/apks dir for archiving Resource names are being stripped from the apk. Output the R.txt files to the apks directory for archiving during official builds. Bug: 894208 Change-Id: Iba0499fb2ac6dccc22a8997b546955733d4e5e23 Reviewed-on: https://chromium-review.googlesource.com/c/1305813 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Cr-Commit-Position: refs/heads/master@{#603938} [modify] https://crrev.com/1b52ff6a17671e9dcfc91fcdc6d86398730473e0/build/android/gyp/create_app_bundle.py [modify] https://crrev.com/1b52ff6a17671e9dcfc91fcdc6d86398730473e0/build/android/gyp/write_build_config.py [modify] https://crrev.com/1b52ff6a17671e9dcfc91fcdc6d86398730473e0/build/config/android/internal_rules.gni [modify] https://crrev.com/1b52ff6a17671e9dcfc91fcdc6d86398730473e0/build/config/android/rules.gni
,
Oct 30
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/1633f3bbef6c0113f684bab454c9d92b3cbdd375 commit 1633f3bbef6c0113f684bab454c9d92b3cbdd375 Author: Mohamed Heikal <mheikal@chromium.org> Date: Tue Oct 30 22:12:32 2018
,
Oct 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2747b4b86b9c0f6a22b66bd15394b0c98c6be616 commit 2747b4b86b9c0f6a22b66bd15394b0c98c6be616 Author: Mohamed Heikal <mheikal@chromium.org> Date: Wed Oct 31 22:20:42 2018 [Build] Update third_party/android_build_tools/aapt2 Updating third_party/android_build_tools/aapt2 to version 3.3.0-beta01-5013011. This is because the current version has bug with regards to webview resources package ids (see https://crbug.com/899656) Bug: 894208, 899656 Change-Id: I0258c3b9ab0018a9fe01f23d5ab3b81faf3bd49c Reviewed-on: https://chromium-review.googlesource.com/c/1307995 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Cr-Commit-Position: refs/heads/master@{#604424} [modify] https://crrev.com/2747b4b86b9c0f6a22b66bd15394b0c98c6be616/DEPS [modify] https://crrev.com/2747b4b86b9c0f6a22b66bd15394b0c98c6be616/third_party/android_build_tools/aapt2/README.chromium [modify] https://crrev.com/2747b4b86b9c0f6a22b66bd15394b0c98c6be616/third_party/android_build_tools/aapt2/cipd.yaml
,
Oct 31
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5 commit d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5 Author: Mohamed Heikal <mheikal@chromium.org> Date: Wed Oct 31 22:25:29 2018 Reland "[Android] Remove most resource names in resources.arsc" This is a reland of 8625939271762113637e56173b8d5a6c6ea6c765 TBR=This is the same cl no changes Original change's description: > [Android] Remove most resource names in resources.arsc > > resource names are stored in the resources.arsc file to allow for > getIdentifier with the string name of the resource to work. This cl > obfuscates all of those resources to one name (and thus stored only > once) for all resources that are not accessed via getIdentifier. > > This improves binary size by about 200KB. > > Bug: 894208 > > Change-Id: I28c440c22b90cd045f53017073fdb88c7410d530 > Reviewed-on: https://chromium-review.googlesource.com/c/1265897 > Commit-Queue: Mohamed Heikal <mheikal@chromium.org> > Reviewed-by: Richard Coles <torne@chromium.org> > Reviewed-by: agrieve <agrieve@chromium.org> > Reviewed-by: Ted Choc <tedchoc@chromium.org> > Cr-Commit-Position: refs/heads/master@{#603229} Bug: 894208 Change-Id: Id023518c91c0c98cf68f15efad286c4874b4976d Reviewed-on: https://chromium-review.googlesource.com/c/1308991 Reviewed-by: Mohamed Heikal <mheikal@chromium.org> Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Cr-Commit-Position: refs/heads/master@{#604426} [add] https://crrev.com/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5/android_webview/aapt2.config [modify] https://crrev.com/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5/android_webview/apk/java/src/com/android/webview/chromium/LicenseActivity.java [modify] https://crrev.com/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5/build/android/gyp/apkbuilder.py [modify] https://crrev.com/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5/build/android/gyp/compile_resources.py [modify] https://crrev.com/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5/build/config/android/internal_rules.gni [modify] https://crrev.com/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5/build/config/android/rules.gni [modify] https://crrev.com/d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5/chrome/android/chrome_public_apk_tmpl.gni
,
Nov 1
The resource ids are pretty important for tests in general which may want to interact with the UI so we should not land that cl. The assumption "Since resources are rarely accessed by anything other than their ids" is not true for appium based tests. Text or xpath is not as reliable as resource ids when used for these tests. I don't think the savings is worth the cost. We have a large number of appium based tests that rely on this.
,
Nov 1
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/65f0e20ac217ae4cfbfec49fa98d73c004690148 commit 65f0e20ac217ae4cfbfec49fa98d73c004690148 Author: Mohamed Heikal <mheikal@chromium.org> Date: Thu Nov 01 20:51:21 2018 Revert "Reland "[Android] Remove most resource names in resources.arsc"" This reverts commit d3fd7a3e0505a4ca842d5c74cdfa125a8a381ef5. Reason for revert: breaks ui tests Original change's description: > Reland "[Android] Remove most resource names in resources.arsc" > > This is a reland of 8625939271762113637e56173b8d5a6c6ea6c765 > > TBR=This is the same cl no changes > > Original change's description: > > [Android] Remove most resource names in resources.arsc > > > > resource names are stored in the resources.arsc file to allow for > > getIdentifier with the string name of the resource to work. This cl > > obfuscates all of those resources to one name (and thus stored only > > once) for all resources that are not accessed via getIdentifier. > > > > This improves binary size by about 200KB. > > > > Bug: 894208 > > > > Change-Id: I28c440c22b90cd045f53017073fdb88c7410d530 > > Reviewed-on: https://chromium-review.googlesource.com/c/1265897 > > Commit-Queue: Mohamed Heikal <mheikal@chromium.org> > > Reviewed-by: Richard Coles <torne@chromium.org> > > Reviewed-by: agrieve <agrieve@chromium.org> > > Reviewed-by: Ted Choc <tedchoc@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#603229} > > Bug: 894208 > Change-Id: Id023518c91c0c98cf68f15efad286c4874b4976d > Reviewed-on: https://chromium-review.googlesource.com/c/1308991 > Reviewed-by: Mohamed Heikal <mheikal@chromium.org> > Commit-Queue: Mohamed Heikal <mheikal@chromium.org> > Cr-Commit-Position: refs/heads/master@{#604426} TBR=torne@chromium.org,tedchoc@chromium.org,agrieve@chromium.org,mheikal@chromium.org Change-Id: I1e4d1de0ea707d6dddfc82b13a44854fc30be753 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 894208 Reviewed-on: https://chromium-review.googlesource.com/c/1313279 Reviewed-by: Mohamed Heikal <mheikal@chromium.org> Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Cr-Commit-Position: refs/heads/master@{#604700} [delete] https://crrev.com/d833bb6b23a98904d06ba045f07d15409e373e2b/android_webview/aapt2.config [modify] https://crrev.com/65f0e20ac217ae4cfbfec49fa98d73c004690148/android_webview/apk/java/src/com/android/webview/chromium/LicenseActivity.java [modify] https://crrev.com/65f0e20ac217ae4cfbfec49fa98d73c004690148/build/android/gyp/apkbuilder.py [modify] https://crrev.com/65f0e20ac217ae4cfbfec49fa98d73c004690148/build/android/gyp/compile_resources.py [modify] https://crrev.com/65f0e20ac217ae4cfbfec49fa98d73c004690148/build/config/android/internal_rules.gni [modify] https://crrev.com/65f0e20ac217ae4cfbfec49fa98d73c004690148/build/config/android/rules.gni [modify] https://crrev.com/65f0e20ac217ae4cfbfec49fa98d73c004690148/chrome/android/chrome_public_apk_tmpl.gni
,
Nov 1
I'm unsure how this would affect other tests, but espresso tests also use id's. I haven't tried the tests yet, so am unsure if they're broken, just hypothesizing. https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromium/chrome/browser/ExampleUiCaptureTest.java?q=import+android.support.test.espresso.Espresso;&sq=package:chromium&dr=C&l=7 Has "Espresso.onView(ViewMatchers.withId(R.id.tab_switcher_button)).perform(ViewActions.click());"
,
Nov 1
I am not removing ids, I am removing names. So that line would still work. The only issue is if something that is not java (ie. does not use the R.java file to get the id but relies on the name of the resource in the final apk, like franky).
,
Nov 1
We have 200+ tests on Franky (built on Appium so is affected by resource ids) that uses this. Most of them rely on the resource ids to work. The Android automation viewer is another application that surfaces the resource ids. Internal and external testers + developers may have workflows that make use of these ids as well either manually or in tests.
,
Nov 1
re c10, it's not using the string form, so should be ok.
,
Nov 2
The way I see it, there are a few ways to work around this issue: 1- Patch franky/appium to allow it to translate resource name strings to resource ids from the archived R.txt file. The difficulty of this might be more apparent to aluo@ or bjoyce@ 2- Upload an unoptimized apk to the storage bucket to be used by the testers. The problem with that is we would be testing a different apk than the one that is actually shipped. 3- Go through the test scripts (they are google sheets as I understand) extract the resource ids used and whitelist them keeping them in the final apk. The problem with this is that the test scripts are spread out among different google sheets and it would require a lot of manual labour or writing a tool that interacts with google sheets api. 4. Keep all id resources names (which are the string names given to UI elements, basically R.id.*). I will add those to the design doc (go/chrome-resource-name-obfuscation).
,
Nov 2
If at all possible, we try not to include things in Chrome.apk that are only for testing purposes. I think good options would be: 1) Add support to the test framework to map names->numeric IDs via the R.txt files, or 2) Have Franky / other test frameworks use ChromePublicApkForTesting.apk instead.
,
Nov 2
As an aside, Franky sounds really neat! I didn't know we had such tests! I can find any info about it in markdown, nor on go/clank though. How can I run them, or write my own?
,
Nov 2
System health benchmarks are also breaking: Issue 900909
,
Nov 2
Catapult/devil uses uiautomator to dump an "xml screenshot" and use that to send touch events in the correct location and automate the ui for some benchmarks.
,
Nov 3
Apparently it also causes failures in espresso tests that look for the webview text actionmode popup such as "Copy" and "Cut", it's causing 2 copies of them to show up: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8931559147125214656/+/steps/webview_ui_test_app_test_apk/0/stdout https://chromium.googlesource.com/chromium/src/tools/franky/ is the link for franky.
,
Nov 3
Issue 899976 has been merged into this issue.
,
Nov 3
That issue was fixed before the last reland. I am working on a fix for Franky and uiautomator issues, at least as a stopgap measure to go forward with another reland.
,
Nov 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e734bc5eb8b08668196b95a9e38e6bea0773748c commit e734bc5eb8b08668196b95a9e38e6bea0773748c Author: Mohamed Heikal <mheikal@chromium.org> Date: Mon Nov 05 20:48:41 2018 Reland x2 "[Android] Remove most resource names in resources.arsc" This reverts commit 65f0e20ac217ae4cfbfec49fa98d73c004690148. Reason for reland: Fixes crbug.com/900909 and crbug.com/900993 TBR=torne@chromium.org (No changes in webview section) Original change's description: > [Android] Remove most resource names in resources.arsc > > resource names are stored in the resources.arsc file to allow for > getIdentifier with the string name of the resource to work. This cl > obfuscates all of those resources to one name (and thus stored only > once) for all resources that are not accessed via getIdentifier. > > This improves binary size by about 200KB. > > Bug: 894208 > > Change-Id: I28c440c22b90cd045f53017073fdb88c7410d530 > Reviewed-on: https://chromium-review.googlesource.com/c/1265897 > Commit-Queue: Mohamed Heikal <mheikal@chromium.org> > Reviewed-by: Richard Coles <torne@chromium.org> > Reviewed-by: agrieve <agrieve@chromium.org> > Reviewed-by: Ted Choc <tedchoc@chromium.org> > Cr-Commit-Position: refs/heads/master@{#603229} Bug: 894208, 900909 , 900993 Change-Id: Ie1e8790b392b5cbfff6d207c11e12bed6b8cc765 Reviewed-on: https://chromium-review.googlesource.com/c/1316092 Reviewed-by: Mohamed Heikal <mheikal@chromium.org> Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: Mohamed Heikal <mheikal@chromium.org> Cr-Commit-Position: refs/heads/master@{#605451} [add] https://crrev.com/e734bc5eb8b08668196b95a9e38e6bea0773748c/android_webview/aapt2.config [modify] https://crrev.com/e734bc5eb8b08668196b95a9e38e6bea0773748c/android_webview/apk/java/src/com/android/webview/chromium/LicenseActivity.java [modify] https://crrev.com/e734bc5eb8b08668196b95a9e38e6bea0773748c/build/android/gyp/apkbuilder.py [modify] https://crrev.com/e734bc5eb8b08668196b95a9e38e6bea0773748c/build/android/gyp/compile_resources.py [modify] https://crrev.com/e734bc5eb8b08668196b95a9e38e6bea0773748c/build/config/android/internal_rules.gni [modify] https://crrev.com/e734bc5eb8b08668196b95a9e38e6bea0773748c/build/config/android/rules.gni [modify] https://crrev.com/e734bc5eb8b08668196b95a9e38e6bea0773748c/chrome/android/chrome_public_apk_tmpl.gni |
|
►
Sign in to add a comment |
|
Comment 1 by mheikal@chromium.org
, Oct 16