Switch StatusCoordinator/Mediator to using TextWatcher

Events aren't getting through when they should when using
UrlBar.UrlTextChangeListener. Using a combination of TextWatcher and
UrlBarCoordinator triggers more frequently.

Bug: 1008474
Change-Id: I60252e7f2bca878089667cc29f79f82e18884789
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1835101
Commit-Queue: Brandon Wylie <wylieb@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704849}

chrome/android/java/src/org/chromium/chrome/browser/omnibox/LocationBarLayout.java

@@ -217,11 +217,11 @@ public class LocationBarLayout extends FrameLayout
 
         StatusView statusView = findViewById(R.id.location_bar_status);
         statusView.setCompositeTouchDelegate(mCompositeTouchDelegate);
-        mStatusViewCoordinator = new StatusViewCoordinator(mIsTablet, statusView);
+        mStatusViewCoordinator = new StatusViewCoordinator(mIsTablet, statusView, mUrlCoordinator);
+        mUrlCoordinator.addTextChangedListener(mStatusViewCoordinator);
 
         updateShouldAnimateIconChanges();
         mUrlBar.setOnKeyListener(new UrlBarKeyListener());
-        mUrlCoordinator.addUrlTextChangeListener(mStatusViewCoordinator);
 
         // mLocationBar's direction is tied to this UrlBar's text direction. Icons inside the
         // location bar, e.g. lock, refresh, X, should be reversed if UrlBar's text is RTL.
@@ -1118,6 +1118,5 @@ public class LocationBarLayout extends FrameLayout
         String textWithoutAutocomplete = mUrlCoordinator.getTextWithoutAutocomplete();
         String textWithAutocomplete = mUrlCoordinator.getTextWithAutocomplete();
         mAutocompleteCoordinator.onTextChanged(textWithoutAutocomplete, textWithAutocomplete);
-        mStatusViewCoordinator.onTextChanged(textWithoutAutocomplete, textWithAutocomplete);
     }
 }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java

@@ -13,12 +13,14 @@ import android.os.StrictMode;
 import android.provider.Settings;
 import android.support.v13.view.inputmethod.EditorInfoCompat;
 import android.support.v4.text.BidiFormatter;
+import android.support.v4.util.ObjectsCompat;
 import android.text.Editable;
 import android.text.InputType;
 import android.text.Layout;
 import android.text.Selection;
 import android.text.SpannableStringBuilder;
 import android.text.TextUtils;
+import android.text.TextWatcher;
 import android.text.style.ReplacementSpan;
 import android.util.AttributeSet;
 import android.view.GestureDetector;
