Do not mutate RenderText content during layout.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Sep 2017 20:28:29 +0000 (20:28 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Sep 2017 20:28:29 +0000 (20:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=176219
<rdar://problem/34205724>

Reviewed by David Hyatt.

Source/WebCore:

Update combined text when the style/content change as opposed to lazily, during layout.
-content mutation during layout might make the inline tree go out of sync.

Test: fast/text/international/dynamic-text-combine-crash.html

* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::computeInlinePreferredLogicalWidths const):
* rendering/RenderCombineText.cpp:
(WebCore::RenderCombineText::styleDidChange):
(WebCore::RenderCombineText::setRenderedText):
(WebCore::RenderCombineText::combineTextIfNeeded):
(WebCore::RenderCombineText::combineText): Deleted.
* rendering/RenderCombineText.h:
* rendering/RenderText.h:
* rendering/line/BreakingContext.h:
(WebCore::BreakingContext::handleText):
* rendering/line/LineBreaker.cpp:
(WebCore::LineBreaker::skipLeadingWhitespace):

LayoutTests:

* fast/text/international/dynamic-text-combine-crash.html: Added.
* fast/text/text-combine-crash-expected.txt:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@222221 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/text/international/dynamic-text-combine-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/text/international/dynamic-text-combine-crash.html [new file with mode: 0644]
LayoutTests/fast/text/text-combine-crash-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlockFlow.cpp
Source/WebCore/rendering/RenderCombineText.cpp
Source/WebCore/rendering/RenderCombineText.h
Source/WebCore/rendering/RenderText.cpp
Source/WebCore/rendering/RenderText.h
Source/WebCore/rendering/line/BreakingContext.h
Source/WebCore/rendering/line/LineBreaker.cpp

index ab71d8d..9d5252d 100644 (file)
@@ -1,3 +1,14 @@
+2017-09-19  Zalan Bujtas  <zalan@apple.com>
+
+        Do not mutate RenderText content during layout.
+        https://bugs.webkit.org/show_bug.cgi?id=176219
+        <rdar://problem/34205724>
+
+        Reviewed by David Hyatt.
+
+        * fast/text/international/dynamic-text-combine-crash.html: Added.
+        * fast/text/text-combine-crash-expected.txt:
+
 2017-09-15  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         createMarkupInternal should protect its pointer to the Range's common ancestor
diff --git a/LayoutTests/fast/text/international/dynamic-text-combine-crash-expected.txt b/LayoutTests/fast/text/international/dynamic-text-combine-crash-expected.txt
new file mode 100644 (file)
index 0000000..a7e3805
--- /dev/null
@@ -0,0 +1,6 @@
+Pass if no crash.
+
+
+
+
+
diff --git a/LayoutTests/fast/text/international/dynamic-text-combine-crash.html b/LayoutTests/fast/text/international/dynamic-text-combine-crash.html
new file mode 100644 (file)
index 0000000..47fb3d3
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+h3 { 
+  max-height: 0; 
+  -webkit-text-combine: horizontal; 
+  -webkit-writing-mode: vertical-rl; 
+}
+</style>
+</head>
+<body><listing>Pass if no crash.<dd contenteditable="true"><h3 id="h">foobar</h3></body>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+    window.getSelection().setPosition(h, 1);
+    document.execCommand("delete", false);
+    document.execCommand("delete", false);
+</script>
+</html>
\ No newline at end of file
index 6bd5841..6b1bce2 100644 (file)
@@ -4,14 +4,14 @@ Test passes if there's no crash.
 
 
 
-
+
 Errlog webtest_fn_1: TypeError: undefined is not an object (evaluating 'document.applets[0].addEventListener')
 Errlog webtest_fn_2: TypeError: Argument 1 ('node') to Range.setStartBefore must be an instance of Node
 Errlog webtest_fn_3: TypeError: undefined is not an object (evaluating 'document.images[2].contentEditable="true"')
 Errlog webtest_fn_8: TypeError: null is not an object (evaluating 'lis.length')
-Errlog webtest_fn_9: TypeError: undefined is not an object (evaluating 'document.anchors[4].setAttribute')
+Errlog webtest_fn_9: TypeError: undefined is not an object (evaluating 'document.anchors[4].setAttribute')
 Errlog webtest_fn_10: TypeError: Argument 1 ('node') to Range.setStartAfter must be an instance of Node
-Errlog webtest_fn_15: TypeError: Argument 1 ('node') to Range.setStart must be an instance of Node
+Errlog webtest_fn_15: TypeError: Argument 1 ('node') to Range.setStart must be an instance of Node
 Errlog webtest_fn_16: TypeError: undefined is not an object (evaluating 'elem.parentNode')
 Errlog webtest_fn_18: TypeError: undefined is not an object (evaluating 'document.applets[0].contentEditable="true"')
 Errlog webtest_fn_21: TypeError: undefined is not an object (evaluating 'document.anchors[4].appendChild')
index 590d1f1..5ff7c60 100644 (file)
@@ -1,3 +1,30 @@
+2017-09-19  Zalan Bujtas  <zalan@apple.com>
+
+        Do not mutate RenderText content during layout.
+        https://bugs.webkit.org/show_bug.cgi?id=176219
+        <rdar://problem/34205724>
+
+        Reviewed by David Hyatt.
+
+        Update combined text when the style/content change as opposed to lazily, during layout.
+        -content mutation during layout might make the inline tree go out of sync.
+
+        Test: fast/text/international/dynamic-text-combine-crash.html
+
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::computeInlinePreferredLogicalWidths const):
+        * rendering/RenderCombineText.cpp:
+        (WebCore::RenderCombineText::styleDidChange):
+        (WebCore::RenderCombineText::setRenderedText):
+        (WebCore::RenderCombineText::combineTextIfNeeded):
+        (WebCore::RenderCombineText::combineText): Deleted.
+        * rendering/RenderCombineText.h:
+        * rendering/RenderText.h:
+        * rendering/line/BreakingContext.h:
+        (WebCore::BreakingContext::handleText):
+        * rendering/line/LineBreaker.cpp:
+        (WebCore::LineBreaker::skipLeadingWhitespace):
+
 2017-09-15  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         createMarkupInternal should protect its pointer to the Range's common ancestor
