Faster and simpler replace in SSB, take two

Gilles Debunne · Gilles Debunne · commit 0249b43f6ce5 · 2012-04-09T16:08:37.000-07:00
This is a new version of CL 179343 which had to be reverted.

This problem of the previous CL is that the ComposingSpan that
was part of the replacement text was correctly added during the
replace but was immediately removed because it had a zero-length
size.

Swapping the add and remove blocks solves the problem.

The new non-zero length enforcement also revealed a bug in the
spell checker where we were creating useless range spans.

Change-Id: I59cebd4708af3becc7ab625ae41bc36837f1a1cf
diff --git a/core/java/android/text/SpannableStringBuilder.java b/core/java/android/text/SpannableStringBuilder.java
@@ -50,6 +50,8 @@ public SpannableStringBuilder(CharSequence text) {
     public SpannableStringBuilder(CharSequence text, int start, int end) {
         int srclen = end - start;
 
+        if (srclen < 0) throw new StringIndexOutOfBoundsException();
+
         int len = ArrayUtils.idealCharArraySize(srclen + 1);
         mText = new char[len];
         mGapStart = srclen;
@@ -87,7 +89,7 @@ public SpannableStringBuilder(CharSequence text, int start, int end) {
                 if (en > end - start)
                     en = end - start;
 
-                setSpan(spans[i], st, en, fl);
+                setSpan(false, spans[i], st, en, fl);
             }
         }
     }
@@ -149,7 +151,7 @@ private void moveGapTo(int where) {
         if (where == mGapStart)
             return;
 
-        boolean atend = (where == length());
+        boolean atEnd = (where == length());
 
         if (where < mGapStart) {
             int overlap = mGapStart - where;
@@ -171,7 +173,7 @@ private void moveGapTo(int where) {
             else if (start == where) {
                 int flag = (mSpanFlags[i] & START_MASK) >> START_SHIFT;
 
-                if (flag == POINT || (atend && flag == PARAGRAPH))
+                if (flag == POINT || (atEnd && flag == PARAGRAPH))
                     start += mGapLength;
             }
 
@@ -182,7 +184,7 @@ else if (start == where) {
             else if (end == where) {
                 int flag = (mSpanFlags[i] & END_MASK);
 
-                if (flag == POINT || (atend && flag == PARAGRAPH))
+                if (flag == POINT || (atEnd && flag == PARAGRAPH))
                     end += mGapLength;
             }
 
@@ -284,7 +286,7 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
                 }
 
                 if (st != ost || en != oen)
-                    setSpan(mSpans[i], st, en, mSpanFlags[i]);
+                    setSpan(false, mSpans[i], st, en, mSpanFlags[i]);
             }
         }
 
@@ -305,28 +307,6 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
 
         TextUtils.getChars(tb, tbstart, tbend, mText, start);
 
-        if (tb instanceof Spanned) {
-            Spanned sp = (Spanned) tb;
-            Object[] spans = sp.getSpans(tbstart, tbend, Object.class);
-
-            for (int i = 0; i < spans.length; i++) {
-                int st = sp.getSpanStart(spans[i]);
-                int en = sp.getSpanEnd(spans[i]);
-
-                if (st < tbstart)
-                    st = tbstart;
-                if (en > tbend)
-                    en = tbend;
-
-                if (getSpanStart(spans[i]) < 0) {
-                    setSpan(false, spans[i],
-                            st - tbstart + start,
-                            en - tbstart + start,
-                            sp.getSpanFlags(spans[i]));
-                }
-            }
-        }
-
         if (end > start) {
             // no need for span fixup on pure insertion
             boolean atEnd = (mGapStart + mGapLength == mText.length);
@@ -358,6 +338,25 @@ private void change(int start, int end, CharSequence tb, int tbstart, int tbend)
                 }
             }
         }
