Changeset 214726 in webkit

Timestamp:

Apr 1, 2017 10:39:29 PM (7 years ago)

Author:

Alan Bujtas

Message:

Long Arabic text in ContentEditable with css white-space=pre hangs Safari
​https://bugs.webkit.org/show_bug.cgi?id=170245

Reviewed by Myles C. Maxfield.

While searching for mid-word break, we measure the text by codepoints in a loop until the accumulated width > available width.
When we see that the accumulated width for the individual codepoints overflows, we join the codepoints and re-measure them.
These 2 widths could be considerably different for number of reasons (ligatures is a prime example). When we figure that
the run still fits, we go back to the main loop (since we are not supposed to wrap the line here) and take the next codepoint.
However this time we start the measurement from the last whitespace, so we end up remeasuring a potentially long chuck of text
until we hit the wrapping point. This is way too expensive.
This patch changes the logic so that we just go back to measuring individual codepoints until we hit the constrain again.

Covered by existing tests.

• rendering/line/BreakingContext.h:

(WebCore::BreakingContext::handleText): canUseSimpleFontCodePath() is just to mitigate the potential risk of regression and
complex text is more likely to fall into this category.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/line/BreakingContext.h (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-04-01  Zalan Bujtas  <zalan@apple.com>
+
+        Long Arabic text in ContentEditable with css white-space=pre hangs Safari
+        https://bugs.webkit.org/show_bug.cgi?id=170245
+
+        Reviewed by Myles C. Maxfield.
+
+        While searching for mid-word break, we measure the text by codepoints in a loop until the accumulated width > available width.
+        When we see that the accumulated width for the individual codepoints overflows, we join the codepoints and re-measure them.
+        These 2 widths could be considerably different for number of reasons (ligatures is a prime example). When we figure that
+        the run still fits, we go back to the main loop (since we are not supposed to wrap the line here) and take the next codepoint.
+        However this time we start the measurement from the last whitespace, so we end up remeasuring a potentially long chuck of text
+        until we hit the wrapping point. This is way too expensive.
+        This patch changes the logic so that we just go back to measuring individual codepoints until we hit the constrain again.  
+
+        Covered by existing tests.
+
+        * rendering/line/BreakingContext.h:
+        (WebCore::BreakingContext::handleText): canUseSimpleFontCodePath() is just to mitigate the potential risk of regression and
+        complex text is more likely to fall into this category. 
+
 2017-04-01  Jon Lee  <jonlee@apple.com>
 

trunk/Source/WebCore/rendering/line/BreakingContext.h

@@ -795 +795 @@
     float wordSpacingForWordMeasurement = 0;
 
-    float wrapW = m_width.uncommittedWidth() + inlineLogicalWidth(m_current.renderer(), !m_appliedStartWidth, true);
+    float wrapWidthOffset = m_width.uncommittedWidth() + inlineLogicalWidth(m_current.renderer(), !m_appliedStartWidth, true);
+    float wrapW = wrapWidthOffset;
     float charWidth = 0;
     bool breakNBSP = m_autoWrap && m_currentStyle->nbspMode() == SPACE;
@@ -1029 +1030 @@
             if (m_autoWrap && betweenWords) {
                 commitLineBreakAtCurrentWidth(renderObject, m_current.offset(), m_current.nextBreakablePosition());
-                wrapW = 0;
+                wrapWidthOffset = 0;
+                wrapW = wrapWidthOffset;
                 // Auto-wrapping text should not wrap in the middle of a word once it has had an
                 // opportunity to break after a word.
@@ -1061 +1063 @@
                 }
             }
-            
+            // Measuring the width of complex text character-by-character, rather than measuring it all together,
+            // could produce considerably different width values.
+            if (!renderText.canUseSimpleFontCodePath() && midWordBreak && m_width.fitsOnLine()) {
+                midWordBreak = false;
+                wrapW = wrapWidthOffset + additionalTempWidth;
+            }
             isLineEmpty = m_lineInfo.isEmpty();
         } else {