[Autofill Assistant] Prevent crash for mismatching link tags.

Before this change, texts like "<link0>Click</link1>" would hard-crash
with "java.lang.IllegalArgumentException: Input string is missing tags
<link0></link0>".

Now such mistakes are ignored.

Bug: b/147482903
Change-Id: I8eae2b57d6b1c2141b2d5870ffea5c0c6ba92aa2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2000236
Commit-Queue: Sandro Maggi <sandromaggi@google.com>
Reviewed-by: Mathias Carlen <mcarlen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#732353}

chrome/android/features/autofill_assistant/java/src/org/chromium/chrome/browser/autofill_assistant/AssistantTextUtils.java

@@ -12,6 +12,7 @@ import android.widget.TextView;
 import androidx.annotation.Nullable;
 
 import org.chromium.base.Callback;
+import org.chromium.base.Log;
 import org.chromium.ui.text.NoUnderlineClickableSpan;
 import org.chromium.ui.text.SpanApplier;
 
@@ -26,6 +27,8 @@ import java.util.regex.Pattern;
  * Common text utilities used by autofill assistant.
  */
 public class AssistantTextUtils {
+    private static final String TAG = "AssistantTextUtils";
+
     /** Bold tags of the form <b>...</b>. */
     private static final SpanApplier.SpanInfo BOLD_SPAN_INFO =
             new SpanApplier.SpanInfo("<b>", "</b>", new StyleSpan(android.graphics.Typeface.BOLD));
@@ -33,7 +36,7 @@ public class AssistantTextUtils {
     private static final SpanApplier.SpanInfo ITALIC_SPAN_INFO =
             new SpanApplier.SpanInfo("<i>", "</i>", new StyleSpan(Typeface.ITALIC));
     /** Links of the form <link0>...</link0>. */
-    private static final Pattern LINK_PATTERN = Pattern.compile("<link(\\d+)>");
+    private static final Pattern LINK_START_PATTERN = Pattern.compile("<link(\\d+)>");
 
     /**
      * Sets the text of {@code view} by automatically applying all special appearance tags in {@code
@@ -54,7 +57,7 @@ public class AssistantTextUtils {
 
         // We first collect the link IDs into a set to allow multiple links with same ID.
         Set<Integer> linkIds = new HashSet<>();
-        Matcher matcher = LINK_PATTERN.matcher(text);
+        Matcher matcher = LINK_START_PATTERN.matcher(text);
         while (matcher.find()) {
             linkIds.add(Integer.parseInt(matcher.group(1)));
         }
@@ -68,8 +71,20 @@ public class AssistantTextUtils {
                     })));
         }
 
+        List<SpanApplier.SpanInfo> successfulSpans = new ArrayList<>();
+        for (SpanApplier.SpanInfo info : spans) {
+            try {
+                SpanApplier.applySpans(text, info);
+
+                // Apply successful. Add the info to the list.
+                successfulSpans.add(info);
+            } catch (IllegalArgumentException e) {
+                Log.d(TAG, "Mismatching span", e);
+            }
+        }
+
         view.setText(SpanApplier.applySpans(
-                text, spans.toArray(new SpanApplier.SpanInfo[spans.size()])));
+                text, successfulSpans.toArray(new SpanApplier.SpanInfo[successfulSpans.size()])));
         view.setMovementMethod(linkIds.isEmpty() ? null : LinkMovementMethod.getInstance());
     }
 }

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantTextUtilsTest.java

@@ -41,6 +41,20 @@ public class AutofillAssistantTextUtilsTest {
     @Rule
     public CustomTabActivityTestRule mTestRule = new CustomTabActivityTestRule();
 
+    /* Simple helper class that keeps track of the most recent callback result. */
+    class LinkCallback implements Callback<Integer> {
+        private int mLastCallback = -1;
+
+        private int getLastCallback() {
+            return mLastCallback;
+        }
+
+        @Override
+        public void onResult(Integer result) {
+            mLastCallback = result;
+        }
+    }
+
     @Before
     public void setUp() {
         AutofillAssistantUiTestUtil.startOnBlankPage(mTestRule);
@@ -92,20 +106,26 @@ public class AutofillAssistantTextUtilsTest {
 
     @Test
     @MediumTest
-    public void testTextLinks() throws Exception {
-        /* Simple helper class that keeps track of the most recent callback result. */
-        class LinkCallback implements Callback<Integer> {
-            private int mLastCallback = -1;
-
-            private int getLastCallback() {
-                return mLastCallback;
-            }
-            @Override
-            public void onResult(Integer result) {
-                mLastCallback = result;
-            }
-        }
+    public void testMismatchingTextTags() throws Exception {
+        TextView textView = runOnUiThreadBlocking(() -> {
+            TextView view = new TextView(mTestRule.getActivity());
+            mTestLayout.addView(view);
+            AssistantTextUtils.applyVisualAppearanceTags(
+                    view, "<b>Fat</b>. <b>Not fat</br>. <i>Italic</i>. <i>Not italic</ii>.", null);
+            StyleSpan[] spans =
+                    ((SpannedString) view.getText()).getSpans(0, view.length(), StyleSpan.class);
+            assertThat(spans.length, is(2));
+            assertThat(spans[0].getStyle(), is(Typeface.BOLD));
+            assertThat(spans[1].getStyle(), is(Typeface.ITALIC));
+            return view;
+        });
+        onView(is(textView))
+                .check(matches(withText("Fat. <b>Not fat</br>. Italic. <i>Not italic</ii>.")));
+    }
 
+    @Test
+    @MediumTest
+    public void testTextLinks() throws Exception {
         LinkCallback linkCallback = new LinkCallback();
         TextView multiLinkView = runOnUiThreadBlocking(() -> {
             TextView view = new TextView(mTestRule.getActivity());
@@ -125,4 +145,25 @@ public class AutofillAssistantTextUtilsTest {
         onView(is(multiLinkView)).perform(openTextLink("there"));
         assertThat(linkCallback.getLastCallback(), is(2));
     }
+
+    @Test
+    @MediumTest
+    public void testMismatchingTextLinks() throws Exception {
+        LinkCallback linkCallback = new LinkCallback();
+
+        TextView singleLinkView = runOnUiThreadBlocking(() -> {
+            TextView view = new TextView(mTestRule.getActivity());
+            mTestLayout.addView(view);
+            AssistantTextUtils.applyVisualAppearanceTags(view,
+                    "Don't click <link0>here</link1> or <link2>this</lin2>, "
+                            + "click <link3>me</link3>.",
+                    linkCallback);
+            return view;
+        });
+        onView(is(singleLinkView))
+                .check(matches(withText(
+                        "Don't click <link0>here</link1> or <link2>this</lin2>, click me.")));
+        onView(is(singleLinkView)).perform(openTextLink("me"));
+        assertThat(linkCallback.getLastCallback(), is(3));
+    }
 }
\ No newline at end of file

chrome/android/features/autofill_assistant/javatests/src/org/chromium/chrome/browser/autofill_assistant/AutofillAssistantUiTestUtil.java

@@ -25,7 +25,7 @@ import android.support.test.espresso.ViewAction;
 import android.support.test.espresso.ViewAssertion;
 import android.support.test.espresso.core.deps.guava.base.Preconditions;
 import android.support.test.espresso.matcher.BoundedMatcher;
-import android.text.SpannableString;
+import android.text.Spanned;
 import android.text.style.ClickableSpan;
 import android.view.Gravity;
 import android.view.LayoutInflater;
@@ -235,13 +235,13 @@ class AutofillAssistantUiTestUtil {
             @Override
             public void perform(UiController uiController, View view) {
                 TextView textView = (TextView) view;
-                SpannableString spannableString = (SpannableString) textView.getText();
+                Spanned spannedString = (Spanned) textView.getText();
                 ClickableSpan[] spans =
-                        spannableString.getSpans(0, spannableString.length(), ClickableSpan.class);
+                        spannedString.getSpans(0, spannedString.length(), ClickableSpan.class);
                 for (ClickableSpan span : spans) {
                     if (textLink.contentEquals(
-                                spannableString.subSequence(spannableString.getSpanStart(span),
-                                        spannableString.getSpanEnd(span)))) {
+                                spannedString.subSequence(spannedString.getSpanStart(span),
+                                        spannedString.getSpanEnd(span)))) {
                         span.onClick(view);
                         return;
                     }