From f3d4acf240dc2679f585618516eaee33778e4d11 Mon Sep 17 00:00:00 2001 From: Johannes Gallmann Date: Fri, 24 May 2024 15:06:58 +0000 Subject: Refactor IME callback registration This refactoring aims to slightly simplify the IME back callback registration. The alternate constructor for OnBackInvokedCallbackWrapper is removed and with it a number of back and forth communication between ImeOnBackInvokedDispatcher and WindowOnBackInvokedDispatcher. ImeOnBackInvokedDispatcher also no longer needs to hold its own touch tracker, handler and BackProgressAnimator. It comes at the cost of one more layer of callback wrapping. This means that the OnBackInvokedCallback coming from the IME is wrapped once before sending it from the IME process to the app process. It is wrapped a second time when it is sent from the app process to WM (if it is the top callback). Bug: 341013064 Test: atest FrameworksCoreTests:WindowOnBackInvokedDispatcherTest Test: Manual, i.e. testing that registered ImeOnBackInvokedCallbacks work correctly. Change-Id: I45744fb5c62786a437880c013427083f42562e55 --- core/java/android/window/BackEvent.java | 6 + .../android/window/ImeOnBackInvokedDispatcher.java | 186 ++++++++++----------- .../window/WindowOnBackInvokedDispatcher.java | 106 ++---------- 3 files changed, 108 insertions(+), 190 deletions(-) diff --git a/core/java/android/window/BackEvent.java b/core/java/android/window/BackEvent.java index 55623608b025..d3733b71f7ee 100644 --- a/core/java/android/window/BackEvent.java +++ b/core/java/android/window/BackEvent.java @@ -48,6 +48,12 @@ public final class BackEvent { @SwipeEdge private final int mSwipeEdge; + /** @hide */ + public static BackEvent fromBackMotionEvent(BackMotionEvent backMotionEvent) { + return new BackEvent(backMotionEvent.getTouchX(), backMotionEvent.getTouchY(), + backMotionEvent.getProgress(), backMotionEvent.getSwipeEdge()); + } + /** * Creates a new {@link BackEvent} instance. * diff --git a/core/java/android/window/ImeOnBackInvokedDispatcher.java b/core/java/android/window/ImeOnBackInvokedDispatcher.java index 3b9b16270600..2a12507679f5 100644 --- a/core/java/android/window/ImeOnBackInvokedDispatcher.java +++ b/core/java/android/window/ImeOnBackInvokedDispatcher.java @@ -32,6 +32,7 @@ import android.view.ViewRootImpl; import com.android.internal.annotations.VisibleForTesting; import java.util.ArrayList; +import java.util.function.Consumer; /** * A {@link OnBackInvokedDispatcher} for IME that forwards {@link OnBackInvokedCallback} @@ -52,17 +53,8 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc static final String RESULT_KEY_PRIORITY = "priority"; static final int RESULT_CODE_REGISTER = 0; static final int RESULT_CODE_UNREGISTER = 1; - static final int RESULT_CODE_START_DISPATCHING = 2; - static final int RESULT_CODE_STOP_DISPATCHING = 3; @NonNull private final ResultReceiver mResultReceiver; - @NonNull - private final BackProgressAnimator mProgressAnimator = new BackProgressAnimator(); - @NonNull - private final BackTouchTracker mTouchTracker = new BackTouchTracker(); - // The handler to run callbacks on. This should be on the same thread - // the ViewRootImpl holding IME's WindowOnBackInvokedDispatcher is created on. - private Handler mHandler; public ImeOnBackInvokedDispatcher(Handler handler) { mResultReceiver = new ResultReceiver(handler) { @@ -89,10 +81,6 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc mResultReceiver = in.readTypedObject(ResultReceiver.CREATOR); } - void setHandler(@NonNull Handler handler) { - mHandler = handler; - } - @Override public void registerOnBackInvokedCallback( @OnBackInvokedDispatcher.Priority int priority, @@ -103,14 +91,7 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc // This is necessary because the callback is sent to and registered from // the app process, which may treat the IME callback as weakly referenced. This will not // cause a memory leak because the app side already clears the reference correctly. - final IOnBackInvokedCallback iCallback = - new ImeOnBackInvokedCallbackWrapper( - callback, - mTouchTracker, - mProgressAnimator, - this, - mHandler != null ? mHandler : Handler.getMain(), - false /* useWeakRef */); + final IOnBackInvokedCallback iCallback = new ImeOnBackInvokedCallbackWrapper(callback); bundle.putBinder(RESULT_KEY_CALLBACK, iCallback.asBinder()); bundle.putInt(RESULT_KEY_PRIORITY, priority); bundle.putInt(RESULT_KEY_ID, callback.hashCode()); @@ -135,12 +116,6 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc dest.writeTypedObject(mResultReceiver, flags); } - /** Sets the progress thresholds for touch tracking */ - public void setProgressThresholds(float linearDistance, float maxDistance, - float nonLinearFactor) { - mTouchTracker.setProgressThresholds(linearDistance, maxDistance, nonLinearFactor); - } - @NonNull public static final Parcelable.Creator CREATOR = new Parcelable.Creator() { @@ -162,15 +137,10 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc int priority = resultData.getInt(RESULT_KEY_PRIORITY); final IOnBackInvokedCallback callback = IOnBackInvokedCallback.Stub.asInterface( resultData.getBinder(RESULT_KEY_CALLBACK)); - registerReceivedCallback( - callback, priority, callbackId, receivingDispatcher); + registerReceivedCallback(callback, priority, callbackId, receivingDispatcher); } else if (resultCode == RESULT_CODE_UNREGISTER) { final int callbackId = resultData.getInt(RESULT_KEY_ID); unregisterReceivedCallback(callbackId, receivingDispatcher); - } else if (resultCode == RESULT_CODE_START_DISPATCHING) { - receiveStartDispatching(receivingDispatcher); - } else if (resultCode == RESULT_CODE_STOP_DISPATCHING) { - receiveStopDispatching(receivingDispatcher); } } @@ -212,63 +182,6 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc mImeCallbacks.remove(callback); } - static class ImeOnBackInvokedCallbackWrapper extends - WindowOnBackInvokedDispatcher.OnBackInvokedCallbackWrapper { - @NonNull - private final ImeOnBackInvokedDispatcher mDispatcher; - - ImeOnBackInvokedCallbackWrapper( - @NonNull OnBackInvokedCallback callback, - @NonNull BackTouchTracker touchTracker, - @NonNull BackProgressAnimator progressAnimator, - @NonNull ImeOnBackInvokedDispatcher dispatcher, - @NonNull Handler handler, - boolean useWeakRef) { - super(callback, touchTracker, progressAnimator, handler, useWeakRef); - mDispatcher = dispatcher; - } - - @Override - public void onBackStarted(BackMotionEvent backEvent) { - super.onBackStarted(backEvent); - mDispatcher.sendStartDispatching(); - } - - @Override - public void onBackCancelled() { - super.onBackCancelled(); - mDispatcher.sendStopDispatching(); - } - - @Override - public void onBackInvoked() throws RemoteException { - super.onBackInvoked(); - mDispatcher.sendStopDispatching(); - } - } - - /** Notifies the app process that we've stopped dispatching to an IME callback */ - private void sendStopDispatching() { - mResultReceiver.send(RESULT_CODE_STOP_DISPATCHING, null /* unused bundle */); - } - - /** Notifies the app process that we've started dispatching to an IME callback */ - private void sendStartDispatching() { - mResultReceiver.send(RESULT_CODE_START_DISPATCHING, null /* unused bundle */); - } - - /** Receives IME's message that dispatching has started. */ - private void receiveStopDispatching( - @NonNull WindowOnBackInvokedDispatcher receivingDispatcher) { - receivingDispatcher.onStopImeDispatching(); - } - - /** Receives IME's message that dispatching has stopped. */ - private void receiveStartDispatching( - @NonNull WindowOnBackInvokedDispatcher receivingDispatcher) { - receivingDispatcher.onStartImeDispatching(); - } - /** Clears all registered callbacks on the instance. */ public void clear() { // Unregister previously registered callbacks if there's any. @@ -278,14 +191,10 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc } } mImeCallbacks.clear(); - // We should also stop running animations since all callbacks have been removed. - // note: mSpring.skipToEnd(), in ProgressAnimator.reset(), requires the main handler. - Handler.getMain().post(mProgressAnimator::reset); - sendStopDispatching(); } @VisibleForTesting(visibility = PACKAGE) - public static class ImeOnBackInvokedCallback implements OnBackInvokedCallback { + public static class ImeOnBackInvokedCallback implements OnBackAnimationCallback { @NonNull private final IOnBackInvokedCallback mIOnBackInvokedCallback; /** @@ -302,23 +211,50 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc mPriority = priority; } + @Override + public void onBackStarted(@NonNull BackEvent backEvent) { + try { + mIOnBackInvokedCallback.onBackStarted( + new BackMotionEvent(backEvent.getTouchX(), backEvent.getTouchY(), + backEvent.getProgress(), 0f, 0f, false, backEvent.getSwipeEdge(), + null)); + } catch (RemoteException e) { + Log.e(TAG, "Exception when invoking forwarded callback. e: ", e); + } + } + + @Override + public void onBackProgressed(@NonNull BackEvent backEvent) { + try { + mIOnBackInvokedCallback.onBackProgressed( + new BackMotionEvent(backEvent.getTouchX(), backEvent.getTouchY(), + backEvent.getProgress(), 0f, 0f, false, backEvent.getSwipeEdge(), + null)); + } catch (RemoteException e) { + Log.e(TAG, "Exception when invoking forwarded callback. e: ", e); + } + } + @Override public void onBackInvoked() { try { - if (mIOnBackInvokedCallback != null) { - mIOnBackInvokedCallback.onBackInvoked(); - } + mIOnBackInvokedCallback.onBackInvoked(); } catch (RemoteException e) { Log.e(TAG, "Exception when invoking forwarded callback. e: ", e); } } - private int getId() { - return mId; + @Override + public void onBackCancelled() { + try { + mIOnBackInvokedCallback.onBackCancelled(); + } catch (RemoteException e) { + Log.e(TAG, "Exception when invoking forwarded callback. e: ", e); + } } - IOnBackInvokedCallback getIOnBackInvokedCallback() { - return mIOnBackInvokedCallback; + private int getId() { + return mId; } @Override @@ -358,4 +294,50 @@ public class ImeOnBackInvokedDispatcher implements OnBackInvokedDispatcher, Parc } } } + + /** + * Wrapper class that wraps an OnBackInvokedCallback. This is used when a callback is sent from + * the IME process to the app process. + */ + private class ImeOnBackInvokedCallbackWrapper extends IOnBackInvokedCallback.Stub { + + private final OnBackInvokedCallback mCallback; + + ImeOnBackInvokedCallbackWrapper(@NonNull OnBackInvokedCallback callback) { + mCallback = callback; + } + + @Override + public void onBackStarted(BackMotionEvent backMotionEvent) { + maybeRunOnAnimationCallback((animationCallback) -> animationCallback.onBackStarted( + BackEvent.fromBackMotionEvent(backMotionEvent))); + } + + @Override + public void onBackProgressed(BackMotionEvent backMotionEvent) { + maybeRunOnAnimationCallback((animationCallback) -> animationCallback.onBackProgressed( + BackEvent.fromBackMotionEvent(backMotionEvent))); + } + + @Override + public void onBackCancelled() { + maybeRunOnAnimationCallback(OnBackAnimationCallback::onBackCancelled); + } + + @Override + public void onBackInvoked() { + mCallback.onBackInvoked(); + } + + @Override + public void setTriggerBack(boolean triggerBack) { + // no-op + } + + private void maybeRunOnAnimationCallback(Consumer block) { + if (mCallback instanceof OnBackAnimationCallback) { + block.accept((OnBackAnimationCallback) mCallback); + } + } + } } diff --git a/core/java/android/window/WindowOnBackInvokedDispatcher.java b/core/java/android/window/WindowOnBackInvokedDispatcher.java index b9c8839ea407..0ff52f13222d 100644 --- a/core/java/android/window/WindowOnBackInvokedDispatcher.java +++ b/core/java/android/window/WindowOnBackInvokedDispatcher.java @@ -105,7 +105,6 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { // The threshold for back swipe full progress. private float mBackSwipeLinearThreshold; private float mNonLinearProgressFactor; - private boolean mImeDispatchingActive; public WindowOnBackInvokedDispatcher(@NonNull Context context, Looper looper) { mChecker = new Checker(context); @@ -176,18 +175,19 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { mImeDispatcher.registerOnBackInvokedCallback(priority, callback); return; } - if ((callback instanceof ImeOnBackInvokedDispatcher.DefaultImeOnBackAnimationCallback - || callback instanceof ImeOnBackInvokedDispatcher.ImeOnBackInvokedCallback) - && !isOnBackInvokedCallbackEnabled()) { + if (callback instanceof ImeOnBackInvokedDispatcher.ImeOnBackInvokedCallback) { // Fall back to compat back key injection if legacy back behaviour should be used. - return; + if (!isOnBackInvokedCallbackEnabled()) return; + if (callback instanceof ImeOnBackInvokedDispatcher.DefaultImeOnBackAnimationCallback + && mImeBackAnimationController != null) { + // register ImeBackAnimationController instead to play predictive back animation + callback = mImeBackAnimationController; + } } + if (!mOnBackInvokedCallbacks.containsKey(priority)) { mOnBackInvokedCallbacks.put(priority, new ArrayList<>()); } - if (callback instanceof ImeOnBackInvokedDispatcher.DefaultImeOnBackAnimationCallback) { - callback = mImeBackAnimationController; - } ArrayList callbacks = mOnBackInvokedCallbacks.get(priority); // If callback has already been added, remove it and re-add it. @@ -250,7 +250,7 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { */ public boolean isBackGestureInProgress() { synchronized (mLock) { - return mTouchTracker.isActive() || mImeDispatchingActive; + return mTouchTracker.isActive(); } } @@ -308,16 +308,8 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { OnBackInvokedCallbackInfo callbackInfo = null; if (callback != null) { int priority = mAllCallbacks.get(callback); - final IOnBackInvokedCallback iCallback = - callback instanceof ImeOnBackInvokedDispatcher - .ImeOnBackInvokedCallback - ? ((ImeOnBackInvokedDispatcher.ImeOnBackInvokedCallback) - callback).getIOnBackInvokedCallback() - : new OnBackInvokedCallbackWrapper( - callback, - mTouchTracker, - mProgressAnimator, - mHandler); + final IOnBackInvokedCallback iCallback = new OnBackInvokedCallbackWrapper( + callback, mTouchTracker, mProgressAnimator, mHandler); callbackInfo = new OnBackInvokedCallbackInfo( iCallback, priority, @@ -367,10 +359,6 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { float linearDistance = Math.min(maxDistance, mBackSwipeLinearThreshold); mTouchTracker.setProgressThresholds( linearDistance, maxDistance, mNonLinearProgressFactor); - if (mImeDispatcher != null) { - mImeDispatcher.setProgressThresholds( - linearDistance, maxDistance, mNonLinearProgressFactor); - } } /** @@ -402,46 +390,9 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { } } - /** - * Called when we start dispatching to a callback registered from IME. - */ - public void onStartImeDispatching() { - synchronized (mLock) { - mImeDispatchingActive = true; - } - } - - /** - * Called when we stop dispatching to a callback registered from IME. - */ - public void onStopImeDispatching() { - synchronized (mLock) { - mImeDispatchingActive = false; - } - } - - static class OnBackInvokedCallbackWrapper extends IOnBackInvokedCallback.Stub { - static class CallbackRef { - final WeakReference mWeakRef; - final OnBackInvokedCallback mStrongRef; - CallbackRef(@NonNull OnBackInvokedCallback callback, boolean useWeakRef) { - if (useWeakRef) { - mWeakRef = new WeakReference<>(callback); - mStrongRef = null; - } else { - mStrongRef = callback; - mWeakRef = null; - } - } - - OnBackInvokedCallback get() { - if (mStrongRef != null) { - return mStrongRef; - } - return mWeakRef.get(); - } - } - final CallbackRef mCallbackRef; + private static class OnBackInvokedCallbackWrapper extends IOnBackInvokedCallback.Stub { + @NonNull + private final WeakReference mCallback; @NonNull private final BackProgressAnimator mProgressAnimator; @NonNull @@ -454,19 +405,7 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { @NonNull BackTouchTracker touchTracker, @NonNull BackProgressAnimator progressAnimator, @NonNull Handler handler) { - mCallbackRef = new CallbackRef(callback, true /* useWeakRef */); - mTouchTracker = touchTracker; - mProgressAnimator = progressAnimator; - mHandler = handler; - } - - OnBackInvokedCallbackWrapper( - @NonNull OnBackInvokedCallback callback, - @NonNull BackTouchTracker touchTracker, - @NonNull BackProgressAnimator progressAnimator, - @NonNull Handler handler, - boolean useWeakRef) { - mCallbackRef = new CallbackRef(callback, useWeakRef); + mCallback = new WeakReference<>(callback); mTouchTracker = touchTracker; mProgressAnimator = progressAnimator; mHandler = handler; @@ -489,11 +428,7 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { backEvent.getTouchX(), backEvent.getTouchY(), backEvent.getSwipeEdge()); if (callback != null) { - callback.onBackStarted(new BackEvent( - backEvent.getTouchX(), - backEvent.getTouchY(), - backEvent.getProgress(), - backEvent.getSwipeEdge())); + callback.onBackStarted(BackEvent.fromBackMotionEvent(backEvent)); mProgressAnimator.onBackStarted(backEvent, callback::onBackProgressed); } }); @@ -519,7 +454,7 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { boolean isInProgress = mProgressAnimator.isBackAnimationInProgress(); mProgressAnimator.reset(); // TODO(b/333957271): Re-introduce auto fling progress generation. - final OnBackInvokedCallback callback = mCallbackRef.get(); + final OnBackInvokedCallback callback = mCallback.get(); if (callback == null) { Log.d(TAG, "Trying to call onBackInvoked() on a null callback reference."); return; @@ -539,7 +474,7 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { @Nullable private OnBackAnimationCallback getBackAnimationCallback() { - OnBackInvokedCallback callback = mCallbackRef.get(); + OnBackInvokedCallback callback = mCallback.get(); return callback instanceof OnBackAnimationCallback ? (OnBackAnimationCallback) callback : null; } @@ -569,11 +504,6 @@ public class WindowOnBackInvokedDispatcher implements OnBackInvokedDispatcher { public void setImeOnBackInvokedDispatcher( @NonNull ImeOnBackInvokedDispatcher imeDispatcher) { mImeDispatcher = imeDispatcher; - mImeDispatcher.setHandler(mHandler); - mImeDispatcher.setProgressThresholds( - mTouchTracker.getLinearDistance(), - mTouchTracker.getMaxDistance(), - mTouchTracker.getNonLinearFactor()); } /** Returns true if a non-null {@link ImeOnBackInvokedDispatcher} has been set. **/ -- cgit v1.2.3-59-g8ed1b