From bc7b8b4b1388f7b084bdf58312adc75727805261 Mon Sep 17 00:00:00 2001 From: Thomas Stuart Date: Tue, 30 Aug 2022 16:50:18 +0000 Subject: [PATCH] Revert "Allow for replaceable clock within Looper" This reverts commit f861619bb016e97b7d097a4e72ee2351b0b2a546. Reason for revert: causes multiple test breakages Change-Id: I7a46e7a0d55ea726abb1150dff5c9fdda6539637 --- core/java/android/os/Handler.java | 82 ++++++------------- core/java/android/os/Looper.java | 26 +----- core/java/android/os/MessageQueue.java | 66 +++++++++------ .../java/android/os/test/TestLooper.java | 15 ++-- 4 files changed, 81 insertions(+), 108 deletions(-) diff --git a/core/java/android/os/Handler.java b/core/java/android/os/Handler.java index aefa5f827f4ad..ceaf337b2122d 100644 --- a/core/java/android/os/Handler.java +++ b/core/java/android/os/Handler.java @@ -182,7 +182,7 @@ public class Handler { * * Asynchronous messages represent interrupts or events that do not require global ordering * with respect to synchronous messages. Asynchronous messages are not subject to - * the synchronization barriers introduced by {@link MessageQueue#postSyncBarrier()}. + * the synchronization barriers introduced by {@link MessageQueue#enqueueSyncBarrier(long)}. * * @param async If true, the handler calls {@link Message#setAsynchronous(boolean)} for * each {@link Message} that is sent to it or {@link Runnable} that is posted to it. @@ -203,7 +203,7 @@ public class Handler { * * Asynchronous messages represent interrupts or events that do not require global ordering * with respect to synchronous messages. Asynchronous messages are not subject to - * the synchronization barriers introduced by {@link MessageQueue#postSyncBarrier()}. + * the synchronization barriers introduced by {@link MessageQueue#enqueueSyncBarrier(long)}. * * @param callback The callback interface in which to handle messages, or null. * @param async If true, the handler calls {@link Message#setAsynchronous(boolean)} for @@ -212,17 +212,25 @@ public class Handler { * @hide */ public Handler(@Nullable Callback callback, boolean async) { - this(getThreadLooper(), callback, async); - } - - private static Looper getThreadLooper() { - final Looper looper = Looper.myLooper(); - if (looper == null) { - throw new RuntimeException( - "Can't create handler inside thread " + Thread.currentThread() - + " that has not called Looper.prepare()"); + if (FIND_POTENTIAL_LEAKS) { + final Class klass = getClass(); + if ((klass.isAnonymousClass() || klass.isMemberClass() || klass.isLocalClass()) && + (klass.getModifiers() & Modifier.STATIC) == 0) { + Log.w(TAG, "The following Handler class should be static or leaks might occur: " + + klass.getCanonicalName()); + } } - return looper; + + mLooper = Looper.myLooper(); + if (mLooper == null) { + throw new RuntimeException( + "Can't create handler inside thread " + Thread.currentThread() + + " that has not called Looper.prepare()"); + } + mQueue = mLooper.mQueue; + mCallback = callback; + mAsynchronous = async; + mIsShared = false; } /** @@ -249,44 +257,14 @@ public class Handler { this(looper, callback, async, /* shared= */ false); } - /** - * Use the provided {@link Looper} instead of the default one and take a callback - * interface in which to handle messages. Also set whether the handler - * should be asynchronous. - * - * Handlers are synchronous by default unless this constructor is used to make - * one that is strictly asynchronous. - * - * Asynchronous messages represent interrupts or events that do not require global ordering - * with respect to synchronous messages. Asynchronous messages are not subject to - * the synchronization barriers introduced by conditions such as display vsync. - * - * @param looper The looper, must not be null. - * @param callback The callback interface in which to handle messages, or null. - * @param async If true, the handler calls {@link Message#setAsynchronous(boolean)} for - * each {@link Message} that is sent to it or {@link Runnable} that is posted to - * it. - * @param shared Whether this Handler might be used by more than one client. A shared Handler - * applies some extra policy, such as disallowing the removal of all messages, - * in order to avoid one client affecting another's messages. - * @hide - */ + /** @hide */ public Handler(@NonNull Looper looper, @Nullable Callback callback, boolean async, boolean shared) { - if (FIND_POTENTIAL_LEAKS) { - final Class klass = getClass(); - if ((klass.isAnonymousClass() || klass.isMemberClass() || klass.isLocalClass()) - && (klass.getModifiers() & Modifier.STATIC) == 0) { - Log.w(TAG, "The following Handler class should be static or leaks might occur: " - + klass.getCanonicalName()); - } - } mLooper = looper; mQueue = looper.mQueue; mCallback = callback; mAsynchronous = async; mIsShared = shared; - mClock = looper.getClock(); } /** @@ -724,14 +702,7 @@ public class Handler { if (delayMillis < 0) { delayMillis = 0; } - // mClock should theoretically never be null, but some tests create a mock handler that - // instantiates an instance where all members are null. Ideally we'd fix these tests to - // not rely on this but there are quite a lot at this point, so it's easier to just keep - // the existing behavior. - if (mClock == null) { - return false; - } - return sendMessageAtTime(msg, mClock.uptimeMillis() + delayMillis); + return sendMessageAtTime(msg, SystemClock.uptimeMillis() + delayMillis); } /** @@ -924,7 +895,7 @@ public class Handler { } public final void dump(@NonNull Printer pw, @NonNull String prefix) { - pw.println(prefix + this + " @ " + mClock.uptimeMillis()); + pw.println(prefix + this + " @ " + SystemClock.uptimeMillis()); if (mLooper == null) { pw.println(prefix + "looper uninitialized"); } else { @@ -936,7 +907,7 @@ public class Handler { * @hide */ public final void dumpMine(@NonNull Printer pw, @NonNull String prefix) { - pw.println(prefix + this + " @ " + mClock.uptimeMillis()); + pw.println(prefix + this + " @ " + SystemClock.uptimeMillis()); if (mLooper == null) { pw.println(prefix + "looper uninitialized"); } else { @@ -993,7 +964,6 @@ public class Handler { @UnsupportedAppUsage final Callback mCallback; final boolean mAsynchronous; - final MessageQueue.Clock mClock; @UnsupportedAppUsage IMessenger mMessenger; @@ -1027,9 +997,9 @@ public class Handler { synchronized (this) { if (timeout > 0) { - final long expirationTime = handler.mClock.uptimeMillis() + timeout; + final long expirationTime = SystemClock.uptimeMillis() + timeout; while (!mDone) { - long delay = expirationTime - handler.mClock.uptimeMillis(); + long delay = expirationTime - SystemClock.uptimeMillis(); if (delay <= 0) { return false; // timeout } diff --git a/core/java/android/os/Looper.java b/core/java/android/os/Looper.java index f0fda15f6e90f..a529ac6569bd5 100644 --- a/core/java/android/os/Looper.java +++ b/core/java/android/os/Looper.java @@ -24,8 +24,6 @@ import android.util.Printer; import android.util.Slog; import android.util.proto.ProtoOutputStream; -import java.util.Objects; - /** * Class used to run a message loop for a thread. Threads by default do * not have a message loop associated with them; to create one, call @@ -71,7 +69,7 @@ public final class Looper { // sThreadLocal.get() will return null unless you've called prepare(). @UnsupportedAppUsage - static final ThreadLocal sThreadLocal = new ThreadLocal<>(); + static final ThreadLocal sThreadLocal = new ThreadLocal(); @UnsupportedAppUsage private static Looper sMainLooper; // guarded by Looper.class private static Observer sObserver; @@ -79,7 +77,6 @@ public final class Looper { @UnsupportedAppUsage final MessageQueue mQueue; final Thread mThread; - final MessageQueue.Clock mClock; private boolean mInLoop; @UnsupportedAppUsage @@ -194,7 +191,7 @@ public final class Looper { Trace.traceBegin(traceTag, msg.target.getTraceName(msg)); } - final long dispatchStart = needStartTime ? me.mClock.uptimeMillis() : 0; + final long dispatchStart = needStartTime ? SystemClock.uptimeMillis() : 0; final long dispatchEnd; Object token = null; if (observer != null) { @@ -206,7 +203,7 @@ public final class Looper { if (observer != null) { observer.messageDispatched(token, msg); } - dispatchEnd = needEndTime ? me.mClock.uptimeMillis() : 0; + dispatchEnd = needEndTime ? SystemClock.uptimeMillis() : 0; } catch (Exception exception) { if (observer != null) { observer.dispatchingThrewException(token, msg, exception); @@ -328,13 +325,8 @@ public final class Looper { } private Looper(boolean quitAllowed) { - this(quitAllowed, SystemClock::uptimeMillis); - } - - private Looper(boolean quitAllowed, @NonNull MessageQueue.Clock clock) { - mQueue = new MessageQueue(quitAllowed, Objects.requireNonNull(clock)); + mQueue = new MessageQueue(quitAllowed); mThread = Thread.currentThread(); - mClock = clock; } /** @@ -426,16 +418,6 @@ public final class Looper { return mQueue; } - /** - * Gets the looper's clock. - * - * @return The looper's clock - * @hide - */ - public @NonNull MessageQueue.Clock getClock() { - return mClock; - } - /** * Dumps the state of the looper for debugging purposes. * diff --git a/core/java/android/os/MessageQueue.java b/core/java/android/os/MessageQueue.java index a8ed5ba994899..87c4f331e93f8 100644 --- a/core/java/android/os/MessageQueue.java +++ b/core/java/android/os/MessageQueue.java @@ -19,7 +19,6 @@ package android.os; import android.annotation.IntDef; import android.annotation.NonNull; import android.annotation.TestApi; -import android.annotation.UptimeMillisLong; import android.compat.annotation.UnsupportedAppUsage; import android.util.Log; import android.util.Printer; @@ -30,7 +29,6 @@ import java.io.FileDescriptor; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.util.ArrayList; -import java.util.Objects; /** * Low-level class holding the list of messages to be dispatched by a @@ -56,7 +54,6 @@ public final class MessageQueue { Message mMessages; @UnsupportedAppUsage private final ArrayList mIdleHandlers = new ArrayList(); - private final Clock mClock; private SparseArray mFileDescriptorRecords; private IdleHandler[] mPendingIdleHandlers; private boolean mQuitting; @@ -77,10 +74,9 @@ public final class MessageQueue { private native static boolean nativeIsPolling(long ptr); private native static void nativeSetFileDescriptorEvents(long ptr, int fd, int events); - MessageQueue(boolean quitAllowed, @NonNull Clock clock) { + MessageQueue(boolean quitAllowed) { mQuitAllowed = quitAllowed; mPtr = nativeInit(); - mClock = Objects.requireNonNull(clock); } @Override @@ -110,7 +106,7 @@ public final class MessageQueue { */ public boolean isIdle() { synchronized (this) { - final long now = mClock.uptimeMillis(); + final long now = SystemClock.uptimeMillis(); return mMessages == null || now < mMessages.when; } } @@ -340,7 +336,7 @@ public final class MessageQueue { synchronized (this) { // Try to retrieve the next message. Return if found. - final long now = mClock.uptimeMillis(); + final long now = SystemClock.uptimeMillis(); Message prevMsg = null; Message msg = mMessages; if (msg != null && msg.target == null) { @@ -474,7 +470,7 @@ public final class MessageQueue { @UnsupportedAppUsage @TestApi public int postSyncBarrier() { - return postSyncBarrier(mClock.uptimeMillis()); + return postSyncBarrier(SystemClock.uptimeMillis()); } private int postSyncBarrier(long when) { @@ -776,6 +772,41 @@ public final class MessageQueue { } } + void removeEqualMessages(Handler h, Runnable r, Object object) { + if (h == null || r == null) { + return; + } + + synchronized (this) { + Message p = mMessages; + + // Remove all messages at front. + while (p != null && p.target == h && p.callback == r + && (object == null || object.equals(p.obj))) { + Message n = p.next; + mMessages = n; + p.recycleUnchecked(); + p = n; + } + + // Remove all messages after front. + while (p != null) { + Message n = p.next; + if (n != null) { + if (n.target == h && n.callback == r + && (object == null || object.equals(n.obj))) { + Message nn = n.next; + n.recycleUnchecked(); + p.next = nn; + continue; + } + } + p = n; + } + } + } + + void removeCallbacksAndMessages(Handler h, Object object) { if (h == null) { return; @@ -853,7 +884,7 @@ public final class MessageQueue { } private void removeAllFutureMessagesLocked() { - final long now = mClock.uptimeMillis(); + final long now = SystemClock.uptimeMillis(); Message p = mMessages; if (p != null) { if (p.when > now) { @@ -882,7 +913,7 @@ public final class MessageQueue { void dump(Printer pw, String prefix, Handler h) { synchronized (this) { - long now = mClock.uptimeMillis(); + long now = SystemClock.uptimeMillis(); int n = 0; for (Message msg = mMessages; msg != null; msg = msg.next) { if (h == null || h == msg.target) { @@ -911,7 +942,7 @@ public final class MessageQueue { * Callback interface for discovering when a thread is going to block * waiting for more messages. */ - public interface IdleHandler { + public static interface IdleHandler { /** * Called when the message queue has run out of messages and will now * wait for more. Return true to keep your idle handler active, false @@ -1010,17 +1041,4 @@ public final class MessageQueue { mListener = listener; } } - - /** - * Time supplier for MessageQueue and the things that interact with it (e.g. {@link Looper}). - * - * Intentionally replaceable for testing. - * - * @hide - */ - public interface Clock { - /** @see SystemClock#uptimeMillis */ - @UptimeMillisLong - long uptimeMillis(); - } } diff --git a/tests/utils/testutils/java/android/os/test/TestLooper.java b/tests/utils/testutils/java/android/os/test/TestLooper.java index 9ad6d53a32462..a826646f69f3f 100644 --- a/tests/utils/testutils/java/android/os/test/TestLooper.java +++ b/tests/utils/testutils/java/android/os/test/TestLooper.java @@ -48,14 +48,13 @@ public class TestLooper { private static final Method MESSAGE_MARK_IN_USE_METHOD; private static final String TAG = "TestLooper"; - private final MessageQueue.Clock mClock; + private final Clock mClock; private AutoDispatchThread mAutoDispatchThread; static { try { - LOOPER_CONSTRUCTOR = Looper.class.getDeclaredConstructor(Boolean.TYPE, - MessageQueue.Clock.class); + LOOPER_CONSTRUCTOR = Looper.class.getDeclaredConstructor(Boolean.TYPE); LOOPER_CONSTRUCTOR.setAccessible(true); THREAD_LOCAL_LOOPER_FIELD = Looper.class.getDeclaredField("sThreadLocal"); THREAD_LOCAL_LOOPER_FIELD.setAccessible(true); @@ -84,15 +83,15 @@ public class TestLooper { * thread. * * Messages are dispatched when their {@link Message#when} is before or at {@link - * MessageQueue.Clock#uptimeMillis()}. + * Clock#uptimeMillis()}. * Use a custom clock with care. When using an offsettable clock like {@link * com.android.server.testutils.OffsettableClock} be sure not to double offset messages by * offsetting the clock and calling {@link #moveTimeForward(long)}. Instead, offset the clock * and call {@link #dispatchAll()}. */ - public TestLooper(MessageQueue.Clock clock) { + public TestLooper(Clock clock) { try { - mLooper = LOOPER_CONSTRUCTOR.newInstance(false, clock); + mLooper = LOOPER_CONSTRUCTOR.newInstance(false); ThreadLocal threadLocalLooper = (ThreadLocal) THREAD_LOCAL_LOOPER_FIELD .get(null); @@ -225,6 +224,10 @@ public class TestLooper { return count; } + public interface Clock { + long uptimeMillis(); + } + /** * Thread used to dispatch messages when the main thread is blocked waiting for a response. */