+
+        if (tb instanceof Spanned) {
+            Spanned sp = (Spanned) tb;
+            Object[] spans = sp.getSpans(tbstart, tbend, Object.class);
+
+            for (int i = 0; i < spans.length; i++) {
+                int st = sp.getSpanStart(spans[i]);
+                int en = sp.getSpanEnd(spans[i]);
+
+                if (st < tbstart) st = tbstart;
+                if (en > tbend) en = tbend;
+
+                // Add span only if this object is not yet used as a span in this string
+                if (getSpanStart(spans[i]) < 0) {
+                        setSpan(false, spans[i], st - tbstart + start, en - tbstart + start,
+                                sp.getSpanFlags(spans[i]));
+                }
+            }
+        }
     }
 
     private void removeSpan(int i) {
@@ -389,7 +388,7 @@ public SpannableStringBuilder replace(int start, int end, CharSequence tb) {
 
     // Documentation from interface
     public SpannableStringBuilder replace(final int start, final int end,
-                        CharSequence tb, int tbstart, int tbend) {
+            CharSequence tb, int tbstart, int tbend) {
         int filtercount = mFilters.length;
         for (int i = 0; i < filtercount; i++) {
             CharSequence repl = mFilters[i].filter(tb, tbstart, tbend, this, start, end);
@@ -411,53 +410,26 @@ public SpannableStringBuilder replace(final int start, final int end,
         TextWatcher[] textWatchers = getSpans(start, start + origLen, TextWatcher.class);
         sendBeforeTextChanged(textWatchers, start, origLen, newLen);
 
-        if (origLen == 0 || newLen == 0) {
-            change(start, end, tb, tbstart, tbend);
-        } else {
-            int selstart = Selection.getSelectionStart(this);
-            int selend = Selection.getSelectionEnd(this);
-
-            // XXX just make the span fixups in change() do the right thing
-            // instead of this madness!
-
-            checkRange("replace", start, end);
-            moveGapTo(end);
-
-            if (mGapLength < 2)
-                resizeFor(length() + 1);
-
-            for (int i = mSpanCount - 1; i >= 0; i--) {
-                if (mSpanStarts[i] == mGapStart)
-                    mSpanStarts[i]++;
+        // Try to keep the cursor / selection at the same relative position during
+        // a text replacement. If replaced or replacement text length is zero, this
+        // is already taken care of.
+        boolean adjustSelection = origLen != 0 && newLen != 0;
+        int selstart = 0;
+        int selend = 0;
+        if (adjustSelection) {
+            selstart = Selection.getSelectionStart(this);
+            selend = Selection.getSelectionEnd(this);
+        }
 
-                if (mSpanEnds[i] == mGapStart)
-                    mSpanEnds[i]++;
-            }
+        checkRange("replace", start, end);
 
-            mText[mGapStart] = ' ';
-            mGapStart++;
-            mGapLength--;
+        change(start, end, tb, tbstart, tbend);
 
-            if (mGapLength < 1) {
-                new Exception("mGapLength < 1").printStackTrace();
-            }
-
-            change(start + 1, start + 1, tb, tbstart, tbend);
-            change(start, start + 1, "", 0, 0);
-            change(start + newLen, start + newLen + origLen, "", 0, 0);
-
-            /*
-             * Special case to keep the cursor in the same position
-             * if it was somewhere in the middle of the replaced region.
-             * If it was at the start or the end or crossing the whole
-             * replacement, it should already be where it belongs.
-             * TODO: Is there some more general mechanism that could
-             * accomplish this?
-             */
+        if (adjustSelection) {
             if (selstart > start && selstart < end) {
                 long off = selstart - start;
 
-                off = off * newLen / (end - start);
+                off = off * newLen / origLen;
                 selstart = (int) off + start;
 
                 setSpan(false, Selection.SELECTION_START, selstart, selstart,
@@ -466,7 +438,7 @@ public SpannableStringBuilder replace(final int start, final int end,
             if (selend > start && selend < end) {
                 long off = selend - start;
 
-                off = off * newLen / (end - start);
+                off = off * newLen / origLen;
                 selend = (int) off + start;
 
                 setSpan(false, Selection.SELECTION_END, selend, selend, Spanned.SPAN_POINT_POINT);
@@ -489,12 +461,10 @@ public void setSpan(Object what, int start, int end, int flags) {
     }
 
     private void setSpan(boolean send, Object what, int start, int end, int flags) {
-        int nstart = start;
-        int nend = end;
-
         checkRange("setSpan", start, end);
 
-        if ((flags & START_MASK) == (PARAGRAPH << START_SHIFT)) {
+        int flagsStart = (flags & START_MASK) >> START_SHIFT;
+        if (flagsStart == PARAGRAPH) {
             if (start != 0 && start != length()) {
                 char c = charAt(start - 1);
 
@@ -503,7 +473,8 @@ private void setSpan(boolean send, Object what, int start, int end, int flags) {
             }
         }
 
-        if ((flags & END_MASK) == PARAGRAPH) {
+        int flagsEnd = flags & END_MASK;
+        if (flagsEnd == PARAGRAPH) {
             if (end != 0 && end != length()) {
                 char c = charAt(end - 1);
 
@@ -512,26 +483,33 @@ private void setSpan(boolean send, Object what, int start, int end, int flags) {
             }
         }
 
-        if (flags == Spanned.SPAN_EXCLUSIVE_EXCLUSIVE && start == end) {
-            throw new IllegalArgumentException(
-                    "SPAN_EXCLUSIVE_EXCLUSIVE spans cannot have a zero length");
+        // 0-length Spanned.SPAN_EXCLUSIVE_EXCLUSIVE
+        if (flagsStart == POINT && flagsEnd == MARK && start == end) {
+            if (send) {
+                throw new IllegalArgumentException(
+                        "SPAN_EXCLUSIVE_EXCLUSIVE spans cannot have a zero length");
+            } else {
+                // Silently ignore invalid spans when they are created from this class.
+                // This avoids the duplication of the above test code before all the
+                // calls to setSpan that are done in this class
+                return;
+            }
         }
 
+        int nstart = start;
+        int nend = end;
+
         if (start > mGapStart) {
             start += mGapLength;
         } else if (start == mGapStart) {
-            int flag = (flags & START_MASK) >> START_SHIFT;
-
-            if (flag == POINT || (flag == PARAGRAPH && start == length()))
+            if (flagsStart == POINT || (flagsStart == PARAGRAPH && start == length()))
                 start += mGapLength;
         }
 
         if (end > mGapStart) {
             end += mGapLength;
         } else if (end == mGapStart) {
-            int flag = (flags & END_MASK);
-
-            if (flag == POINT || (flag == PARAGRAPH && end == length()))
+            if (flagsEnd == POINT || (flagsEnd == PARAGRAPH && end == length()))
                 end += mGapLength;
         }
 
@@ -1231,6 +1209,7 @@ public InputFilter[] getFilters() {
     private int mSpanCount;
 
     // TODO These value are tightly related to the public SPAN_MARK/POINT values in {@link Spanned}
+    private static final int MARK = 1;
     private static final int POINT = 2;
     private static final int PARAGRAPH = 3;
 
diff --git a/core/java/android/widget/SpellChecker.java b/core/java/android/widget/SpellChecker.java
@@ -227,8 +227,7 @@ public void spellCheck(int start, int end) {
         for (int i = 0; i < length; i++) {
             final SpellParser spellParser = mSpellParsers[i];
             if (spellParser.isFinished()) {
-                spellParser.init(start, end);
-                spellParser.parse();
+                spellParser.parse(start, end);
                 return;
             }
         }
@@ -240,8 +239,7 @@ public void spellCheck(int start, int end) {
 
         SpellParser spellParser = new SpellParser();
         mSpellParsers[length] = spellParser;
-        spellParser.init(start, end);
-        spellParser.parse();
+        spellParser.parse(start, end);
     }
 
     private void spellCheck() {
@@ -421,8 +419,11 @@ private void createMisspelledSuggestionSpan(Editable editable, SuggestionsInfo s
     private class SpellParser {
         private Object mRange = new Object();
 
-        public void init(int start, int end) {
-            setRangeSpan((Editable) mTextView.getText(), start, end);
+        public void parse(int start, int end) {
+            if (end > start) {
+                setRangeSpan((Editable) mTextView.getText(), start, end);
+                parse();
+            }
         }
 
         public boolean isFinished() {
