Move Spannable container to common class.

This change allows all suggestions to benefit from Spannable
container that introduces .equals for Spannables.

Bug: 982818
Change-Id: Ia31d7baf2fb4a97556090867b1f29e9f8fb1547b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1968115Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Brandon Wylie <wylieb@chromium.org>
Commit-Queue: Ender <ender@google.com>
Cr-Commit-Position: refs/heads/master@{#728228}

chrome/android/chrome_java_sources.gni

@@ -1179,6 +1179,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/omnibox/suggestions/base/DecoratedSuggestionView.java",
   "java/src/org/chromium/chrome/browser/omnibox/suggestions/base/SimpleHorizontalLayoutView.java",
   "java/src/org/chromium/chrome/browser/omnibox/suggestions/base/SuggestionDrawableState.java",
+  "java/src/org/chromium/chrome/browser/omnibox/suggestions/base/SuggestionSpannable.java",
   "java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/BasicSuggestionProcessor.java",
   "java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/SuggestionHost.java",
   "java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/SuggestionView.java",
@@ -1617,6 +1618,7 @@ chrome_java_sources = [
   "java/src/org/chromium/chrome/browser/tab/TabFavicon.java",
   "java/src/org/chromium/chrome/browser/tab/TabFeatureUtilities.java",
   "java/src/org/chromium/chrome/browser/tab/TabHelpers.java",
+  "java/src/org/chromium/chrome/browser/tab/TabHelpers.java",
   "java/src/org/chromium/chrome/browser/tab/TabHidingType.java",
   "java/src/org/chromium/chrome/browser/tab/TabIdManager.java",
   "java/src/org/chromium/chrome/browser/tab/TabImpl.java",

chrome/android/chrome_junit_test_java_sources.gni

@@ -162,6 +162,7 @@ chrome_junit_test_java_sources = [
   "junit/src/org/chromium/chrome/browser/omnibox/suggestions/base/BaseSuggestionViewProcessorUnitTest.java",
   "junit/src/org/chromium/chrome/browser/omnibox/suggestions/base/BaseSuggestionViewTest.java",
   "junit/src/org/chromium/chrome/browser/omnibox/suggestions/base/SimpleHorizontalLayoutViewTest.java",
+  "junit/src/org/chromium/chrome/browser/omnibox/suggestions/base/SuggestionSpannableUnitTest.java",
   "junit/src/org/chromium/chrome/browser/omnibox/suggestions/basic/BasicSuggestionProcessorTest.java",
   "junit/src/org/chromium/chrome/browser/omnibox/suggestions/editurl/EditUrlSuggestionTest.java",
   "junit/src/org/chromium/chrome/browser/omnibox/suggestions/entity/EntitySuggestionProcessorUnitTest.java",

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/base/SuggestionSpannable.java

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.omnibox.suggestions.base;
+
+import android.text.SpannableString;
+import android.text.TextUtils;
+import android.text.style.BackgroundColorSpan;
+import android.text.style.ForegroundColorSpan;
+import android.text.style.StyleSpan;
+import android.text.style.TextAppearanceSpan;
+import android.text.style.UpdateAppearance;
+
+/**
+ * SpannableString that supplies .equals method for content comparison.
+ * TODO(ender): This code assumes the use of a limited subset of span types in suggestion text
+ * and may need revisiting if/when the scope of these types expands (eg. different colors are
+ * introduced). With the limited set of Span features we use this code is working well.
+ * When this is done, consider sharing this with UrlBarMediator#isNewTextEquivalentToExitingText.
+ */
+public class SuggestionSpannable extends SpannableString {
+    public SuggestionSpannable(CharSequence text) {
+        super(text);
+        assert text != null;
+    }
+
+    /**
+     * Custom equals method that addresses some of the issues found in SpannableStringInternal:
+     * 1. getSpan may reorder returned spans that was not reflected in original code until
+     *    http://b2/73359036 (http://shortn/_vKcawFBIoL)
+     * 2. Individual Span objects still do not necessarily override .equals themselves
+     *    (eg. ForegroundColorSpan), making them always fail the comparison.
+     *
+     * This code simply addresses some issues in SpannableString.equals() method, therefore does
+     * not come paired with hashCode() call. hashCode() is correctly supplied by SpannableString.
+     */
+    @Override
+    public final boolean equals(Object obj) {
+        if (!(obj instanceof SuggestionSpannable)) return false;
+
+        final SuggestionSpannable other = (SuggestionSpannable) obj;
+        if (!TextUtils.equals(this, other)) return false;
+        if (TextUtils.isEmpty(this)) return true;
+
+        final UpdateAppearance[] thisSpans = getSpans(0, length(), UpdateAppearance.class);
+        final UpdateAppearance[] otherSpans =
+                other.getSpans(0, other.length(), UpdateAppearance.class);
+
+        if (thisSpans.length != otherSpans.length) return false;
+
+        for (int i = 0; i < thisSpans.length; i++) {
+            final UpdateAppearance thisSpan = thisSpans[i];
+            final UpdateAppearance otherSpan = otherSpans[i];
+
+            if (getSpanStart(thisSpan) != other.getSpanStart(otherSpan)
+                    || getSpanEnd(thisSpan) != other.getSpanEnd(otherSpan)
+                    || getSpanFlags(thisSpan) != other.getSpanFlags(otherSpan)
+                    || thisSpan.getClass() != otherSpan.getClass()) {
+                return false;
+            }
+
+            if (thisSpan instanceof ForegroundColorSpan) {
+                ForegroundColorSpan ta1 = (ForegroundColorSpan) thisSpan;
+                ForegroundColorSpan ta2 = (ForegroundColorSpan) otherSpan;
+                if (ta1.getForegroundColor() != ta2.getForegroundColor()) {
+                    return false;
+                }
+            } else if (thisSpan instanceof BackgroundColorSpan) {
+                BackgroundColorSpan ta1 = (BackgroundColorSpan) thisSpan;
+                BackgroundColorSpan ta2 = (BackgroundColorSpan) otherSpan;
+                if (ta1.getBackgroundColor() != ta2.getBackgroundColor()) {
+                    return false;
+                }
+            } else if (thisSpan instanceof StyleSpan) {
+                StyleSpan ta1 = (StyleSpan) thisSpan;
+                StyleSpan ta2 = (StyleSpan) otherSpan;
+                if (ta1.getStyle() != ta2.getStyle()) {
+                    return false;
+                }
+            } else if (thisSpan instanceof TextAppearanceSpan) {
+                TextAppearanceSpan ta1 = (TextAppearanceSpan) thisSpan;
+                TextAppearanceSpan ta2 = (TextAppearanceSpan) otherSpan;
+                if (!TextUtils.equals(ta1.getFamily(), ta2.getFamily())
+                        || !ta1.getLinkTextColor().equals(ta2.getLinkTextColor())
+                        || !ta1.getTextColor().equals(ta2.getTextColor())
+                        || ta1.getTextSize() != ta2.getTextSize()
+                        || ta1.getTextStyle() != ta2.getTextStyle()) {
+                    return false;
+                }
+            } else {
+                assert false : "Unexpected visual span specified: " + thisSpan.getClass();
+                return false;
+            }
+        }
+        return true;
+    }
+}

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/BasicSuggestionProcessor.java

@@ -7,8 +7,6 @@ package org.chromium.chrome.browser.omnibox.suggestions.basic;
 import android.content.Context;
 import android.graphics.Bitmap;
 import android.support.annotation.DrawableRes;
-import android.text.Spannable;
-import android.text.SpannableString;
 import android.text.TextUtils;
 
 import org.chromium.base.Supplier;
@@ -23,8 +21,8 @@ import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
 import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionUiType;
 import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewProcessor;
 import org.chromium.chrome.browser.omnibox.suggestions.base.SuggestionDrawableState;
+import org.chromium.chrome.browser.omnibox.suggestions.base.SuggestionSpannable;
 import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewProperties.SuggestionIcon;
-import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewProperties.SuggestionTextContainer;
 import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.ui.modelutil.PropertyModel;
 
@@ -193,21 +191,21 @@ public class BasicSuggestionProcessor extends BaseSuggestionViewProcessor {
     public void populateModel(OmniboxSuggestion suggestion, PropertyModel model, int position) {
         super.populateModel(suggestion, model, position);
         final @OmniboxSuggestionType int suggestionType = suggestion.getType();
-        Spannable textLine2 = null;
+        SuggestionSpannable textLine2 = null;
         boolean urlHighlighted = false;
 
         if (suggestion.isUrlSuggestion()) {
             if (!TextUtils.isEmpty(suggestion.getUrl())) {
-                Spannable str = SpannableString.valueOf(suggestion.getDisplayText());
+                SuggestionSpannable str = new SuggestionSpannable(suggestion.getDisplayText());
                 urlHighlighted = applyHighlightToMatchRegions(
                         str, suggestion.getDisplayTextClassifications());
                 textLine2 = str;
             }
         } else if (suggestionType == OmniboxSuggestionType.SEARCH_SUGGEST_PROFILE) {
-            textLine2 = SpannableString.valueOf(suggestion.getDescription());
+            textLine2 = new SuggestionSpannable(suggestion.getDescription());
         }
 
-        final Spannable textLine1 =
+        final SuggestionSpannable textLine1 =
                 getSuggestedQuery(suggestion, suggestion.isUrlSuggestion(), !urlHighlighted);
 
         updateSuggestionIcon(suggestion, model);
@@ -216,10 +214,8 @@ public class BasicSuggestionProcessor extends BaseSuggestionViewProcessor {
                         || suggestionType == OmniboxSuggestionType.CLIPBOARD_IMAGE
                         || suggestionType == OmniboxSuggestionType.CLIPBOARD_TEXT);
         model.set(SuggestionViewProperties.SUGGESTION_TYPE, suggestion.getType());
-        model.set(
-                SuggestionViewProperties.TEXT_LINE_1_TEXT, new SuggestionTextContainer(textLine1));
-        model.set(
-                SuggestionViewProperties.TEXT_LINE_2_TEXT, new SuggestionTextContainer(textLine2));
+        model.set(SuggestionViewProperties.TEXT_LINE_1_TEXT, textLine1);
+        model.set(SuggestionViewProperties.TEXT_LINE_2_TEXT, textLine2);
         fetchSuggestionFavicon(model, suggestion.getUrl(), suggestion.getType());
     }
 
@@ -263,7 +259,7 @@ public class BasicSuggestionProcessor extends BaseSuggestionViewProcessor {
      * @param shouldHighlight Whether the query should be highlighted.
      * @return The first line of text.
      */
-    private Spannable getSuggestedQuery(OmniboxSuggestion suggestion,
+    private SuggestionSpannable getSuggestedQuery(OmniboxSuggestion suggestion,
             boolean showDescriptionIfPresent, boolean shouldHighlight) {
         String userQuery = mUrlBarEditingTextProvider.getTextWithoutAutocomplete();
         String suggestedQuery = null;
@@ -284,7 +280,7 @@ public class BasicSuggestionProcessor extends BaseSuggestionViewProcessor {
                     new OmniboxSuggestion.MatchClassification(0, MatchClassificationStyle.NONE));
         }
 
-        Spannable str = SpannableString.valueOf(suggestedQuery);
+        SuggestionSpannable str = new SuggestionSpannable(suggestedQuery);
         if (shouldHighlight) applyHighlightToMatchRegions(str, classifications);
         return str;
     }

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/SuggestionViewProperties.java

@@ -4,13 +4,10 @@
 
 package org.chromium.chrome.browser.omnibox.suggestions.basic;
 
-import android.text.Spannable;
-import android.text.TextUtils;
-import android.text.style.UpdateAppearance;
-
 import androidx.annotation.IntDef;
 
 import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewProperties;
+import org.chromium.chrome.browser.omnibox.suggestions.base.SuggestionSpannable;
 import org.chromium.ui.modelutil.PropertyKey;
 import org.chromium.ui.modelutil.PropertyModel;
 import org.chromium.ui.modelutil.PropertyModel.WritableBooleanPropertyKey;
@@ -42,56 +39,6 @@ public class SuggestionViewProperties {
         int TOTAL_COUNT = 8;
     }
 
-    /**
-     * Container for suggestion text that prevents updates when the text/spans has not changed.
-     */
-    public static class SuggestionTextContainer {
-        public final Spannable text;
-
-        public SuggestionTextContainer(Spannable text) {
-            this.text = text;
-        }
-
-        @Override
-        public String toString() {
-            return "SuggestionTextContainer: " + (text == null ? "null" : text.toString());
-        }
-
-        @Override
-        public boolean equals(Object obj) {
-            if (!(obj instanceof SuggestionTextContainer)) return false;
-            SuggestionTextContainer other = (SuggestionTextContainer) obj;
-            if (!TextUtils.equals(text, other.text)) return false;
-            if (text == null) return false;
-
-            UpdateAppearance[] thisSpans = text.getSpans(0, text.length(), UpdateAppearance.class);
-            UpdateAppearance[] otherSpans =
-                    other.text.getSpans(0, other.text.length(), UpdateAppearance.class);
-            if (thisSpans.length != otherSpans.length) return false;
-            for (int i = 0; i < thisSpans.length; i++) {
-                UpdateAppearance thisSpan = thisSpans[i];
-                UpdateAppearance otherSpan = otherSpans[i];
-                if (!thisSpan.getClass().equals(otherSpan.getClass())) return false;
-
-                if (text.getSpanStart(thisSpan) != other.text.getSpanStart(otherSpan)
-                        || text.getSpanEnd(thisSpan) != other.text.getSpanEnd(otherSpan)
-                        || text.getSpanFlags(thisSpan) != other.text.getSpanFlags(otherSpan)) {
-                    return false;
-                }
-
-                // TODO(tedchoc): This is a dangerous assumption.  We should actually update all
-                //                span types we use in suggestion text to implement .equals and
-                //                ensure the internal styles (e.g. color used in a foreground span)
-                //                is actually the same.  This "seems" safe for now, but not
-                //                particularly robust.
-                //
-                //                Once that happens, share this logic with
-                //                UrlBarMediator#isNewTextEquivalentToExistingText.
-            }
-            return true;
-        }
-    }
-
     /** @see OmniboxSuggestionType */
     public static final WritableIntPropertyKey SUGGESTION_TYPE = new WritableIntPropertyKey();
 
@@ -103,11 +50,11 @@ public class SuggestionViewProperties {
             new WritableBooleanPropertyKey();
 
     /** The actual text content for the first line of text. */
-    public static final WritableObjectPropertyKey<SuggestionTextContainer> TEXT_LINE_1_TEXT =
+    public static final WritableObjectPropertyKey<SuggestionSpannable> TEXT_LINE_1_TEXT =
             new WritableObjectPropertyKey<>();
 
     /** The actual text content for the second line of text. */
-    public static final WritableObjectPropertyKey<SuggestionTextContainer> TEXT_LINE_2_TEXT =
+    public static final WritableObjectPropertyKey<SuggestionSpannable> TEXT_LINE_2_TEXT =
             new WritableObjectPropertyKey<>();
 
     public static final PropertyKey[] ALL_UNIQUE_KEYS = new PropertyKey[] {SUGGESTION_TYPE,

chrome/android/java/src/org/chromium/chrome/browser/omnibox/suggestions/basic/SuggestionViewViewBinder.java

@@ -5,7 +5,6 @@
 package org.chromium.chrome.browser.omnibox.suggestions.basic;
 
 import android.support.annotation.ColorRes;
-import android.text.Spannable;
 import android.text.TextUtils;
 import android.view.View;
 import android.widget.TextView;
@@ -17,7 +16,7 @@ import org.chromium.chrome.browser.omnibox.suggestions.SuggestionCommonPropertie
 import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionView;
 import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewBinder;
 import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewProperties;
-import org.chromium.chrome.browser.omnibox.suggestions.basic.SuggestionViewProperties.SuggestionTextContainer;
+import org.chromium.chrome.browser.omnibox.suggestions.base.SuggestionSpannable;
 import org.chromium.ui.modelutil.PropertyKey;
 import org.chromium.ui.modelutil.PropertyModel;
 
@@ -33,10 +32,7 @@ public class SuggestionViewViewBinder extends BaseSuggestionViewBinder {
             content.setDelegate(model.get(BaseSuggestionViewProperties.SUGGESTION_DELEGATE));
         } else if (propertyKey == SuggestionViewProperties.TEXT_LINE_1_TEXT) {
             TextView tv = view.findContentView(R.id.line_1);
-            final SuggestionTextContainer container =
-                    model.get(SuggestionViewProperties.TEXT_LINE_1_TEXT);
-            final Spannable span = container != null ? container.text : null;
-            tv.setText(span);
+            tv.setText(model.get(SuggestionViewProperties.TEXT_LINE_1_TEXT));
             // Force layout to ensure infinite suggest aligns correctly.
             // The view may be re-used and may not require re-positioning by itself.
             // We want to make sure that regardless whether it's a regular text suggestion or
@@ -68,9 +64,7 @@ public class SuggestionViewViewBinder extends BaseSuggestionViewBinder {
                     isSearch ? TextView.TEXT_DIRECTION_INHERIT : TextView.TEXT_DIRECTION_LTR);
         } else if (propertyKey == SuggestionViewProperties.TEXT_LINE_2_TEXT) {
             TextView tv = view.findContentView(R.id.line_2);
-            final SuggestionTextContainer container =
-                    model.get(SuggestionViewProperties.TEXT_LINE_2_TEXT);
-            final Spannable span = container != null ? container.text : null;
+            final SuggestionSpannable span = model.get(SuggestionViewProperties.TEXT_LINE_2_TEXT);
             if (!TextUtils.isEmpty(span)) {
                 tv.setText(span);
                 tv.setVisibility(View.VISIBLE);