Log BROADCAST_DELIVERY_EVENT_REPORTED only when delivery is attempted.

Fixes: 269211533
Test: services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
Test: atest ./services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java
Change-Id: I40ca9f45eac4fcf82c1d50de759b647539c9ecd8
Merged-In: I40ca9f45eac4fcf82c1d50de759b647539c9ecd8
(cherry picked from commit c11eeeb1c9)

services/core/java/com/android/server/am/BroadcastQueueModernImpl.java

@@ -1620,6 +1620,18 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
      */
     private void notifyFinishReceiver(@Nullable BroadcastProcessQueue queue,
             @NonNull BroadcastRecord r, int index, @NonNull Object receiver) {
+        if (r.wasDeliveryAttempted(index)) {
+            logBroadcastDeliveryEventReported(queue, r, index, receiver);
+        }
+
+        final boolean recordFinished = (r.terminalCount == r.receivers.size());
+        if (recordFinished) {
+            notifyFinishBroadcast(r);
+        }
+    }
+
+    private void logBroadcastDeliveryEventReported(@Nullable BroadcastProcessQueue queue,
+            @NonNull BroadcastRecord r, int index, @NonNull Object receiver) {
         // Report statistics for each individual receiver
         final int uid = getReceiverUid(receiver);
         final int senderUid = (r.callingUid == -1) ? Process.SYSTEM_UID : r.callingUid;
@@ -1647,11 +1659,6 @@ class BroadcastQueueModernImpl extends BroadcastQueue {
             FrameworkStatsLog.write(BROADCAST_DELIVERY_EVENT_REPORTED, uid, senderUid, actionName,
                     receiverType, type, dispatchDelay, receiveDelay, finishDelay, packageState);
         }
-
-        final boolean recordFinished = (r.terminalCount == r.receivers.size());
-        if (recordFinished) {
-            notifyFinishBroadcast(r);
-        }
     }
 
     private void notifyFinishBroadcast(@NonNull BroadcastRecord r) {

services/core/java/com/android/server/am/BroadcastRecord.java

@@ -632,6 +632,18 @@ final class BroadcastRecord extends Binder {
         return delivery[index];
     }
 
+    boolean wasDeliveryAttempted(int index) {
+        final int deliveryState = getDeliveryState(index);
+        switch (deliveryState) {
+            case DELIVERY_DELIVERED:
+            case DELIVERY_TIMEOUT:
+            case DELIVERY_FAILURE:
+                return true;
+            default:
+                return false;
+        }
+    }
+
     void copyEnqueueTimeFrom(@NonNull BroadcastRecord replacedBroadcast) {
         originalEnqueueClockTime = enqueueClockTime;
         enqueueTime = replacedBroadcast.enqueueTime;

services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java

@@ -16,6 +16,10 @@
 
 package com.android.server.am;
 
+import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify;
+import static com.android.internal.util.FrameworkStatsLog.BROADCAST_DELIVERY_EVENT_REPORTED;
+import static com.android.internal.util.FrameworkStatsLog.BROADCAST_DELIVERY_EVENT_REPORTED__PROC_START_TYPE__PROCESS_START_TYPE_COLD;
+import static com.android.internal.util.FrameworkStatsLog.BROADCAST_DELIVERY_EVENT_REPORTED__RECEIVER_TYPE__MANIFEST;
 import static com.android.server.am.BroadcastProcessQueue.REASON_CONTAINS_ALARM;
 import static com.android.server.am.BroadcastProcessQueue.REASON_CONTAINS_FOREGROUND;
 import static com.android.server.am.BroadcastProcessQueue.REASON_CONTAINS_INTERACTIVE;
@@ -44,8 +48,12 @@ import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 
 import android.annotation.NonNull;
 import android.app.Activity;
@@ -70,6 +78,11 @@ import android.util.Pair;
 
 import androidx.test.filters.SmallTest;
 
+import com.android.dx.mockito.inline.extended.StaticMockitoSessionBuilder;
+import com.android.internal.util.FrameworkStatsLog;
+import com.android.server.ExtendedMockitoTestCase;
+import com.android.server.am.BroadcastQueueTest.SyncBarrier;
+
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -86,7 +99,7 @@ import java.util.List;
 
 @SmallTest
 @RunWith(MockitoJUnitRunner.class)
-public class BroadcastQueueModernImplTest {
+public class BroadcastQueueModernImplTest extends ExtendedMockitoTestCase {
     private static final int TEST_UID = android.os.Process.FIRST_APPLICATION_UID;
     private static final int TEST_UID2 = android.os.Process.FIRST_APPLICATION_UID + 1;
 
@@ -105,6 +118,11 @@ public class BroadcastQueueModernImplTest {
 
     BroadcastProcessQueue mHead;
 
+    @Override
+    protected void initializeSession(StaticMockitoSessionBuilder builder) {
+        builder.spyStatic(FrameworkStatsLog.class);
+    }
+
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
@@ -1081,6 +1099,28 @@ public class BroadcastQueueModernImplTest {
         }
     }
 
+    @Test
+    public void testBroadcastDeliveryEventReported() throws Exception {
+        final Intent timeTick = new Intent(Intent.ACTION_TIME_TICK);
+        final BroadcastOptions optionsTimeTick = BroadcastOptions.makeBasic();
+        optionsTimeTick.setDeliveryGroupPolicy(BroadcastOptions.DELIVERY_GROUP_POLICY_MOST_RECENT);
+
+        // Halt all processing so that we get a consistent view
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
+            mImpl.enqueueBroadcastLocked(makeBroadcastRecord(timeTick, optionsTimeTick));
+            mImpl.enqueueBroadcastLocked(makeBroadcastRecord(timeTick, optionsTimeTick));
+        }
+        mImpl.waitForIdle(null);
+
+        // Verify that there is only one delivery event reported since one of the broadcasts
+        // should have been skipped.
+        verify(() -> FrameworkStatsLog.write(eq(BROADCAST_DELIVERY_EVENT_REPORTED),
+                eq(getUidForPackage(PACKAGE_GREEN)), anyInt(), eq(Intent.ACTION_TIME_TICK),
+                eq(BROADCAST_DELIVERY_EVENT_REPORTED__RECEIVER_TYPE__MANIFEST),
+                eq(BROADCAST_DELIVERY_EVENT_REPORTED__PROC_START_TYPE__PROCESS_START_TYPE_COLD),
+                anyLong(), anyLong(), anyLong(), anyInt()), times(1));
+    }
+
     private Intent createPackageChangedIntent(int uid, List<String> componentNameList) {
         final Intent packageChangedIntent = new Intent(Intent.ACTION_PACKAGE_CHANGED);
         packageChangedIntent.putExtra(Intent.EXTRA_UID, uid);

services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueTest.java

@@ -346,16 +346,18 @@ public class BroadcastQueueTest {
      * Helper that leverages try-with-resources to pause dispatch of
      * {@link #mHandlerThread} until released.
      */
-    private class SyncBarrier implements AutoCloseable {
+    static class SyncBarrier implements AutoCloseable {
         private final int mToken;
+        private HandlerThread mThread;
 
-        public SyncBarrier() {
+        SyncBarrier(HandlerThread thread) {
-            mToken = mHandlerThread.getLooper().getQueue().postSyncBarrier();
+            mThread = thread;
+            mToken = mThread.getLooper().getQueue().postSyncBarrier();
         }
 
         @Override
         public void close() throws Exception {
-            mHandlerThread.getLooper().getQueue().removeSyncBarrier(mToken);
+            mThread.getLooper().getQueue().removeSyncBarrier(mToken);
         }
     }
 
@@ -1120,7 +1122,7 @@ public class BroadcastQueueTest {
         final ProcessRecord receiverApp = makeActiveProcessRecord(PACKAGE_GREEN);
 
         final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, new ArrayList<>(
                     List.of(makeRegisteredReceiver(receiverApp),
                             makeManifestReceiver(PACKAGE_GREEN, CLASS_RED),
@@ -1164,7 +1166,7 @@ public class BroadcastQueueTest {
 
         final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
         final Intent timeZone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, USER_GUEST, new ArrayList<>(
                     List.of(makeRegisteredReceiver(callerApp),
                             makeManifestReceiver(PACKAGE_GREEN, CLASS_RED, USER_GUEST),
@@ -1204,7 +1206,7 @@ public class BroadcastQueueTest {
         final ProcessRecord oldApp = makeActiveProcessRecord(PACKAGE_GREEN);
 
         final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             enqueueBroadcast(makeBroadcastRecord(airplane, callerApp, new ArrayList<>(
                     List.of(makeRegisteredReceiver(oldApp),
                             makeManifestReceiver(PACKAGE_GREEN, CLASS_GREEN)))));
@@ -1585,7 +1587,7 @@ public class BroadcastQueueTest {
         final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);
         final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
         airplane.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             enqueueBroadcast(makeBroadcastRecord(timezone, callerApp,
                     List.of(makeRegisteredReceiver(receiverBlueApp, 10),
                             makeRegisteredReceiver(receiverGreenApp, 10),
@@ -1638,7 +1640,7 @@ public class BroadcastQueueTest {
         final IIntentReceiver resultToFirst = mock(IIntentReceiver.class);
         final IIntentReceiver resultToSecond = mock(IIntentReceiver.class);
 
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             enqueueBroadcast(makeOrderedBroadcastRecord(timezoneFirst, callerApp,
                     List.of(makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE),
                             makeManifestReceiver(PACKAGE_BLUE, CLASS_GREEN)),
@@ -1729,7 +1731,7 @@ public class BroadcastQueueTest {
         timeTickFirst.putExtra(Intent.EXTRA_INDEX, "third");
         timeTickThird.addFlags(Intent.FLAG_RECEIVER_REPLACE_PENDING);
 
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             enqueueBroadcast(makeBroadcastRecord(timeTickFirst, callerApp,
                     List.of(makeManifestReceiver(PACKAGE_BLUE, CLASS_BLUE))));
             enqueueBroadcast(makeBroadcastRecord(timeTickSecond, callerApp,
@@ -1771,7 +1773,7 @@ public class BroadcastQueueTest {
         assertTrue(mQueue.isIdleLocked());
         assertTrue(mQueue.isBeyondBarrierLocked(beforeFirst));
 
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             final Intent timezone = new Intent(Intent.ACTION_TIMEZONE_CHANGED);
             enqueueBroadcast(makeBroadcastRecord(timezone, callerApp,
                     List.of(makeRegisteredReceiver(receiverApp))));
@@ -1865,7 +1867,7 @@ public class BroadcastQueueTest {
         final ProcessRecord receiverBlueApp = makeActiveProcessRecord(PACKAGE_BLUE);
 
         final Intent airplane = new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED);
-        try (SyncBarrier b = new SyncBarrier()) {
+        try (SyncBarrier b = new SyncBarrier(mHandlerThread)) {
             final Object greenReceiver = makeRegisteredReceiver(receiverGreenApp);
             final Object blueReceiver = makeRegisteredReceiver(receiverBlueApp);
             final Object yellowReceiver = makeManifestReceiver(PACKAGE_YELLOW, CLASS_YELLOW);