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

Commit 54716a54 authored by Haoyu Zhang's avatar Haoyu Zhang
Browse files

Fix insert mode crash when text has UpdateLayout span 

Previously, when text has UpdateLayout span and OffsetMapping(typically used by insert mode) the DynamicLayout reflow the wrong range when the span is updated. As a result, DynamicLayout will enter an inconsistent state where getLineStart/End() will return the wrong result. This CL fixed the issue by applying the reflow correctly.

Bug: 355137282
Test: DynamicLayoutOffsetMappingTest
Flag: com.android.text.flags.insert_mode_crash_update_layout_span
Change-Id: I5bd3980ff3adf22f1ff843941e303f540dac730d
parent 22bba40d

core/java/android/text/DynamicLayout.java

+10 −2
Original line number Diff line number Diff line
@@ -1320,7 +1320,11 @@ public class DynamicLayout extends Layout { 
                        // It's possible that a Span is removed when the text covering it is

                        // deleted, in this case, the original start and end of the span might be

                        // OOB. So it'll reflow the entire string instead.

                        if (Flags.insertModeCrashUpdateLayoutSpan()) {

                            transformAndReflow(s, 0, s.length());

                        } else {

                            reflow(s, 0, 0, s.length());

                        }

                    } else {

                        reflow(s, start, end - start, end - start);

                    }