@@ -97,7 +99,8 @@ public abstract class UrlBar extends AutocompleteEditText {
     private int mUrlDirection;
 
     private UrlBarDelegate mUrlBarDelegate;
-    private UrlTextChangeListener mTextChangeListener;
+    private UrlTextChangeListener mUrlTextChangeListener;
+    private TextWatcher mTextChangedListener;
     private UrlBarTextContextMenuDelegate mTextContextMenuDelegate;
     private UrlDirectionListener mUrlDirectionListener;
 
@@ -590,7 +593,22 @@ public abstract class UrlBar extends AutocompleteEditText {
      * @param listener The listener to be notified.
      */
     public void setUrlTextChangeListener(UrlTextChangeListener listener) {
-        mTextChangeListener = listener;
+        mUrlTextChangeListener = listener;
+    }
+
+    /**
+     * Set the listener to be notified when the view's text has changed.
+     * @param textChangedListener The listener to be notified.
+     */
+    public void setTextChangedListener(TextWatcher textChangedListener) {
+        if (ObjectsCompat.equals(mTextChangedListener, textChangedListener)) {
+            return;
+        } else if (mTextChangedListener != null) {
+            removeTextChangedListener(mTextChangedListener);
+        }
+
+        mTextChangedListener = textChangedListener;
+        addTextChangedListener(mTextChangedListener);
     }
 
     /**
@@ -950,12 +968,13 @@ public abstract class UrlBar extends AutocompleteEditText {
         if (DEBUG) {
             Log.i(TAG, "onAutocompleteTextStateChanged: DIS[%b]", updateDisplay);
         }
-        if (mTextChangeListener == null) return;
+        if (mUrlTextChangeListener == null) return;
         if (updateDisplay) limitDisplayableLength();
         // crbug.com/764749
         Log.w(TAG, "Text change observed, triggering autocomplete.");
 
-        mTextChangeListener.onTextChanged(getTextWithoutAutocomplete(), getTextWithAutocomplete());
+        mUrlTextChangeListener.onTextChanged(
+                getTextWithoutAutocomplete(), getTextWithAutocomplete());
     }
 
     /**

chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarCoordinator.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.omnibox;
 
+import android.text.TextWatcher;
 import android.view.ActionMode;
 
 import androidx.annotation.IntDef;
@@ -60,6 +61,11 @@ public class UrlBarCoordinator implements UrlBarEditingTextStateProvider {
         mMediator.addUrlTextChangeListener(listener);
     }
 
+    /** @see TextWatcher */
+    public void addTextChangedListener(TextWatcher textWatcher) {
+        mMediator.addTextChangedListener(textWatcher);
+    }
+
     /** @see UrlBarMediator#setUrlBarData(UrlBarData, int, int) */
     public boolean setUrlBarData(
             UrlBarData data, @ScrollType int scrollType, @SelectionState int state) {

chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarMediator.java

@@ -8,8 +8,10 @@ import android.content.ClipData;
 import android.content.ClipboardManager;
 import android.content.Context;
 import android.net.Uri;
+import android.text.Editable;
 import android.text.Spanned;
 import android.text.TextUtils;
+import android.text.TextWatcher;
 import android.text.format.DateUtils;
 import android.view.ActionMode;
 
@@ -41,7 +43,8 @@ import java.util.List;
 /**
  * Handles collecting and pushing state information to the UrlBar model.
  */
-class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate, UrlBar.UrlTextChangeListener {
+class UrlBarMediator
+        implements UrlBar.UrlBarTextContextMenuDelegate, UrlBar.UrlTextChangeListener, TextWatcher {
     private final PropertyModel mModel;
 
     private Callback<Boolean> mOnFocusChangeCallback;
@@ -58,6 +61,7 @@ class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate, UrlBar.Url
     private static final int NUM_OF_BUCKETS = 100;
 
     private final List<UrlTextChangeListener> mUrlTextChangeListeners = new ArrayList<>();
+    private final List<TextWatcher> mTextChangedListeners = new ArrayList<>();
 
     public UrlBarMediator(PropertyModel model) {
         mModel = model;
@@ -66,6 +70,7 @@ class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate, UrlBar.Url
         mModel.set(UrlBarProperties.SHOW_CURSOR, false);
         mModel.set(UrlBarProperties.TEXT_CONTEXT_MENU_DELEGATE, this);
         mModel.set(UrlBarProperties.URL_TEXT_CHANGE_LISTENER, this);
+        mModel.set(UrlBarProperties.TEXT_CHANGED_LISTENER, this);
         setUseDarkTextColors(true);
     }
 
@@ -81,6 +86,11 @@ class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate, UrlBar.Url
         mUrlTextChangeListeners.add(listener);
     }
 
+    /** @see android.widget.TextView#addTextChangedListener */
+    public void addTextChangedListener(TextWatcher textWatcher) {
+        mTextChangedListeners.add(textWatcher);
+    }
+
     /**
      * Updates the text content of the UrlBar.
      *
@@ -355,6 +365,7 @@ class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate, UrlBar.Url
         return url.substring(0, pathIndex);
     }
 
+    /** @see UrlTextChangeListener */
     @Override
     public void onTextChanged(String textWithoutAutocomplete, String textWithAutocomplete) {
         for (int i = 0; i < mUrlTextChangeListeners.size(); i++) {
@@ -363,6 +374,30 @@ class UrlBarMediator implements UrlBar.UrlBarTextContextMenuDelegate, UrlBar.Url
         }
     }
 
+    /** @see TextWatcher */
+    @Override
+    public void beforeTextChanged(CharSequence s, int start, int count, int after) {
+        for (int i = 0; i < mTextChangedListeners.size(); i++) {
+            mTextChangedListeners.get(i).beforeTextChanged(s, start, count, after);
+        }
+    }
+
+    /** @see TextWatcher */
+    @Override
+    public void onTextChanged(CharSequence s, int start, int before, int count) {
+        for (int i = 0; i < mTextChangedListeners.size(); i++) {
+            mTextChangedListeners.get(i).onTextChanged(s, start, before, count);
+        }
+    }
+
+    /** @see TextWatcher */
+    @Override
+    public void afterTextChanged(Editable editable) {
+        for (int i = 0; i < mTextChangedListeners.size(); i++) {
+            mTextChangedListeners.get(i).afterTextChanged(editable);
+        }
+    }
+
     private void recordPasteMetrics(String text) {
         boolean isUrl = BrowserStartupController.get(LibraryProcessType.PROCESS_BROWSER)
                                 .isFullBrowserStarted()

chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarProperties.java

@@ -4,6 +4,7 @@
 
 package org.chromium.chrome.browser.omnibox;
 
+import android.text.TextWatcher;
 import android.view.ActionMode;
 
 import org.chromium.base.Callback;
@@ -115,10 +116,14 @@ class UrlBarProperties {
     public static final WritableObjectPropertyKey<UrlDirectionListener> URL_DIRECTION_LISTENER =
             new WritableObjectPropertyKey<>();
 
-    /** The callback to be notified on text changes. */
+    /** The callback to be notified on url text changes. @see UrlTextChangeListener. */
     public static final WritableObjectPropertyKey<UrlTextChangeListener> URL_TEXT_CHANGE_LISTENER =
             new WritableObjectPropertyKey<>();
 
+    /** The callback to be notified on text changes. @see TextWatcher. */
+    public static final WritableObjectPropertyKey<TextWatcher> TEXT_CHANGED_LISTENER =
+            new WritableObjectPropertyKey<>();
+
     /** Specifies whether dark text colors should be used in the view. */
     public static final WritableBooleanPropertyKey USE_DARK_TEXT_COLORS =
             new WritableBooleanPropertyKey();
@@ -130,5 +135,5 @@ class UrlBarProperties {
     public static final PropertyKey[] ALL_KEYS = new PropertyKey[] {ACTION_MODE_CALLBACK,
             ALLOW_FOCUS, AUTOCOMPLETE_TEXT, DELEGATE, FOCUS_CHANGE_CALLBACK, SHOW_CURSOR,
             TEXT_CONTEXT_MENU_DELEGATE, TEXT_STATE, URL_DIRECTION_LISTENER,
-            URL_TEXT_CHANGE_LISTENER, USE_DARK_TEXT_COLORS, WINDOW_DELEGATE};
+            URL_TEXT_CHANGE_LISTENER, TEXT_CHANGED_LISTENER, USE_DARK_TEXT_COLORS, WINDOW_DELEGATE};
 }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBarViewBinder.java

@@ -73,6 +73,8 @@ class UrlBarViewBinder {
             view.setUrlDirectionListener(model.get(UrlBarProperties.URL_DIRECTION_LISTENER));
         } else if (UrlBarProperties.URL_TEXT_CHANGE_LISTENER.equals(propertyKey)) {
             view.setUrlTextChangeListener(model.get(UrlBarProperties.URL_TEXT_CHANGE_LISTENER));
+        } else if (UrlBarProperties.TEXT_CHANGED_LISTENER.equals(propertyKey)) {
+            view.setTextChangedListener(model.get(UrlBarProperties.TEXT_CHANGED_LISTENER));
         } else if (UrlBarProperties.WINDOW_DELEGATE.equals(propertyKey)) {
             view.setWindowDelegate(model.get(UrlBarProperties.WINDOW_DELEGATE));
         }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/status/StatusMediator.java

@@ -18,7 +18,7 @@ import org.chromium.base.VisibleForTesting;
 import org.chromium.base.library_loader.LibraryProcessType;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.omnibox.SearchEngineLogoUtils;
-import org.chromium.chrome.browser.omnibox.UrlBar;
+import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
 import org.chromium.chrome.browser.omnibox.suggestions.AutocompleteCoordinatorFactory;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.toolbar.ToolbarCommonPropertiesModel;
@@ -89,15 +89,18 @@ class StatusMediator {
     private StatusMediatorDelegate mDelegate;
     private Resources mResources;
     private ToolbarCommonPropertiesModel mToolbarCommonPropertiesModel;
+    private UrlBarEditingTextStateProvider mUrlBarEditingTextStateProvider;
     private String mUrlBarTextWithAutocomplete = "";
     private boolean mUrlBarTextIsValidUrl;
 
-    StatusMediator(PropertyModel model, Resources resources) {
+    StatusMediator(PropertyModel model, Resources resources,
+            UrlBarEditingTextStateProvider urlBarEditingTextStateProvider) {
         mModel = model;
         mDelegate = new StatusMediatorDelegate();
         updateColorTheme();
 
         mResources = resources;
+        mUrlBarEditingTextStateProvider = urlBarEditingTextStateProvider;
     }
 
     /**
@@ -497,8 +500,13 @@ class StatusMediator {
         return 0;
     }
 
-    /** @see {@link UrlBar.UrlTextChangeListener} */
-    void onTextChanged(String urlTextWithoutAutocomplete, String urlTextWithAutocomplete) {
+    /** @see android.text.TextWatcher#onTextChanged */
+    void onTextChanged(CharSequence charSequence) {
+        // TODO (crbug.com/1012870): This is a workaround for the linked bug. Once the bug is fixed,
+        //                           it should be removed.
+        String urlTextWithAutocomplete = TextUtils.isEmpty(charSequence)
+                ? ""
+                : mUrlBarEditingTextStateProvider.getTextWithAutocomplete();
         if (TextUtils.equals(mUrlBarTextWithAutocomplete, urlTextWithAutocomplete)) {
             return;
         }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/status/StatusViewCoordinator.java

@@ -5,13 +5,15 @@
 package org.chromium.chrome.browser.omnibox.status;
 
 import android.content.res.Resources;
+import android.text.Editable;
+import android.text.TextWatcher;
 import android.view.View;
 
 import androidx.annotation.DrawableRes;
 
 import org.chromium.base.VisibleForTesting;
 import org.chromium.chrome.R;
-import org.chromium.chrome.browser.omnibox.UrlBar;
+import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
 import org.chromium.chrome.browser.page_info.PageInfoController;
 import org.chromium.chrome.browser.toolbar.ToolbarDataProvider;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -21,7 +23,7 @@ import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
  * A component for displaying a status icon (e.g. security icon or navigation icon) and optional
  * verbose status text.
  */
-public class StatusViewCoordinator implements View.OnClickListener, UrlBar.UrlTextChangeListener {
+public class StatusViewCoordinator implements View.OnClickListener, TextWatcher {
     private final StatusView mStatusView;
     private final StatusMediator mMediator;
     private final PropertyModel mModel;
@@ -34,8 +36,10 @@ public class StatusViewCoordinator implements View.OnClickListener, UrlBar.UrlTe
      * Creates a new StatusViewCoordinator.
      * @param isTablet Whether the UI is shown on a tablet.
      * @param statusView The status view, used to supply and manipulate child views.
+     * @param urlBarEditingTextStateProvider The url coordinator.
      */
-    public StatusViewCoordinator(boolean isTablet, StatusView statusView) {
+    public StatusViewCoordinator(boolean isTablet, StatusView statusView,
+            UrlBarEditingTextStateProvider urlBarEditingTextStateProvider) {
         mIsTablet = isTablet;
         mStatusView = statusView;
 
@@ -44,7 +48,8 @@ public class StatusViewCoordinator implements View.OnClickListener, UrlBar.UrlTe
                          .build();
 
         PropertyModelChangeProcessor.create(mModel, mStatusView, new StatusViewBinder());
-        mMediator = new StatusMediator(mModel, mStatusView.getResources());
+        mMediator = new StatusMediator(
+                mModel, mStatusView.getResources(), urlBarEditingTextStateProvider);
 
         Resources res = mStatusView.getResources();
         mMediator.setUrlMinWidth(res.getDimensionPixelSize(R.dimen.location_bar_min_url_width)
@@ -216,7 +221,13 @@ public class StatusViewCoordinator implements View.OnClickListener, UrlBar.UrlTe
     }
 
     @Override
-    public void onTextChanged(String textWithoutAutocomplete, String textWithAutocomplete) {
-        mMediator.onTextChanged(textWithoutAutocomplete, textWithAutocomplete);
+    public void beforeTextChanged(CharSequence s, int start, int count, int after) {}
+
+    @Override
+    public void onTextChanged(CharSequence charSequence, int start, int before, int count) {
+        mMediator.onTextChanged(charSequence);
     }
+
+    @Override
+    public void afterTextChanged(Editable editable) {}
 }

chrome/android/junit/src/org/chromium/chrome/browser/omnibox/status/StatusMediatorUnitTest.java

@@ -25,6 +25,7 @@ import org.chromium.base.Callback;
 import org.chromium.base.test.BaseRobolectricTestRunner;
 import org.chromium.chrome.R;
 import org.chromium.chrome.browser.ChromeFeatureList;
+import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
 import org.chromium.chrome.browser.toolbar.ToolbarCommonPropertiesModel;
 import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.ui.modelutil.PropertyModel;
@@ -42,6 +43,8 @@ public final class StatusMediatorUnitTest {
     @Mock
     ToolbarCommonPropertiesModel mToolbarCommonPropertiesModel;
     @Mock
+    UrlBarEditingTextStateProvider mUrlBarEditingTextStateProvider;
+    @Mock
     StatusMediator.StatusMediatorDelegate mDelegate;
     @Mock
     Bitmap mBitmap;
@@ -56,7 +59,7 @@ public final class StatusMediatorUnitTest {
         MockitoAnnotations.initMocks(this);
 
         mModel = new PropertyModel(StatusProperties.ALL_KEYS);
-        mMediator = new StatusMediator(mModel, mResources);
+        mMediator = new StatusMediator(mModel, mResources, mUrlBarEditingTextStateProvider);
         mMediator.setToolbarCommonPropertiesModel(mToolbarCommonPropertiesModel);
         mMediator.setDelegateForTesting(mDelegate);
     }
@@ -123,7 +126,7 @@ public final class StatusMediatorUnitTest {
 
         mMediator.setUrlHasFocus(true);
         mMediator.setShowIconsWhenUrlFocused(true);
-        mMediator.onTextChanged("", "testing");
+        mMediator.onTextChanged(TEST_SEARCH_URL);
         Assert.assertEquals(R.drawable.ic_globe_24dp, mModel.get(StatusProperties.STATUS_ICON_RES));
     }