Trace android:onClick method references
These method references are not neccesarily bound to the context
(class) of the current xml file, but instead can be based on the
activity that calls setContentView.
Bug: b/383471657
Change-Id: Ic7f98f3e7d4bad47f41c969cbc2beae748f167bb
diff --git a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
index b39b630..bc74f85 100644
--- a/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
+++ b/src/main/java/com/android/tools/r8/shaking/Enqueuer.java
@@ -17,6 +17,7 @@
import static com.android.tools.r8.utils.MapUtils.ignoreKey;
import static java.util.Collections.emptySet;
+import com.android.build.shrinker.r8integration.R8ResourceShrinkerState;
import com.android.tools.r8.Diagnostic;
import com.android.tools.r8.cf.code.CfInstruction;
import com.android.tools.r8.cf.code.CfInvoke;
@@ -527,7 +528,9 @@
this.initialPrunedTypes = initialPrunedTypes;
if (options.isOptimizedResourceShrinking()) {
- appView.getResourceShrinkerState().setEnqueuerCallback(this::recordReferenceFromResources);
+ R8ResourceShrinkerState resourceShrinkerState = appView.getResourceShrinkerState();
+ resourceShrinkerState.setEnqueuerCallback(this::recordReferenceFromResources);
+ resourceShrinkerState.setEnqueuerMethodCallback(this::recordMethodReferenceFromResources);
}
EnqueuerAnalysisCollection.Builder analysesBuilder = EnqueuerAnalysisCollection.builder();
@@ -664,6 +667,12 @@
recordTypeReference(type, context, this::recordNonProgramClass, this::reportMissingClass);
}
+ private final Map<DexString, Origin> onClickMethodReferences = new HashMap<>();
+
+ private void recordMethodReferenceFromResources(String method, Origin origin) {
+ onClickMethodReferences.put(appView.dexItemFactory().createString(method), origin);
+ }
+
private boolean recordReferenceFromResources(String possibleClass, Origin origin) {
if (!DescriptorUtils.isValidJavaType(possibleClass)) {
return false;
@@ -673,17 +682,7 @@
DexProgramClass clazz = appView.definitionForProgramType(dexType);
if (clazz != null) {
ReflectiveUseFromXml reason = KeepReason.reflectiveUseFromXml(origin);
- applyMinimumKeepInfoWhenLive(
- clazz,
- KeepClassInfo.newEmptyJoiner()
- .disallowMinification()
- .disallowRepackaging()
- .disallowOptimization());
- if (clazz.isAnnotation() || clazz.isInterface()) {
- markTypeAsLive(clazz, reason);
- } else {
- markClassAsInstantiatedWithReason(clazz, reason);
- }
+ ensureClassKeptForResourceLookup(clazz, reason);
for (ProgramMethod programInstanceInitializer : clazz.programInstanceInitializers()) {
// TODO(b/325884671): Only keep the actually framework targeted constructors.
applyMinimumKeepInfoWhenLiveOrTargeted(
@@ -695,6 +694,21 @@
return clazz != null;
}
+ private void ensureClassKeptForResourceLookup(
+ DexProgramClass clazz, ReflectiveUseFromXml reason) {
+ applyMinimumKeepInfoWhenLive(
+ clazz,
+ KeepClassInfo.newEmptyJoiner()
+ .disallowMinification()
+ .disallowRepackaging()
+ .disallowOptimization());
+ if (clazz.isAnnotation() || clazz.isInterface()) {
+ markTypeAsLive(clazz, reason);
+ } else {
+ markClassAsInstantiatedWithReason(clazz, reason);
+ }
+ }
+
private void recordTypeReference(
DexType type,
ProgramDerivedContext context,
@@ -1062,7 +1076,10 @@
worklist.enqueueMarkMethodKeptAction(
method,
graphReporter.reportKeepMethod(
- precondition, minimumKeepInfo.getRules(), method.getDefinition()));
+ precondition,
+ minimumKeepInfo.getReasons(),
+ minimumKeepInfo.getRules(),
+ method.getDefinition()));
}
private void enqueueFirstNonSerializableClassInitializer(
@@ -2316,6 +2333,37 @@
analyses.processNewlyLiveClass(clazz, worklist);
}
+ private void processOnClickMethods() {
+ if (onClickMethodReferences.isEmpty()) {
+ return;
+ }
+ for (DexProgramClass item : liveTypes.items) {
+ for (ProgramMethod method :
+ item.virtualProgramMethods(
+ p ->
+ p.getParameters().size() == 1
+ && p.getParameter(0)
+ .isIdenticalTo(appInfo.dexItemFactory().androidViewViewType)
+ && onClickMethodReferences.containsKey(p.getName()))) {
+ KeepMethodInfo methodInfo = keepInfo.getMethodInfo(method);
+ if (!methodInfo.isOptimizationAllowed(options)
+ && !methodInfo.isShrinkingAllowed(options)
+ && !methodInfo.isMinificationAllowed(options)) {
+ continue;
+ }
+ ReflectiveUseFromXml reason =
+ KeepReason.reflectiveUseFromXml(onClickMethodReferences.get(method.getName()));
+ Joiner minimumKeepInfo =
+ KeepMethodInfo.newEmptyJoiner()
+ .disallowOptimization()
+ .disallowShrinking()
+ .disallowMinification()
+ .addReason(reason);
+ applyMinimumKeepInfo(method, minimumKeepInfo);
+ }
+ }
+ }
+
private void processDeferredAnnotations(
Map<DexType, Map<DexAnnotation, List<ProgramDefinition>>> deferredAnnotations,
Function<ProgramDefinition, AnnotatedKind> kindProvider) {
@@ -4020,6 +4068,11 @@
}
}
+ private void applyMinimumKeepInfo(ProgramMethod method, KeepMethodInfo.Joiner minimumKeepInfo) {
+ keepInfo.joinMethod(method, info -> info.merge(minimumKeepInfo));
+ enqueueMethodIfShrinkingIsDisallowed(method, UnconditionalKeepInfoEvent.get(), minimumKeepInfo);
+ }
+
public void applyMinimumKeepInfoWhenLiveOrTargeted(
ProgramMethod method, KeepMethodInfo.Joiner minimumKeepInfo) {
applyMinimumKeepInfoWhenLiveOrTargeted(method, minimumKeepInfo, EnqueuerEvent.unconditional());
@@ -4701,6 +4754,14 @@
deferredParameterAnnotations, annotatedItem -> AnnotatedKind.PARAMETER);
timing.end();
+ timing.begin("Process onclick methods");
+ processOnClickMethods();
+ timing.end();
+ if (worklist.hasNext()) {
+ timing.end();
+ continue;
+ }
+
// Continue fix-point processing while there are additional work items to ensure items that
// are passed to Java reflections are traced.
if (!pendingReflectiveUses.isEmpty()) {
diff --git a/src/main/java/com/android/tools/r8/shaking/GraphReporter.java b/src/main/java/com/android/tools/r8/shaking/GraphReporter.java
index 3ae9270..de2b024 100644
--- a/src/main/java/com/android/tools/r8/shaking/GraphReporter.java
+++ b/src/main/java/com/android/tools/r8/shaking/GraphReporter.java
@@ -30,7 +30,6 @@
import com.android.tools.r8.ir.desugar.itf.InterfaceDesugaringSyntheticHelper;
import com.android.tools.r8.references.Reference;
import com.android.tools.r8.references.TypeReference;
-import com.android.tools.r8.shaking.KeepReason.ReflectiveUseFrom;
import com.android.tools.r8.utils.DequeUtils;
import com.android.tools.r8.utils.InternalOptions;
import com.android.tools.r8.utils.SetUtils;
@@ -142,9 +141,15 @@
}
KeepReasonWitness reportKeepMethod(
- DexDefinition precondition, Collection<ProguardKeepRuleBase> rules, DexEncodedMethod method) {
- assert !rules.isEmpty() || !options.isShrinking();
+ DexDefinition precondition,
+ Collection<KeepReason> reasons,
+ Collection<ProguardKeepRuleBase> rules,
+ DexEncodedMethod method) {
+ assert !reasons.isEmpty() || !rules.isEmpty() || !options.isShrinking();
if (keptGraphConsumer != null) {
+ for (KeepReason reason : reasons) {
+ registerMethod(method, reason);
+ }
for (ProguardKeepRuleBase rule : rules) {
reportKeepMethod(precondition, rule, method);
}
@@ -164,7 +169,7 @@
KeepReasonWitness reportKeepField(
DexDefinition precondition,
- Collection<ReflectiveUseFrom> reasons,
+ Collection<KeepReason> reasons,
Collection<ProguardKeepRuleBase> rules,
DexEncodedField field) {
assert !reasons.isEmpty() || !rules.isEmpty() || !options.isShrinking();
diff --git a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
index 4b13a61..8818777 100644
--- a/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
+++ b/src/main/java/com/android/tools/r8/shaking/KeepInfo.java
@@ -554,7 +554,7 @@
* <p>These are only needed for the interpretation of keep rules into keep info, and is
* therefore not stored in the keep info builder above.
*/
- final Set<ReflectiveUseFrom> reasons = new HashSet<>();
+ final Set<KeepReason> reasons = new HashSet<>();
final Set<ProguardKeepRuleBase> rules = Sets.newIdentityHashSet();
@@ -593,7 +593,7 @@
return null;
}
- public Set<ReflectiveUseFrom> getReasons() {
+ public Set<KeepReason> getReasons() {
return reasons;
}
@@ -630,7 +630,7 @@
return self();
}
- public J addReason(ReflectiveUseFrom reason) {
+ public J addReason(KeepReason reason) {
reasons.add(reason);
return self();
}
diff --git a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
index 34e581b..3dded16 100644
--- a/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
+++ b/src/resourceshrinker/java/com/android/build/shrinker/r8integration/R8ResourceShrinkerState.java
@@ -61,6 +61,7 @@
private final Map<FeatureSplit, ResourceTable> resourceTables = new HashMap<>();
private final ShrinkerDebugReporter shrinkerDebugReporter;
private ClassReferenceCallback enqueuerCallback;
+ private MethodReferenceCallback methodCallback;
private Map<Integer, List<String>> resourceIdToXmlFiles;
private Set<String> packageNames;
private final Set<String> seenNoneClassValues = new HashSet<>();
@@ -85,6 +86,11 @@
boolean tryClass(String possibleClass, Origin xmlFileOrigin);
}
+ @FunctionalInterface
+ public interface MethodReferenceCallback {
+ void tryMethod(String methodName, Origin xmlFileOrigin);
+ }
+
public R8ResourceShrinkerState(
Function<Exception, RuntimeException> errorHandler,
ShrinkerDebugReporter shrinkerDebugReporter) {
@@ -142,6 +148,11 @@
this.enqueuerCallback = enqueuerCallback;
}
+ public void setEnqueuerMethodCallback(MethodReferenceCallback methodCallback) {
+ assert this.methodCallback == null;
+ this.methodCallback = methodCallback;
+ }
+
private synchronized Set<String> getPackageNames() {
// TODO(b/325888516): Consider only doing this for the package corresponding to the current
// feature.
@@ -278,10 +289,11 @@
private void visitNode(XmlNode xmlNode, String xmlName, String manifestPackageName) {
XmlElement element = xmlNode.getElement();
- tryEnqueuerOnString(element.getName(), xmlName);
+ String xmlElementName = element.getName();
+ tryEnqueuerOnString(xmlElementName, xmlName);
for (XmlAttribute xmlAttribute : element.getAttributeList()) {
- if (xmlAttribute.getName().equals("package") && element.getName().equals("manifest")) {
+ if (xmlAttribute.getName().equals("package") && xmlElementName.equals("manifest")) {
// We are traversing a manifest, record the package name if we see it.
manifestPackageName = xmlAttribute.getValue();
}
@@ -295,6 +307,10 @@
// Manifest case
traceManifestSpecificValues(xmlName, manifestPackageName, xmlAttribute, element);
}
+ if (xmlAttribute.getName().equals("onClick")
+ && xmlAttribute.getNamespaceUri().equals("http://schemas.android.com/apk/res/android")) {
+ methodCallback.tryMethod(xmlAttribute.getValue(), new PathOrigin(Paths.get(xmlName)));
+ }
}
for (XmlNode node : element.getChildList()) {
visitNode(node, xmlName, manifestPackageName);
@@ -381,6 +397,7 @@
public void enqueuerDone(boolean isFinalTreeshaking) {
enqueuerCallback = null;
+ methodCallback = null;
seenResourceIds.clear();
if (!isFinalTreeshaking) {
// After final tree shaking we will need the reachability bits to decide what to write out
diff --git a/src/test/java/android/view/View.java b/src/test/java/android/view/View.java
new file mode 100644
index 0000000..2062996
--- /dev/null
+++ b/src/test/java/android/view/View.java
@@ -0,0 +1,7 @@
+// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+package android.view;
+
+public class View {}
diff --git a/src/test/java/com/android/tools/r8/androidresources/OnClickMethodReferenceTest.java b/src/test/java/com/android/tools/r8/androidresources/OnClickMethodReferenceTest.java
new file mode 100644
index 0000000..16cce10
--- /dev/null
+++ b/src/test/java/com/android/tools/r8/androidresources/OnClickMethodReferenceTest.java
@@ -0,0 +1,136 @@
+// Copyright (c) 2024, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package com.android.tools.r8.androidresources;
+
+import static com.android.tools.r8.utils.codeinspector.Matchers.isAbsent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresent;
+import static com.android.tools.r8.utils.codeinspector.Matchers.isPresentAndNotRenamed;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+import android.view.View;
+import com.android.tools.r8.TestBase;
+import com.android.tools.r8.TestParameters;
+import com.android.tools.r8.TestParametersCollection;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResource;
+import com.android.tools.r8.androidresources.AndroidResourceTestingUtils.AndroidTestResourceBuilder;
+import com.android.tools.r8.utils.codeinspector.ClassSubject;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
+
+@RunWith(Parameterized.class)
+public class OnClickMethodReferenceTest extends TestBase {
+
+ @Parameter(0)
+ public TestParameters parameters;
+
+ @Parameters(name = "{0}")
+ public static TestParametersCollection parameters() {
+ return getTestParameters().withDefaultDexRuntime().withAllApiLevels().build();
+ }
+
+ public static String XML_WITH_ONCLICK =
+ "<"
+ + Bar.class.getTypeName()
+ + " xmlns:android=\"http://schemas.android.com/apk/res/android\"\n"
+ + " android:onClick=\"theMagicOnClick\""
+ + "/>";
+
+ public static String XML_WITH_NESTED_CLASS_AND_ONCLICK =
+ "<"
+ + Bar.class.getTypeName()
+ + " xmlns:android=\"http://schemas.android.com/apk/res/android\">\n"
+ + " <"
+ + Foo.class.getTypeName()
+ + " xmlns:android=\"http://schemas.android.com/apk/res/android\"\n"
+ + " android:onClick=\"theMagicOnClick\""
+ + " />"
+ + "</"
+ + Bar.class.getTypeName()
+ + ">";
+
+ public static AndroidTestResource getTestResources(TemporaryFolder temp, String xml)
+ throws Exception {
+ return new AndroidTestResourceBuilder()
+ .withSimpleManifestAndAppNameString()
+ .addRClassInitializeWithDefaultValues(R.xml.class)
+ .addXml("xml_with_onclick.xml", xml)
+ .build(temp);
+ }
+
+ @Test
+ public void testSingleClassWithOnclick() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(TestClass.class, Bar.class, Foo.class)
+ .addAndroidResources(getTestResources(temp, XML_WITH_ONCLICK))
+ .addKeepMainRule(TestClass.class)
+ .enableOptimizedShrinking()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject barClass = codeInspector.clazz(Bar.class);
+ assertThat(barClass, isPresent());
+ assertThat(
+ barClass.uniqueMethodWithOriginalName("theMagicOnClick"),
+ isPresentAndNotRenamed());
+
+ ClassSubject fooClass = codeInspector.clazz(Foo.class);
+ assertThat(fooClass, isAbsent());
+ });
+ }
+
+ @Test
+ public void testNestedClassWithOnclick() throws Exception {
+ testForR8(parameters.getBackend())
+ .setMinApi(parameters)
+ .addProgramClasses(TestClass.class, Bar.class, Foo.class)
+ .addAndroidResources(getTestResources(temp, XML_WITH_NESTED_CLASS_AND_ONCLICK))
+ .addKeepMainRule(TestClass.class)
+ .enableOptimizedShrinking()
+ .compile()
+ .inspect(
+ codeInspector -> {
+ ClassSubject barClass = codeInspector.clazz(Bar.class);
+ assertThat(barClass, isPresent());
+ assertThat(
+ barClass.uniqueMethodWithOriginalName("theMagicOnClick"),
+ isPresentAndNotRenamed());
+ ClassSubject fooClass = codeInspector.clazz(Foo.class);
+ assertThat(fooClass, isPresentAndNotRenamed());
+ assertThat(
+ fooClass.uniqueMethodWithOriginalName("theMagicOnClick"),
+ isPresentAndNotRenamed());
+ });
+ }
+
+ public static class TestClass {
+ public static void main(String[] args) {
+ // Reference only the xml
+ System.out.println(R.xml.xml_with_onclick);
+ }
+ }
+
+ public static class R {
+ public static class xml {
+ public static int xml_with_onclick;
+ }
+ }
+}
+
+// Element names can't include $, so don't have these as an inner class.
+class Bar {
+ public void theMagicOnClick(View view) {
+ System.out.println("clicked the magic button");
+ }
+}
+
+class Foo {
+ public void theMagicOnClick(View view) {
+ System.out.println("clicked the magic button");
+ }
+}
