Skip to content

Commit

Permalink
Reland "Enable unused resource removal for android webview"
Browse files Browse the repository at this point in the history
This reverts commit b4682a7.

Reason for revert: I don't think this was the cause of the tree closure. The tree reopened automatically just before my revert landed.

Original change's description:
> Revert "Enable unused resource removal for android webview"
>
> This reverts commit 0deb4b3.
>
> Reason for revert: Looks like it closed the tree
>
> https://ci.chromium.org/ui/p/chromium/builders/ci/chromeos-arm-generic-dbg/49672/blamelist
>
> The error is about resources.
>
> [1549/76611] ACTION //chromeos/components/hps:hps_internals_ts(//build/toolchain/cros:target)
> FAILED: gen/chromeos/components/hps/tsconfig.json gen/chromeos/components/hps/tsconfig.manifest gen/chromeos/components/hps/resources/hps_internals.js 
> python3 ../../tools/typescript/ts_library.py --root_dir ../../chromeos/components/hps --gen_dir gen/chromeos/components/hps --out_dir gen/chromeos/components/hps --in_files resources/hps_internals.ts --definitions ../../../../../../tools/typescript/definitions/chrome_send.d.ts --path_mappings chrome://resources/\*\|../../../ui/webui/resources/preprocessed/\* //resources/\*\|../../../ui/webui/resources/preprocessed/\* chrome://resources/polymer/v3_0/\*\|../../../../../../third_party/polymer/v3_0/components-chromium/\* //resources/polymer/v3_0/\*\|../../../../../../third_party/polymer/v3_0/components-chromium/\* chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js\|../../../../../../third_party/polymer/v3_0/components-chromium/polymer/polymer.d.ts //resources/polymer/v3_0/polymer/polymer_bundled.min.js\|../../../../../../third_party/polymer/v3_0/components-chromium/polymer/polymer.d.ts /tools/typescript/definitions/\*\|../../../../../../tools/typescript/definitions/\*
> Traceback (most recent call last):
>   File "../../tools/typescript/ts_library.py", line 153, in <module>
>     main(sys.argv[1:])
>   File "../../tools/typescript/ts_library.py", line 117, in main
>     node.RunNode([
>   File "/b/s/w/ir/cache/builder/src/out/Debug/../../third_party/node/node.py", line 34, in RunNode
>     raise RuntimeError('Command \'%s\' failed\n%s' % (' '.join(cmd), err))
> RuntimeError: Command '/b/s/w/ir/cache/builder/src/out/Debug/../../third_party/node/linux/node-linux-x64/bin/node /b/s/w/ir/cache/builder/src/out/Debug/../../third_party/node/node_modules/typescript/bin/tsc --project gen/chromeos/components/hps/tsconfig.json' failed
> ../../chromeos/components/hps/resources/hps_internals.ts:6:32 - error TS2792: Cannot find module 'chrome://resources/js/cr.m.js'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
>
> 6 import {addWebUIListener} from 'chrome://resources/js/cr.m.js';
>                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> ../../chromeos/components/hps/resources/hps_internals.ts:7:17 - error TS2792: Cannot find module 'chrome://resources/js/util.m.js'. Did you mean to set the 'moduleResolution' option to 'node', or to add aliases to the 'paths' option?
>
> 7 import {$} from 'chrome://resources/js/util.m.js';
>
>
> Original change's description:
> > Enable unused resource removal for android webview
> >
> > Bug: 1269757
> > Change-Id: Ib3077a1c56f32b8de5df391ee19aae1790ae507f
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3534059
> > Auto-Submit: Mohamed Heikal <mheikal@chromium.org>
> > Reviewed-by: Richard Coles <torne@chromium.org>
> > Reviewed-by: Andrew Grieve <agrieve@chromium.org>
> > Commit-Queue: Andrew Grieve <agrieve@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#984640}
>
> Bug: 1269757
> Change-Id: I8811142db0fb070881aafa68e7ecc2d0ef594635
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3546167
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Owners-Override: Fergal Daly <fergal@google.com>
> Commit-Queue: Fergal Daly <fergal@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#984657}

Bug: 1269757
Change-Id: I4cdd3dc853c466876faee12d5d1572590dd5f3c1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3546139
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Fergal Daly <fergal@google.com>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/main@{#984677}
  • Loading branch information
fergald authored and Chromium LUCI CQ committed Mar 24, 2022
1 parent 5007494 commit ebb8059
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 1 deletion.
7 changes: 7 additions & 0 deletions android_webview/system_webview_apk_tmpl.gni
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ template("system_webview_apk_or_module_tmpl") {

# Shortens resource file names in apk eg: res/drawable/button.xml -> res/a.xml
short_resource_paths = !is_java_debug

# Removes unused resources from the apk. Only enabled on official builds
# since it adds a slow step and serializes the build graph causing fewer
# expensive tasks (eg: proguarding, resource optimization) to be run in
# parallel by adding dependencies between them (adds around 10-20
# seconds on my machine).
strip_unused_resources = is_official_build
}

if (!_is_bundle_module) {
Expand Down
3 changes: 3 additions & 0 deletions android_webview/system_webview_bundle.gni
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ template("system_webview_bundle") {
proguard_android_sdk_dep = webview_framework_dep
}

# For this to be respected, it must also be set on the base module target.
strip_unused_resources = is_official_build

forward_variables_from(invoker, "*")
}
}
11 changes: 10 additions & 1 deletion build/android/unused_resources/UnusedResources.java
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,10 @@ private void recordClassUsages(File file, String name, byte[] bytes) {
ReferenceChecker callback = new ReferenceChecker() {
@Override
public boolean shouldProcess(String internalName) {
return !isResourceClass(internalName + DOT_CLASS);
// We do not need to ignore R subclasses since R8 now removes
// unused resource id fields in R subclasses thus their
// remaining presence means real usage.
return true;
}

@Override
Expand All @@ -400,6 +403,12 @@ public void referencedStaticField(String internalName, String fieldName) {
Resource resource = getResourceFromCode(internalName, fieldName);
if (resource != null) {
ResourceUsageModel.markReachable(resource);
if (mDebugPrinter != null) {
mDebugPrinter.println("Marking " + resource
+ " reachable: referenced from dex"
+ " in " + file + ":" + name + " (static field access "
+ internalName + "." + fieldName + ")");
}
}
}

Expand Down

0 comments on commit ebb8059

Please sign in to comment.