From a8c29399f8c01fe6a04b7f0b1d2f5934ae7de222 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 28 Mar 2023 14:58:41 -0600 Subject: [PATCH] Fix missing broadcast ANR culprit information. All TimeoutRecord have unfortunately been marked as "unknown culprit" due to the Intent not having a ComponentName filled in. We fix this by passing the relevant package and class name through the record, and we expand it to identify runtime registered receiver classes to aid faster bug triage. For now, we're abusing the class name transported via "receiverId" to avoid the churn of adding yet another method argument late in the release cycle. Bug: 266169149 Test: atest InternalTests:TimeoutRecordTest Test: atest FrameworksMockingServicesTests:BroadcastQueueTest Test: atest FrameworksMockingServicesTests:BroadcastQueueModernImplTest Test: atest FrameworksMockingServicesTests:BroadcastRecordTest Change-Id: I67b6f672c24627b7281de3b218e1114ab8a48f74 --- core/java/android/content/Intent.java | 7 ++-- .../android/internal/os/TimeoutRecord.java | 35 ++++++++++++++++--- .../android/server/am/BroadcastFilter.java | 11 ++++++ .../server/am/BroadcastQueueModernImpl.java | 6 +++- .../android/server/am/BroadcastRecord.java | 8 +++++ .../internal/os/TimeoutRecordTest.java | 30 +++++++++++----- 6 files changed, 81 insertions(+), 16 deletions(-) diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index df8da246c976c..58b0571653f16 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -11356,12 +11356,15 @@ public class Intent implements Parcelable, Cloneable { @Override public String toString() { StringBuilder b = new StringBuilder(128); + toString(b); + return b.toString(); + } + /** @hide */ + public void toString(@NonNull StringBuilder b) { b.append("Intent { "); toShortString(b, true, true, true, false); b.append(" }"); - - return b.toString(); } /** @hide */ diff --git a/core/java/com/android/internal/os/TimeoutRecord.java b/core/java/com/android/internal/os/TimeoutRecord.java index 2f6091bc32661..a0e29347d07f8 100644 --- a/core/java/com/android/internal/os/TimeoutRecord.java +++ b/core/java/com/android/internal/os/TimeoutRecord.java @@ -18,6 +18,8 @@ package com.android.internal.os; import android.annotation.IntDef; import android.annotation.NonNull; +import android.annotation.Nullable; +import android.content.ComponentName; import android.content.Intent; import android.os.SystemClock; @@ -96,20 +98,43 @@ public class TimeoutRecord { return new TimeoutRecord(kind, reason, endUptimeMillis, /* endTakenBeforeLocks */ false); } + /** Record for a broadcast receiver timeout. */ + @NonNull + public static TimeoutRecord forBroadcastReceiver(@NonNull Intent intent, + @Nullable String packageName, @Nullable String className) { + final Intent logIntent; + if (packageName != null) { + if (className != null) { + logIntent = new Intent(intent); + logIntent.setComponent(new ComponentName(packageName, className)); + } else { + logIntent = new Intent(intent); + logIntent.setPackage(packageName); + } + } else { + logIntent = intent; + } + return forBroadcastReceiver(logIntent); + } + /** Record for a broadcast receiver timeout. */ @NonNull public static TimeoutRecord forBroadcastReceiver(@NonNull Intent intent) { - String reason = "Broadcast of " + intent.toString(); - return TimeoutRecord.endingNow(TimeoutKind.BROADCAST_RECEIVER, reason); + final StringBuilder reason = new StringBuilder("Broadcast of "); + intent.toString(reason); + return TimeoutRecord.endingNow(TimeoutKind.BROADCAST_RECEIVER, reason.toString()); } /** Record for a broadcast receiver timeout. */ @NonNull public static TimeoutRecord forBroadcastReceiver(@NonNull Intent intent, long timeoutDurationMs) { - String reason = "Broadcast of " + intent.toString() + ", waited " + timeoutDurationMs - + "ms"; - return TimeoutRecord.endingNow(TimeoutKind.BROADCAST_RECEIVER, reason); + final StringBuilder reason = new StringBuilder("Broadcast of "); + intent.toString(reason); + reason.append(", waited "); + reason.append(timeoutDurationMs); + reason.append("ms"); + return TimeoutRecord.endingNow(TimeoutKind.BROADCAST_RECEIVER, reason.toString()); } /** Record for an input dispatch no focused window timeout */ diff --git a/services/core/java/com/android/server/am/BroadcastFilter.java b/services/core/java/com/android/server/am/BroadcastFilter.java index a92723ee70361..749427730ca0a 100644 --- a/services/core/java/com/android/server/am/BroadcastFilter.java +++ b/services/core/java/com/android/server/am/BroadcastFilter.java @@ -16,6 +16,7 @@ package com.android.server.am; +import android.annotation.Nullable; import android.content.IntentFilter; import android.util.PrintWriterPrinter; import android.util.Printer; @@ -55,6 +56,16 @@ final class BroadcastFilter extends IntentFilter { exported = _exported; } + public @Nullable String getReceiverClassName() { + if (receiverId != null) { + final int index = receiverId.lastIndexOf('@'); + if (index > 0) { + return receiverId.substring(0, index); + } + } + return null; + } + @NeverCompile public void dumpDebug(ProtoOutputStream proto, long fieldId) { long token = proto.start(fieldId); diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java index 1f0b1628aa223..93173abeb106c 100644 --- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java +++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java @@ -32,6 +32,7 @@ import static com.android.server.am.BroadcastProcessQueue.insertIntoRunnableList import static com.android.server.am.BroadcastProcessQueue.reasonToString; import static com.android.server.am.BroadcastProcessQueue.removeFromRunnableList; import static com.android.server.am.BroadcastRecord.deliveryStateToString; +import static com.android.server.am.BroadcastRecord.getReceiverClassName; import static com.android.server.am.BroadcastRecord.getReceiverPackageName; import static com.android.server.am.BroadcastRecord.getReceiverProcessName; import static com.android.server.am.BroadcastRecord.getReceiverUid; @@ -1040,7 +1041,10 @@ class BroadcastQueueModernImpl extends BroadcastQueue { if (deliveryState == BroadcastRecord.DELIVERY_TIMEOUT) { r.anrCount++; if (app != null && !app.isDebugging()) { - mService.appNotResponding(queue.app, TimeoutRecord.forBroadcastReceiver(r.intent)); + final String packageName = getReceiverPackageName(receiver); + final String className = getReceiverClassName(receiver); + mService.appNotResponding(queue.app, + TimeoutRecord.forBroadcastReceiver(r.intent, packageName, className)); } } else { mLocalHandler.removeMessages(MSG_DELIVERY_TIMEOUT_SOFT, queue); diff --git a/services/core/java/com/android/server/am/BroadcastRecord.java b/services/core/java/com/android/server/am/BroadcastRecord.java index 6bd3c7953e018..752526843cf8c 100644 --- a/services/core/java/com/android/server/am/BroadcastRecord.java +++ b/services/core/java/com/android/server/am/BroadcastRecord.java @@ -832,6 +832,14 @@ final class BroadcastRecord extends Binder { } } + static @Nullable String getReceiverClassName(@NonNull Object receiver) { + if (receiver instanceof BroadcastFilter) { + return ((BroadcastFilter) receiver).getReceiverClassName(); + } else /* if (receiver instanceof ResolveInfo) */ { + return ((ResolveInfo) receiver).activityInfo.name; + } + } + static int getReceiverPriority(@NonNull Object receiver) { if (receiver instanceof BroadcastFilter) { return ((BroadcastFilter) receiver).getPriority(); diff --git a/tests/Internal/src/com/android/internal/os/TimeoutRecordTest.java b/tests/Internal/src/com/android/internal/os/TimeoutRecordTest.java index 0f9663442740c..7419ee1230d3f 100644 --- a/tests/Internal/src/com/android/internal/os/TimeoutRecordTest.java +++ b/tests/Internal/src/com/android/internal/os/TimeoutRecordTest.java @@ -16,15 +16,15 @@ package com.android.internal.os; -import android.content.ComponentName; -import android.content.Intent; -import android.platform.test.annotations.Presubmit; - import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import android.content.ComponentName; +import android.content.Intent; +import android.platform.test.annotations.Presubmit; + import androidx.test.filters.SmallTest; import org.junit.Test; @@ -40,7 +40,7 @@ public class TimeoutRecordTest { @Test public void forBroadcastReceiver_returnsCorrectTimeoutRecord() { Intent intent = new Intent(Intent.ACTION_MAIN); - intent.setComponent(ComponentName.createRelative("com.example.app", "ExampleClass")); + intent.setComponent(new ComponentName("com.example.app", "com.example.app.ExampleClass")); TimeoutRecord record = TimeoutRecord.forBroadcastReceiver(intent); @@ -48,14 +48,28 @@ public class TimeoutRecordTest { assertEquals(record.mKind, TimeoutRecord.TimeoutKind.BROADCAST_RECEIVER); assertEquals(record.mReason, "Broadcast of Intent { act=android.intent.action.MAIN cmp=com.example" - + ".app/ExampleClass }"); + + ".app/.ExampleClass }"); + assertTrue(record.mEndTakenBeforeLocks); + } + + @Test + public void forBroadcastReceiver_withPackageAndClass_returnsCorrectTimeoutRecord() { + Intent intent = new Intent(Intent.ACTION_MAIN); + TimeoutRecord record = TimeoutRecord.forBroadcastReceiver(intent, + "com.example.app", "com.example.app.ExampleClass"); + + assertNotNull(record); + assertEquals(record.mKind, TimeoutRecord.TimeoutKind.BROADCAST_RECEIVER); + assertEquals(record.mReason, + "Broadcast of Intent { act=android.intent.action.MAIN cmp=com.example" + + ".app/.ExampleClass }"); assertTrue(record.mEndTakenBeforeLocks); } @Test public void forBroadcastReceiver_withTimeoutDurationMs_returnsCorrectTimeoutRecord() { Intent intent = new Intent(Intent.ACTION_MAIN); - intent.setComponent(ComponentName.createRelative("com.example.app", "ExampleClass")); + intent.setComponent(new ComponentName("com.example.app", "com.example.app.ExampleClass")); TimeoutRecord record = TimeoutRecord.forBroadcastReceiver(intent, 1000L); @@ -63,7 +77,7 @@ public class TimeoutRecordTest { assertEquals(record.mKind, TimeoutRecord.TimeoutKind.BROADCAST_RECEIVER); assertEquals(record.mReason, "Broadcast of Intent { act=android.intent.action.MAIN cmp=com.example" - + ".app/ExampleClass }, waited 1000ms"); + + ".app/.ExampleClass }, waited 1000ms"); assertTrue(record.mEndTakenBeforeLocks); }