Merge "Don't call IMMS on focusing across non-edit views"

This commit is contained in:
Nikolas Havrikov
2022-07-26 16:43:26 +00:00
committed by Android (Google) Code Review
6 changed files with 281 additions and 1 deletions

View File

@@ -1099,4 +1099,41 @@ public class EditorInfo implements InputType, Parcelable {
public int describeContents() {
return 0;
}
/**
* Performs a loose equality check, which means there can be false negatives, but if the method
* returns {@code true}, then both objects are guaranteed to be equal.
* <ul>
* <li>{@link #extras} is compared with {@link Bundle#kindofEquals}</li>
* <li>{@link #actionLabel}, {@link #hintText}, and {@link #label} are compared with
* {@link TextUtils#equals}, which does not account for Spans. </li>
* </ul>
* @hide
*/
public boolean kindofEquals(@Nullable EditorInfo that) {
if (that == null) return false;
if (this == that) return true;
return inputType == that.inputType
&& imeOptions == that.imeOptions
&& internalImeOptions == that.internalImeOptions
&& actionId == that.actionId
&& initialSelStart == that.initialSelStart
&& initialSelEnd == that.initialSelEnd
&& initialCapsMode == that.initialCapsMode
&& fieldId == that.fieldId
&& Objects.equals(autofillId, that.autofillId)
&& Objects.equals(privateImeOptions, that.privateImeOptions)
&& Objects.equals(packageName, that.packageName)
&& Objects.equals(fieldName, that.fieldName)
&& Objects.equals(hintLocales, that.hintLocales)
&& Objects.equals(targetInputMethodUser, that.targetInputMethodUser)
&& Arrays.equals(contentMimeTypes, that.contentMimeTypes)
&& TextUtils.equals(actionLabel, that.actionLabel)
&& TextUtils.equals(hintText, that.hintText)
&& TextUtils.equals(label, that.label)
&& (extras == that.extras || (extras != null && extras.kindofEquals(that.extras)))
&& (mInitialSurroundingText == that.mInitialSurroundingText
|| (mInitialSurroundingText != null
&& mInitialSurroundingText.isEqualTo(that.mInitialSurroundingText)));
}
}

View File

