Revert "More robust updating of "runnable" list."
This reverts commit 7529b207666881c747c2685e4279a4cea6e0a581.
Reason for revert: b/276599346
possibly slow down coverage build boot time
Change-Id: I87dbf23f4ed9dbadaa947f01edbb92a4a01e3dca
diff --git a/services/core/java/com/android/server/am/BroadcastProcessQueue.java b/services/core/java/com/android/server/am/BroadcastProcessQueue.java
index 23eaae2..5140f90 100644
--- a/services/core/java/com/android/server/am/BroadcastProcessQueue.java
+++ b/services/core/java/com/android/server/am/BroadcastProcessQueue.java
@@ -21,7 +21,6 @@
import static com.android.server.am.BroadcastRecord.isDeliveryStateTerminal;
import static com.android.server.am.BroadcastRecord.isReceiverEquals;
-import android.annotation.CheckResult;
import android.annotation.IntDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
@@ -342,12 +341,7 @@
* Predicates that choose to remove a broadcast <em>must</em> finish
* delivery of the matched broadcast, to ensure that situations like ordered
* broadcasts are handled consistently.
- *
- * @return if this operation may have changed internal state, indicating
- * that the caller is responsible for invoking
- * {@link BroadcastQueueModernImpl#updateRunnableList}
*/
- @CheckResult
public boolean forEachMatchingBroadcast(@NonNull BroadcastPredicate predicate,
@NonNull BroadcastConsumer consumer, boolean andRemove) {
boolean didSomething = false;
@@ -360,7 +354,6 @@
return didSomething;
}
- @CheckResult
private boolean forEachMatchingBroadcastInQueue(@NonNull ArrayDeque<SomeArgs> queue,
@NonNull BroadcastPredicate predicate, @NonNull BroadcastConsumer consumer,
boolean andRemove) {
@@ -377,10 +370,6 @@
args.recycle();
it.remove();
onBroadcastDequeued(record, recordIndex, recordWouldBeSkipped);
- } else {
- // Even if we're leaving broadcast in queue, it may have
- // been mutated in such a way to change our runnable time
- invalidateRunnableAt();
}
didSomething = true;
}
@@ -392,43 +381,30 @@
/**
* Update the actively running "warm" process for this process.
- *
- * @return if this operation may have changed internal state, indicating
- * that the caller is responsible for invoking
- * {@link BroadcastQueueModernImpl#updateRunnableList}
*/
- @CheckResult
- public boolean setProcessAndUidFrozen(@Nullable ProcessRecord app, boolean uidFrozen) {
+ public void setProcess(@Nullable ProcessRecord app) {
this.app = app;
+ if (app != null) {
+ setProcessInstrumented(app.getActiveInstrumentation() != null);
+ setProcessPersistent(app.isPersistent());
+ } else {
+ setProcessInstrumented(false);
+ setProcessPersistent(false);
+ }
// Since we may have just changed our PID, invalidate cached strings
mCachedToString = null;
mCachedToShortString = null;
-
- boolean didSomething = false;
- if (app != null) {
- didSomething |= setProcessInstrumented(app.getActiveInstrumentation() != null);
- didSomething |= setProcessPersistent(app.isPersistent());
- didSomething |= setUidFrozen(uidFrozen);
- } else {
- didSomething |= setProcessInstrumented(false);
- didSomething |= setProcessPersistent(false);
- didSomething |= setUidFrozen(uidFrozen);
- }
- return didSomething;
}
/**
* Update if this UID is in the "frozen" state, typically signaling that
* broadcast dispatch should be paused or delayed.
*/
- private boolean setUidFrozen(boolean frozen) {
+ public void setUidFrozen(boolean frozen) {
if (mUidFrozen != frozen) {
mUidFrozen = frozen;
invalidateRunnableAt();
- return true;
- } else {
- return false;
}
}
@@ -437,13 +413,10 @@
* signaling that broadcast dispatch should bypass all pauses or delays, to
* avoid holding up test suites.
*/
- private boolean setProcessInstrumented(boolean instrumented) {
+ public void setProcessInstrumented(boolean instrumented) {
if (mProcessInstrumented != instrumented) {
mProcessInstrumented = instrumented;
invalidateRunnableAt();
- return true;
- } else {
- return false;
}
}
@@ -451,13 +424,10 @@
* Update if this process is in the "persistent" state, which signals broadcast dispatch should
* bypass all pauses or delays to prevent the system from becoming out of sync with itself.
*/
- private boolean setProcessPersistent(boolean persistent) {
+ public void setProcessPersistent(boolean persistent) {
if (mProcessPersistent != persistent) {
mProcessPersistent = persistent;
invalidateRunnableAt();
- return true;
- } else {
- return false;
}
}
@@ -677,20 +647,8 @@
return mActive != null;
}
- /**
- * @return if this operation may have changed internal state, indicating
- * that the caller is responsible for invoking
- * {@link BroadcastQueueModernImpl#updateRunnableList}
- */
- @CheckResult
- boolean forceDelayBroadcastDelivery(long delayedDurationMs) {
- if (mForcedDelayedDurationMs != delayedDurationMs) {
- mForcedDelayedDurationMs = delayedDurationMs;
- invalidateRunnableAt();
- return true;
- } else {
- return false;
- }
+ void forceDelayBroadcastDelivery(long delayedDurationMs) {
+ mForcedDelayedDurationMs = delayedDurationMs;
}
/**
@@ -762,21 +720,10 @@
* broadcasts would be prioritized for dispatching, even if there are urgent broadcasts
* waiting. This is typically used in case there are callers waiting for "barrier" to be
* reached.
- *
- * @return if this operation may have changed internal state, indicating
- * that the caller is responsible for invoking
- * {@link BroadcastQueueModernImpl#updateRunnableList}
*/
- @CheckResult
@VisibleForTesting
- boolean setPrioritizeEarliest(boolean prioritizeEarliest) {
- if (mPrioritizeEarliest != prioritizeEarliest) {
- mPrioritizeEarliest = prioritizeEarliest;
- invalidateRunnableAt();
- return true;
- } else {
- return false;
- }
+ void setPrioritizeEarliest(boolean prioritizeEarliest) {
+ mPrioritizeEarliest = prioritizeEarliest;
}
/**
diff --git a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java
index e35f822..e574b49 100644
--- a/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java
+++ b/services/core/java/com/android/server/am/BroadcastQueueModernImpl.java
@@ -360,6 +360,8 @@
// If app isn't running, and there's nothing in the queue, clean up
if (queue.isEmpty() && !queue.isActive() && !queue.isProcessWarm()) {
removeProcessQueue(queue.processName, queue.uid);
+ } else {
+ updateQueueDeferred(queue);
}
}
@@ -494,7 +496,8 @@
// relevant per-process queue
final BroadcastProcessQueue queue = getProcessQueue(app);
if (queue != null) {
- setQueueProcess(queue, app);
+ queue.setProcess(app);
+ queue.setUidFrozen(mUidFrozen.get(queue.uid, false));
}
boolean didSomething = false;
@@ -535,7 +538,8 @@
// relevant per-process queue
final BroadcastProcessQueue queue = getProcessQueue(app);
if (queue != null) {
- setQueueProcess(queue, null);
+ queue.setProcess(null);
+ queue.setUidFrozen(mUidFrozen.get(queue.uid, false));
}
if ((mRunningColdStart != null) && (mRunningColdStart == queue)) {
@@ -562,7 +566,6 @@
return (r.receivers.get(i) instanceof BroadcastFilter);
}, mBroadcastConsumerSkip, true);
if (didSomething || queue.isEmpty()) {
- updateQueueDeferred(queue);
updateRunnableList(queue);
enqueueUpdateRunningList();
}
@@ -628,7 +631,6 @@
setDeliveryState(queue, null, r, i, receiver, BroadcastRecord.DELIVERY_DEFERRED,
"deferred at enqueue time");
}
- updateQueueDeferred(queue);
updateRunnableList(queue);
enqueueUpdateRunningList();
}
@@ -1077,7 +1079,6 @@
final int queueIndex = getRunningIndexOf(queue);
mRunning[queueIndex] = null;
- updateQueueDeferred(queue);
updateRunnableList(queue);
enqueueUpdateRunningList();
@@ -1160,7 +1161,6 @@
getReceiverUid(otherReceiver));
if (otherQueue != null) {
otherQueue.invalidateRunnableAt();
- updateQueueDeferred(otherQueue);
updateRunnableList(otherQueue);
}
}
@@ -1291,7 +1291,6 @@
if (queuePredicate.test(leaf)) {
if (leaf.forEachMatchingBroadcast(broadcastPredicate,
broadcastConsumer, andRemove)) {
- updateQueueDeferred(leaf);
updateRunnableList(leaf);
didSomething = true;
}
@@ -1305,40 +1304,29 @@
return didSomething;
}
- private boolean forEachMatchingQueue(
+ private void forEachMatchingQueue(
@NonNull Predicate<BroadcastProcessQueue> queuePredicate,
@NonNull Consumer<BroadcastProcessQueue> queueConsumer) {
- boolean didSomething = false;
for (int i = mProcessQueues.size() - 1; i >= 0; i--) {
BroadcastProcessQueue leaf = mProcessQueues.valueAt(i);
while (leaf != null) {
if (queuePredicate.test(leaf)) {
queueConsumer.accept(leaf);
- updateQueueDeferred(leaf);
updateRunnableList(leaf);
- didSomething = true;
}
leaf = leaf.processNameNext;
}
}
- if (didSomething) {
- enqueueUpdateRunningList();
- }
- return didSomething;
}
- @SuppressWarnings("CheckResult")
- private void updateQueueDeferred(@NonNull BroadcastProcessQueue leaf) {
+ private void updateQueueDeferred(
+ @NonNull BroadcastProcessQueue leaf) {
if (leaf.isDeferredUntilActive()) {
- // We ignore the returned value here since callers are invoking us
- // just before updateRunnableList()
leaf.forEachMatchingBroadcast((r, i) -> {
return r.deferUntilActive && (r.getDeliveryState(i)
== BroadcastRecord.DELIVERY_PENDING);
}, mBroadcastConsumerDefer, false);
} else if (leaf.hasDeferredBroadcasts()) {
- // We ignore the returned value here since callers are invoking us
- // just before updateRunnableList()
leaf.forEachMatchingBroadcast((r, i) -> {
return r.deferUntilActive && (r.getDeliveryState(i)
== BroadcastRecord.DELIVERY_DEFERRED);
@@ -1368,7 +1356,10 @@
while (leaf != null) {
// Update internal state by refreshing values previously
// read from any known running process
- setQueueProcess(leaf, leaf.app);
+ leaf.setProcess(leaf.app);
+ leaf.setUidFrozen(frozen);
+ updateQueueDeferred(leaf);
+ updateRunnableList(leaf);
leaf = leaf.processNameNext;
}
enqueueUpdateRunningList();
@@ -1529,20 +1520,8 @@
private void updateWarmProcess(@NonNull BroadcastProcessQueue queue) {
if (!queue.isProcessWarm()) {
- setQueueProcess(queue, mService.getProcessRecordLocked(queue.processName, queue.uid));
- }
- }
-
- /**
- * Update the {@link ProcessRecord} associated with the given
- * {@link BroadcastProcessQueue}. Also updates any runnable status that
- * might have changed as a side-effect.
- */
- private void setQueueProcess(@NonNull BroadcastProcessQueue queue,
- @Nullable ProcessRecord app) {
- if (queue.setProcessAndUidFrozen(app, mUidFrozen.get(queue.uid, false))) {
- updateQueueDeferred(queue);
- updateRunnableList(queue);
+ queue.setProcess(mService.getProcessRecordLocked(queue.processName, queue.uid));
+ queue.setUidFrozen(mUidFrozen.get(queue.uid, false));
}
}
@@ -1736,7 +1715,8 @@
}
BroadcastProcessQueue created = new BroadcastProcessQueue(mConstants, processName, uid);
- setQueueProcess(created, mService.getProcessRecordLocked(processName, uid));
+ created.setProcess(mService.getProcessRecordLocked(processName, uid));
+ created.setUidFrozen(mUidFrozen.get(uid, false));
if (leaf == null) {
mProcessQueues.put(uid, created);
diff --git a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
index 74b68e7..c68db4f 100644
--- a/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/am/BroadcastQueueModernImplTest.java
@@ -371,9 +371,9 @@
List.of(makeMockRegisteredReceiver()), false);
queue.enqueueOrReplaceBroadcast(airplaneRecord, 0, false);
- queue.setProcessAndUidFrozen(null, false);
+ queue.setUidFrozen(false);
final long notCachedRunnableAt = queue.getRunnableAt();
- queue.setProcessAndUidFrozen(null, true);
+ queue.setUidFrozen(true);
final long cachedRunnableAt = queue.getRunnableAt();
assertThat(cachedRunnableAt).isGreaterThan(notCachedRunnableAt);
assertFalse(queue.isRunnable());
@@ -398,9 +398,9 @@
List.of(makeMockRegisteredReceiver()), false);
queue.enqueueOrReplaceBroadcast(airplaneRecord, 0, false);
- queue.setProcessAndUidFrozen(null, false);
+ queue.setUidFrozen(false);
final long notCachedRunnableAt = queue.getRunnableAt();
- queue.setProcessAndUidFrozen(null, true);
+ queue.setUidFrozen(true);
final long cachedRunnableAt = queue.getRunnableAt();
assertThat(cachedRunnableAt).isGreaterThan(notCachedRunnableAt);
assertTrue(queue.isRunnable());
@@ -430,13 +430,13 @@
// verify that:
// (a) the queue is immediately runnable by existence of a fg-priority broadcast
// (b) the next one up is the fg-priority broadcast despite its later enqueue time
- queue.setProcessAndUidFrozen(null, false);
+ queue.setUidFrozen(false);
assertTrue(queue.isRunnable());
assertThat(queue.getRunnableAt()).isAtMost(airplaneRecord.enqueueClockTime);
assertEquals(ProcessList.SCHED_GROUP_DEFAULT, queue.getPreferredSchedulingGroupLocked());
assertEquals(queue.peekNextBroadcastRecord(), airplaneRecord);
- queue.setProcessAndUidFrozen(null, true);
+ queue.setUidFrozen(true);
assertTrue(queue.isRunnable());
assertThat(queue.getRunnableAt()).isAtMost(airplaneRecord.enqueueClockTime);
assertEquals(ProcessList.SCHED_GROUP_DEFAULT, queue.getPreferredSchedulingGroupLocked());
@@ -499,7 +499,7 @@
private void doRunnableAt_Frozen(BroadcastRecord testRecord, int testRunnableAtReason) {
final BroadcastProcessQueue queue = new BroadcastProcessQueue(mConstants,
PACKAGE_GREEN, getUidForPackage(PACKAGE_GREEN));
- queue.setProcessAndUidFrozen(null, true);
+ queue.setUidFrozen(true);
final BroadcastRecord lazyRecord = makeBroadcastRecord(
new Intent(Intent.ACTION_AIRPLANE_MODE_CHANGED),