@@ -1343,7 +1347,11 @@ public class DynamicLayout extends Layout { 
                        // When text is changed, it'll also trigger onSpanChanged. In this case we

                        // can't determine the updated range in the transformed text. So it'll

                        // reflow the entire range instead.

                        if (Flags.insertModeCrashUpdateLayoutSpan()) {

                            transformAndReflow(s, 0, s.length());

                        } else {

                            reflow(s, 0, 0, s.length());

                        }

                    } else {

                        reflow(s, start, end - start, end - start);

                        reflow(s, nstart, nend - nstart, nend - nstart);

core/java/android/text/flags/flags.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -124,6 +124,16 @@ flag { 
  bug: "300850862"

}



flag {

  name: "insert_mode_crash_update_layout_span"

  namespace: "text"

  description: "Fix insert mode crash when the text has UpdateLayout span attached."

  bug: "355137282"

  metadata {

    purpose: PURPOSE_BUGFIX

  }

}



flag {

  name: "icu_bidi_migration"

  namespace: "text"

core/tests/coretests/src/android/text/DynamicLayoutOffsetMappingTest.java

+88 −0
Original line number Diff line number Diff line
@@ -21,11 +21,18 @@ import static android.text.Layout.Alignment.ALIGN_NORMAL; 
import static com.google.common.truth.Truth.assertThat;



import android.platform.test.annotations.Presubmit;

import android.platform.test.annotations.RequiresFlagsEnabled;

import android.platform.test.flag.junit.CheckFlagsRule;

import android.platform.test.flag.junit.DeviceFlagsValueProvider;

import android.text.method.OffsetMapping;

import android.text.style.UpdateLayout;



import androidx.test.filters.SmallTest;

import androidx.test.runner.AndroidJUnit4;



import com.android.text.flags.Flags;



import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -36,6 +43,9 @@ public class DynamicLayoutOffsetMappingTest { 
    private static final int WIDTH = 10000;

    private static final TextPaint sTextPaint = new TextPaint();



    @Rule

    public CheckFlagsRule mCheckFlagsRule = DeviceFlagsValueProvider.createCheckFlagsRule();



    @Test

    public void textWithOffsetMapping() {

        final String text = "abcde";
@@ -119,6 +129,84 @@ public class DynamicLayoutOffsetMappingTest { 
        assertLineRange(layout, /* lineBreaks */ 0, 5, 6, 8);

    }



    @Test

    @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_CRASH_UPDATE_LAYOUT_SPAN)

    public void textWithOffsetMapping_deletion_withUpdateLayoutSpan() {

        final String text = "abcdef";

        final SpannableStringBuilder spannable = new SpannableStringBuilder(text);

        // UpdateLayout span covers the letter 'd'.

        spannable.setSpan(new UpdateLayout() {}, 3, 4, Spanned.SPAN_INCLUSIVE_EXCLUSIVE);



        final CharSequence transformedText =

                new TestOffsetMapping(spannable, 3, "\n\n");



        final DynamicLayout layout = DynamicLayout.Builder.obtain(spannable, sTextPaint, WIDTH)

                .setAlignment(ALIGN_NORMAL)

                .setIncludePad(false)

                .setDisplayText(transformedText)

                .build();



        // delete character 'c', original text becomes "abdef"

        spannable.delete(2, 3);

        assertThat(transformedText.toString()).isEqualTo("ab\n\ndef");

        assertLineRange(layout, /* lineBreaks */ 0, 3, 4, 7);



        // delete character 'd', original text becomes "abef"

        spannable.delete(2, 3);

        assertThat(transformedText.toString()).isEqualTo("ab\n\nef");

        assertLineRange(layout, /* lineBreaks */ 0, 3, 4, 6);



        // delete "be", original text becomes "af"

        spannable.delete(1, 3);

        assertThat(transformedText.toString()).isEqualTo("a\n\nf");

        assertLineRange(layout, /* lineBreaks */ 0, 2, 3, 4);

    }



    @Test

    @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_CRASH_UPDATE_LAYOUT_SPAN)

    public void textWithOffsetMapping_insert_withUpdateLayoutSpan() {

        final String text = "abcdef";

        final SpannableStringBuilder spannable = new SpannableStringBuilder(text);

        final CharSequence transformedText = new TestOffsetMapping(spannable, 3, "\n\n");



        // UpdateLayout span covers the letter 'de'.

        spannable.setSpan(new UpdateLayout() {}, 3, 5, Spanned.SPAN_INCLUSIVE_EXCLUSIVE);



        final DynamicLayout layout = DynamicLayout.Builder.obtain(spannable, sTextPaint, WIDTH)

                .setAlignment(ALIGN_NORMAL)

                .setIncludePad(false)

                .setDisplayText(transformedText)

                .build();



        spannable.insert(3, "x");

        assertThat(transformedText.toString()).isEqualTo("abcx\n\ndef");

        assertLineRange(layout, /* lineBreaks */ 0, 5, 6, 9);



        spannable.insert(5, "x");

        assertThat(transformedText.toString()).isEqualTo("abcx\n\ndxef");

        assertLineRange(layout, /* lineBreaks */ 0, 5, 6, 10);

    }



    @Test

    @RequiresFlagsEnabled(Flags.FLAG_INSERT_MODE_CRASH_UPDATE_LAYOUT_SPAN)

    public void textWithOffsetMapping_replace_withUpdateLayoutSpan() {

        final String text = "abcdef";

        final SpannableStringBuilder spannable = new SpannableStringBuilder(text);

        final CharSequence transformedText = new TestOffsetMapping(spannable, 3, "\n\n");

        // UpdateLayout span covers the letter 'de'.

        spannable.setSpan(new UpdateLayout() {}, 3, 5, Spanned.SPAN_INCLUSIVE_EXCLUSIVE);



        final DynamicLayout layout = DynamicLayout.Builder.obtain(spannable, sTextPaint, WIDTH)

                .setAlignment(ALIGN_NORMAL)

                .setIncludePad(false)

                .setDisplayText(transformedText)

                .build();



        spannable.replace(2, 4, "xx");

        assertThat(transformedText.toString()).isEqualTo("abxx\n\nef");

        assertLineRange(layout, /* lineBreaks */ 0, 5, 6, 8);

    }



    @Test

    public void textWithOffsetMapping_blockBeforeTextChanged_deletion() {

        final String text = "abcdef";