@@ -70,6 +70,7 @@ import android.os.Process;
import android.os.ResultReceiver;
import android.os.ServiceManager;
import android.os.ServiceManager.ServiceNotFoundException;
import android.os.SystemProperties;
import android.os.Trace;
import android.os.UserHandle;
import android.provider.Settings;
@@ -397,6 +398,18 @@ public final class InputMethodManager {
@EnabledSince(targetSdkVersion = Build.VERSION_CODES.TIRAMISU)
public static final long CLEAR_SHOW_FORCED_FLAG_WHEN_LEAVING = 214016041L; // This is a bug id.
/**
* If {@code true}, avoid calling the
* {@link com.android.server.inputmethod.InputMethodManagerService InputMethodManagerService}
* by skipping the call to {@link IInputMethodManager#startInputOrWindowGainedFocus}
* when we are switching focus between two non-editable views. This saves the cost of a binder
* call into the system server.
* <p><b>Note:</b>
* The default value is {@code true}.
*/
private static final boolean OPTIMIZE_NONEDITABLE_VIEWS =
SystemProperties.getBoolean("debug.imm.optimize_noneditable_views", true);
/**
* @deprecated Use {@link #mServiceInvoker} instead.
*/
@@ -646,6 +659,26 @@ public final class InputMethodManager {
private final class DelegateImpl implements
ImeFocusController.InputMethodManagerDelegate {
@GuardedBy("mH")
@Nullable
private ViewFocusParameterInfo mPreviousViewFocusParameters;
@GuardedBy("mH")
private void updatePreviousViewFocusParametersLocked(
@Nullable EditorInfo currentEditorInfo,
@StartInputFlags int startInputFlags,
@StartInputReason int startInputReason,
@SoftInputModeFlags int softInputMode,
int windowFlags) {
mPreviousViewFocusParameters = new ViewFocusParameterInfo(currentEditorInfo,
startInputFlags, startInputReason, softInputMode, windowFlags);
}
@GuardedBy("mH")
private void clearStateLocked() {
mPreviousViewFocusParameters = null;
}
/**
* Used by {@link ImeFocusController} to start input connection.
*/
@@ -1692,8 +1725,10 @@ public final class InputMethodManager {
* Reset all of the state associated with a served view being connected
* to an input method
*/
@GuardedBy("mH")
private void clearConnectionLocked() {
mCurrentEditorInfo = null;
mDelegate.clearStateLocked();
if (mServedInputConnection != null) {
mServedInputConnection.deactivate();
mServedInputConnection = null;
@@ -2344,6 +2379,9 @@ public final class InputMethodManager {
// Hook 'em up and let 'er rip.
mCurrentEditorInfo = tba.createCopyInternal();
// Store the previously served connection so that we can determine whether it is safe
// to skip the call to startInputOrWindowGainedFocus in the IMMS
final RemoteInputConnectionImpl previouslyServedConnection = mServedInputConnection;
mServedConnecting = false;
if (mServedInputConnection != null) {
@@ -2383,6 +2421,22 @@ public final class InputMethodManager {
+ ic + " tba=" + tba + " startInputFlags="
+ InputMethodDebug.startInputFlagsToString(startInputFlags));
}
// When we switch between non-editable views, do not call into the IMMS.
final boolean canSkip = OPTIMIZE_NONEDITABLE_VIEWS
&& previouslyServedConnection == null
&& ic == null
&& isSwitchingBetweenEquivalentNonEditableViews(
mDelegate.mPreviousViewFocusParameters, startInputFlags,
startInputReason, softInputMode, windowFlags);
updatePreviousViewFocusParametersLocked(mCurrentEditorInfo, startInputFlags,
startInputReason, softInputMode, windowFlags);
if (canSkip) {
if (DEBUG) {
Log.d(TAG, "Not calling IMMS due to switching between non-editable views.");
}
return false;
}
res = mServiceInvoker.startInputOrWindowGainedFocus(
startInputReason, mClient, windowGainingFocus, startInputFlags,
softInputMode, windowFlags, tba, servedInputConnection,
@@ -2445,6 +2499,47 @@ public final class InputMethodManager {
return true;
}
/**
* This method exists only so that the
* <a href="https://errorprone.info/bugpattern/GuardedBy">errorprone</a> false positive warning
* can be suppressed without granting a blanket exception to the {@link #startInputInner}
* method.
* <p>
* The warning in question implies that the access to the
* {@link DelegateImpl#updatePreviousViewFocusParametersLocked} method should be guarded by
* {@code InputMethodManager.this.mH}, but instead {@code mDelegate.mH} is held in the caller.
* In this case errorprone fails to realize that it is the same object.
*/
@GuardedBy("mH")
@SuppressWarnings("GuardedBy")
private void updatePreviousViewFocusParametersLocked(
@Nullable EditorInfo currentEditorInfo,
@StartInputFlags int startInputFlags,
@StartInputReason int startInputReason,
@SoftInputModeFlags int softInputMode,
int windowFlags) {
mDelegate.updatePreviousViewFocusParametersLocked(currentEditorInfo, startInputFlags,
startInputReason, softInputMode, windowFlags);
}
/**
* @return {@code true} when we are switching focus between two non-editable views
* so that we can avoid calling {@link IInputMethodManager#startInputOrWindowGainedFocus}.
*/
@GuardedBy("mH")
private boolean isSwitchingBetweenEquivalentNonEditableViews(
@Nullable ViewFocusParameterInfo previousViewFocusParameters,
@StartInputFlags int startInputFlags,
@StartInputReason int startInputReason,
@SoftInputModeFlags int softInputMode,
int windowFlags) {
return (startInputFlags & StartInputFlags.WINDOW_GAINED_FOCUS) == 0
&& (startInputFlags & StartInputFlags.IS_TEXT_EDITOR) == 0
&& previousViewFocusParameters != null
&& previousViewFocusParameters.sameAs(mCurrentEditorInfo,
startInputFlags, startInputReason, softInputMode, windowFlags);
}
private void reportInputConnectionOpened(
InputConnection ic, EditorInfo tba, Handler icHandler, View view) {
view.onInputConnectionOpenedInternal(ic, tba, icHandler);

View File

@@ -181,4 +181,14 @@ public final class SurroundingText implements Parcelable {
}
}
}
/** @hide */
public boolean isEqualTo(@Nullable SurroundingText that) {
if (that == null) return false;
if (this == that) return true;
return mSelectionStart == that.mSelectionStart
&& mSelectionEnd == that.mSelectionEnd
&& mOffset == that.mOffset
&& TextUtils.equals(mText, that.mText);
}
}

View File

@@ -0,0 +1,64 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package android.view.inputmethod;
import android.annotation.Nullable;
import android.view.WindowManager.LayoutParams.SoftInputModeFlags;
import com.android.internal.inputmethod.StartInputFlags;
import com.android.internal.inputmethod.StartInputReason;
import com.android.internal.view.IInputMethodManager;
/**
* This data class is a container for storing the last arguments used when calling into
* {@link IInputMethodManager#startInputOrWindowGainedFocus}. They are used to determine if we
* are switching from a non-editable view to another non-editable view, in which case we avoid
* a binder call into the {@link com.android.server.inputmethod.InputMethodManagerService}.
*/
final class ViewFocusParameterInfo {
@Nullable final EditorInfo mPreviousEditorInfo;
@StartInputFlags final int mPreviousStartInputFlags;
@StartInputReason final int mPreviousStartInputReason;
@SoftInputModeFlags final int mPreviousSoftInputMode;
final int mPreviousWindowFlags;
ViewFocusParameterInfo(@Nullable EditorInfo previousEditorInfo,
@StartInputFlags int previousStartInputFlags,
@StartInputReason int previousStartInputReason,
@SoftInputModeFlags int previousSoftInputMode,
int previousWindowFlags) {
mPreviousEditorInfo = previousEditorInfo;
mPreviousStartInputFlags = previousStartInputFlags;
mPreviousStartInputReason = previousStartInputReason;
mPreviousSoftInputMode = previousSoftInputMode;
mPreviousWindowFlags = previousWindowFlags;
}
boolean sameAs(@Nullable EditorInfo currentEditorInfo,
@StartInputFlags int startInputFlags,
@StartInputReason int startInputReason,
@SoftInputModeFlags int softInputMode,
int windowFlags) {
return mPreviousStartInputFlags == startInputFlags
&& mPreviousStartInputReason == startInputReason
&& mPreviousSoftInputMode == softInputMode
&& mPreviousWindowFlags == windowFlags
&& (mPreviousEditorInfo == currentEditorInfo
|| (mPreviousEditorInfo != null
&& mPreviousEditorInfo.kindofEquals(currentEditorInfo)));
}
}

View File

@@ -19,6 +19,7 @@ package android.view.inputmethod;
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -59,6 +60,28 @@ public class EditorInfoTest {
private static final int TEST_USER_ID = 42;
private static final int LONG_EXP_TEXT_LENGTH = EditorInfo.MEMORY_EFFICIENT_TEXT_LENGTH * 2;
private static final EditorInfo TEST_EDITOR_INFO = new EditorInfo();
static {
TEST_EDITOR_INFO.inputType = InputType.TYPE_CLASS_TEXT; // 0x1
TEST_EDITOR_INFO.imeOptions = EditorInfo.IME_ACTION_GO; // 0x2
TEST_EDITOR_INFO.privateImeOptions = "testOptions";
TEST_EDITOR_INFO.initialSelStart = 0;
TEST_EDITOR_INFO.initialSelEnd = 1;
TEST_EDITOR_INFO.initialCapsMode = TextUtils.CAP_MODE_CHARACTERS; // 0x1000
TEST_EDITOR_INFO.hintText = "testHintText";
TEST_EDITOR_INFO.label = "testLabel";
TEST_EDITOR_INFO.packageName = "android.view.inputmethod";
TEST_EDITOR_INFO.fieldId = 0;
TEST_EDITOR_INFO.autofillId = AutofillId.NO_AUTOFILL_ID;
TEST_EDITOR_INFO.fieldName = "testField";
TEST_EDITOR_INFO.extras = new Bundle();
TEST_EDITOR_INFO.extras.putString("testKey", "testValue");
TEST_EDITOR_INFO.hintLocales = LocaleList.forLanguageTags("en,de,ua");
TEST_EDITOR_INFO.contentMimeTypes = new String[] {"image/png"};
TEST_EDITOR_INFO.targetInputMethodUser = UserHandle.of(TEST_USER_ID);
}
/**
* Makes sure that {@code null} {@link EditorInfo#targetInputMethodUser} can be copied via
* {@link Parcel}.
@@ -526,4 +549,47 @@ public class EditorInfoTest {
+ "prefix: hintLocales=null\n"
+ "prefix: contentMimeTypes=null\n");
}
@Test
public void testKindofEqualsAfterCopyInternal() {
final EditorInfo infoCopy = TEST_EDITOR_INFO.createCopyInternal();
assertTrue(TEST_EDITOR_INFO.kindofEquals(infoCopy));
}
@Test
public void testKindofEqualsAfterCloneViaParcel() {
// This test demonstrates a false negative case when an EditorInfo is
// created from a Parcel and its extras are still parcelled, which in turn
// runs into the edge case in Bundle.kindofEquals
final EditorInfo infoCopy = cloneViaParcel(TEST_EDITOR_INFO);
assertFalse(TEST_EDITOR_INFO.kindofEquals(infoCopy));
}
@Test
public void testKindofEqualsComparesAutofillId() {
final EditorInfo infoCopy = TEST_EDITOR_INFO.createCopyInternal();
infoCopy.autofillId = new AutofillId(42);
assertFalse(TEST_EDITOR_INFO.kindofEquals(infoCopy));
}
@Test
public void testKindofEqualsComparesFieldId() {
final EditorInfo infoCopy = TEST_EDITOR_INFO.createCopyInternal();
infoCopy.fieldId = 42;
assertFalse(TEST_EDITOR_INFO.kindofEquals(infoCopy));
}
@Test
public void testKindofEqualsComparesMimeTypes() {
final EditorInfo infoCopy = TEST_EDITOR_INFO.createCopyInternal();
infoCopy.contentMimeTypes = new String[] {"image/png", "image/gif"};
assertFalse(TEST_EDITOR_INFO.kindofEquals(infoCopy));
}
@Test
public void testKindofEqualsComparesExtras() {
final EditorInfo infoCopy = TEST_EDITOR_INFO.createCopyInternal();
infoCopy.extras.putString("testKey2", "testValue");
assertFalse(TEST_EDITOR_INFO.kindofEquals(infoCopy));
}
}

View File

@@ -17,7 +17,8 @@
package android.view.inputmethod;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertFalse;
import android.os.Parcel;
@@ -70,4 +71,11 @@ public class SurroundingTextTest {
assertThat(surroundingTextFromParcel.getSelectionEnd(), is(1));
assertThat(surroundingTextFromParcel.getOffset(), is(2));
}
@Test
public void testIsEqualComparesText() {
final SurroundingText text1 = new SurroundingText("hello", 0, 1, 0);
final SurroundingText text2 = new SurroundingText("there", 0, 1, 0);
assertFalse(text1.isEqualTo(text2));
}
}