index 5a37a17..5633e39 100644 (file)
@@ -4353,7 +4353,7 @@ void RenderBlockFlow::computeInlinePreferredLogicalWidths(LayoutUnit& minLogical
                 RenderText& renderText = downcast<RenderText>(*child);
 
                 if (renderText.style().hasTextCombine() && renderText.isCombineText())
-                    downcast<RenderCombineText>(renderText).combineText();
+                    downcast<RenderCombineText>(renderText).combineTextIfNeeded();
 
                 // Determine if we have a breakable character. Pass in
                 // whether or not we should ignore any spaces at the front
index a5f19e6..b6e088f 100644 (file)
@@ -54,6 +54,7 @@ void RenderCombineText::styleDidChange(StyleDifference diff, const RenderStyle*
     }
 
     m_needsFontUpdate = true;
+    combineTextIfNeeded();
 }
 
 void RenderCombineText::setRenderedText(const String& text)
@@ -61,6 +62,7 @@ void RenderCombineText::setRenderedText(const String& text)
     RenderText::setRenderedText(text);
 
     m_needsFontUpdate = true;
+    combineTextIfNeeded();
 }
 
 float RenderCombineText::width(unsigned from, unsigned length, const FontCascade& font, float xPosition, HashSet<const Font*>* fallbackFonts, GlyphOverflow* glyphOverflow) const
@@ -95,7 +97,7 @@ String RenderCombineText::combinedStringForRendering() const
     return { };
 }
 
-void RenderCombineText::combineText()
+void RenderCombineText::combineTextIfNeeded()
 {
     if (!m_needsFontUpdate)
         return;
@@ -192,6 +194,8 @@ void RenderCombineText::combineText()
         m_combinedTextWidth = combinedTextWidth;
         m_combinedTextAscent = glyphOverflow.top;
         m_combinedTextDescent = glyphOverflow.bottom;
+        m_lineBoxes.dirtyRange(*this, 0, originalText().length(), originalText().length());
+        setNeedsLayout();
     }
 }
 
index 62ccb9f..968a65c 100644 (file)
@@ -32,7 +32,7 @@ public:
 
     Text& textNode() const { return downcast<Text>(nodeForNonAnonymous()); }
 
-    void combineText();
+    void combineTextIfNeeded();
     std::optional<FloatPoint> computeTextOrigin(const FloatRect& boxRect) const;
     String combinedStringForRendering() const;
     bool isCombined() const { return m_isCombined; }
index 92af0d7..a873e8e 100644 (file)
@@ -69,13 +69,13 @@ using namespace Unicode;
 namespace WebCore {
 
 struct SameSizeAsRenderText : public RenderObject {
+    void* pointers[2];
     uint32_t bitfields : 16;
 #if ENABLE(TEXT_AUTOSIZING)
     float candidateTextSize;
 #endif
     float widths[4];
     String text;
-    void* pointers[2];
 };
 
 COMPILE_ASSERT(sizeof(RenderText) == sizeof(SameSizeAsRenderText), RenderText_should_stay_small);
index 95340fb..675834f 100644 (file)
@@ -186,6 +186,8 @@ protected:
     virtual void setRenderedText(const String&);
     virtual UChar previousCharacter() const;
 
+    RenderTextLineBoxes m_lineBoxes;
+
 private:
     RenderText(Node&, const String&);
 
@@ -243,8 +245,6 @@ private:
     float m_endMinWidth;
 
     String m_text;
-
-    RenderTextLineBoxes m_lineBoxes;
 };
 
 inline UChar RenderText::uncheckedCharacterAt(unsigned i) const
index d405bbf..dfd3e5e 100644 (file)
@@ -755,7 +755,7 @@ inline bool BreakingContext::handleText(WordMeasurements& wordMeasurements, bool
 
     if (renderText.style().hasTextCombine() && is<RenderCombineText>(*m_current.renderer())) {
         auto& combineRenderer = downcast<RenderCombineText>(*m_current.renderer());
-        combineRenderer.combineText();
+        combineRenderer.combineTextIfNeeded();
         // The length of the renderer's text may have changed. Increment stale iterator positions
         if (iteratorIsBeyondEndOfRenderCombineText(m_lineBreakHistory.current(), combineRenderer)) {
             ASSERT(iteratorIsBeyondEndOfRenderCombineText(m_resolver.position(), combineRenderer));
index f64cd36..e25d859 100644 (file)
@@ -68,7 +68,7 @@ void LineBreaker::skipLeadingWhitespace(InlineBidiResolver& resolver, LineInfo&
         } else if (object.isFloating())
             m_block.positionNewFloatOnLine(*m_block.insertFloatingObject(downcast<RenderBox>(object)), lastFloatFromPreviousLine, lineInfo, width);
         else if (object.style().hasTextCombine() && is<RenderCombineText>(object)) {
-            downcast<RenderCombineText>(object).combineText();
+            downcast<RenderCombineText>(object).combineTextIfNeeded();
             if (downcast<RenderCombineText>(object).isCombined())
                 continue;
         }