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
diff --git a/core/java/android/window/BackEvent.java b/core/java/android/window/BackEvent.java
index 5562360..d3733b7 100644
--- a/core/java/android/window/BackEvent.java
+++ b/core/java/android/window/BackEvent.java
@@ -48,6 +48,12 @@
@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 3b9b162..2a12507 100644
--- a/core/java/android/window/ImeOnBackInvokedDispatcher.java
+++ b/core/java/android/window/ImeOnBackInvokedDispatcher.java
@@ -32,6 +32,7 @@
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 @@
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 @@
mResultReceiver = in.readTypedObject(ResultReceiver.CREATOR);
}
- void setHandler(@NonNull Handler handler) {
- mHandler = handler;
- }
-
@Override
public void registerOnBackInvokedCallback(
@OnBackInvokedDispatcher.Priority int priority,
@@ -103,14 +91,7 @@
// 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 @@
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<ImeOnBackInvokedDispatcher> CREATOR =
new Parcelable.Creator<ImeOnBackInvokedDispatcher>() {
@@ -162,15 +137,10 @@
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 @@
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 @@
}
}
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;
/**
@@ -303,11 +212,42 @@
}
@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);
+ }
+ }
+
+ @Override
+ public void onBackCancelled() {
+ try {
+ mIOnBackInvokedCallback.onBackCancelled();
} catch (RemoteException e) {
Log.e(TAG, "Exception when invoking forwarded callback. e: ", e);
}
@@ -317,10 +257,6 @@
return mId;
}
- IOnBackInvokedCallback getIOnBackInvokedCallback() {
- return mIOnBackInvokedCallback;
- }
-
@Override
public String toString() {
return "ImeCallback=ImeOnBackInvokedCallback@" + mId
@@ -358,4 +294,50 @@
}
}
}
+
+ /**
+ * 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<OnBackAnimationCallback> 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 b9c8839..0ff52f1 100644
--- a/core/java/android/window/WindowOnBackInvokedDispatcher.java
+++ b/core/java/android/window/WindowOnBackInvokedDispatcher.java
@@ -105,7 +105,6 @@
// 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 @@
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<OnBackInvokedCallback> callbacks = mOnBackInvokedCallbacks.get(priority);
// If callback has already been added, remove it and re-add it.
@@ -250,7 +250,7 @@
*/
public boolean isBackGestureInProgress() {
synchronized (mLock) {
- return mTouchTracker.isActive() || mImeDispatchingActive;
+ return mTouchTracker.isActive();
}
}
@@ -308,16 +308,8 @@
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 @@
float linearDistance = Math.min(maxDistance, mBackSwipeLinearThreshold);
mTouchTracker.setProgressThresholds(
linearDistance, maxDistance, mNonLinearProgressFactor);
- if (mImeDispatcher != null) {
- mImeDispatcher.setProgressThresholds(
- linearDistance, maxDistance, mNonLinearProgressFactor);
- }
}
/**
@@ -402,46 +390,9 @@
}
}
- /**
- * 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<OnBackInvokedCallback> 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<OnBackInvokedCallback> mCallback;
@NonNull
private final BackProgressAnimator mProgressAnimator;
@NonNull
@@ -454,19 +405,7 @@
@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 @@
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 @@
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 @@
@Nullable
private OnBackAnimationCallback getBackAnimationCallback() {
- OnBackInvokedCallback callback = mCallbackRef.get();
+ OnBackInvokedCallback callback = mCallback.get();
return callback instanceof OnBackAnimationCallback ? (OnBackAnimationCallback) callback
: null;
}
@@ -569,11 +504,6 @@
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. **/