Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 77f85861 authored by Felipe Leme's avatar Felipe Leme
Browse files

Removed afm.notifyValueChanged() optimization. 

On Android P, TextView was keeping track of the last mText that was sent to
AutofillManager to avoid calling it again if the value didn't change, as that
would incur on unnecessary IPC calls from AFM to AFMService in the UIThread.

Now on Android Q this optimization is causing the method to not be called when
it should when the mText is a reference to a SpannableStringBuilder, as it's
equals() method now returns true in this case (before it was returning false,
which was a bug: if the reference didn't change, it should return true).

We have 2 options to solve this problem:

1.Fix TextView to keep a String copy of mText.
2.Remove the optimization.

This CL fixes it using #2, for 2 reasons:

1.On Android Q, the AFM calls to AFMS are async, so it's not a jankiness issue
anymore.
2.Making a copy will actually be *worse* for performance, as it would be making
an unnecessary copy for the cases where autofill is disabled.

Test: atest android.autofillservice.cts.DatasetFilteringTest#testFilter_usingKeyboard
Test: atest CtsAutoFillServiceTestCases # to make sure it didn't break anything
Fixes: 117106046

Change-Id: Ia1c69e2d7a478288f65566e862f4a43e88eca463
parent 45434d2d

core/java/android/widget/TextView.java

+5 −18
Original line number Diff line number Diff line
@@ -882,9 +882,6 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener 
    private boolean mTextSetFromXmlOrResourceId = false;

    // Resource id used to set the text.

    private @StringRes int mTextId = ResourceId.ID_NULL;

    // Last value used on AFM.notifyValueChanged(), used to optimize autofill workflow by avoiding

    // calls when the value did not change

    private CharSequence mLastValueSentToAutofillManager;

    //

    // End of autofill-related attributes
@@ -5884,7 +5881,7 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener 
        if (needEditableForNotification) {

            sendAfterTextChanged((Editable) text);

        } else {

            notifyAutoFillManagerAfterTextChangedIfNeeded();

            notifyAutoFillManagerAfterTextChanged();

        }



        // SelectionModifierCursorController depends on textCanBeSelected, which depends on text
@@ -9933,33 +9930,23 @@ public class TextView extends View implements ViewTreeObserver.OnPreDrawListener 
        }



        // Always notify AutoFillManager - it will return right away if autofill is disabled.

        notifyAutoFillManagerAfterTextChangedIfNeeded();

        notifyAutoFillManagerAfterTextChanged();



        hideErrorIfUnchanged();

    }



    private void notifyAutoFillManagerAfterTextChangedIfNeeded() {

    private void notifyAutoFillManagerAfterTextChanged() {

        // It is important to not check whether the view is important for autofill

        // since the user can trigger autofill manually on not important views.

        if (!isAutofillable()) {

            return;

        }

        final AutofillManager afm = mContext.getSystemService(AutofillManager.class);

        if (afm == null) {

            return;

        }



        if (mLastValueSentToAutofillManager == null

                || !mLastValueSentToAutofillManager.equals(mText)) {

        if (afm != null) {

            if (android.view.autofill.Helper.sVerbose) {

                Log.v(LOG_TAG, "notifying AFM after text changed");

                Log.v(LOG_TAG, "notifyAutoFillManagerAfterTextChanged");

            }

            afm.notifyValueChanged(TextView.this);

            mLastValueSentToAutofillManager = mText;

        } else {

            if (android.view.autofill.Helper.sVerbose) {

                Log.v(LOG_TAG, "not notifying AFM on unchanged text");

            }

        }

    }