Do not update selection rect on dirty lineboxes.
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Oct 2016 04:19:09 +0000 (04:19 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Oct 2016 04:19:09 +0000 (04:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163862
<rdar://problem/28813156>

Reviewed by Simon Fraser.

Source/WebCore:

In certain cases RenderBlock::updateFirstLetter() triggers
unwanted render tree mutation while the caller assumes intact renderers.
This patch ensures that no renderers gets destroyed while computing the preferred widths
when we are outside of layout context.

Test: fast/css-generated-content/dynamic-first-letter-selection-clear-crash.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computePreferredLogicalWidths):
(WebCore::RenderBlock::updateFirstLetter):
* rendering/RenderBlock.h:
* rendering/RenderListItem.cpp:
(WebCore::RenderListItem::insertOrMoveMarkerRendererIfNeeded):
* rendering/RenderRubyRun.cpp:
(WebCore::RenderRubyRun::updateFirstLetter):
* rendering/RenderRubyRun.h:
* rendering/RenderTable.cpp:
(WebCore::RenderTable::updateFirstLetter):
* rendering/RenderTable.h:
* rendering/svg/RenderSVGText.cpp:
(WebCore::RenderSVGText::updateFirstLetter):
* rendering/svg/RenderSVGText.h:

LayoutTests:

* fast/css-generated-content/dynamic-first-letter-selection-clear-crash-expected.txt: Added.
* fast/css-generated-content/dynamic-first-letter-selection-clear-crash.html: Added.

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

13 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css-generated-content/dynamic-first-letter-selection-clear-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/css-generated-content/dynamic-first-letter-selection-clear-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderBlock.h
Source/WebCore/rendering/RenderListItem.cpp
Source/WebCore/rendering/RenderRubyRun.cpp
Source/WebCore/rendering/RenderRubyRun.h
Source/WebCore/rendering/RenderTable.cpp
Source/WebCore/rendering/RenderTable.h
Source/WebCore/rendering/svg/RenderSVGText.cpp
Source/WebCore/rendering/svg/RenderSVGText.h

index 87d88e5..cc391bc 100644 (file)
@@ -1,3 +1,14 @@
+2016-10-24  Zalan Bujtas  <zalan@apple.com>
+
+        Do not update selection rect on dirty lineboxes.
+        https://bugs.webkit.org/show_bug.cgi?id=163862
+        <rdar://problem/28813156>
+
+        Reviewed by Simon Fraser.
+
+        * fast/css-generated-content/dynamic-first-letter-selection-clear-crash-expected.txt: Added.
+        * fast/css-generated-content/dynamic-first-letter-selection-clear-crash.html: Added.
+
 2016-10-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r207795.
diff --git a/LayoutTests/fast/css-generated-content/dynamic-first-letter-selection-clear-crash-expected.txt b/LayoutTests/fast/css-generated-content/dynamic-first-letter-selection-clear-crash-expected.txt
new file mode 100644 (file)
index 0000000..52b7baf
--- /dev/null
@@ -0,0 +1,2 @@
+Pass if
+no crash.
diff --git a/LayoutTests/fast/css-generated-content/dynamic-first-letter-selection-clear-crash.html b/LayoutTests/fast/css-generated-content/dynamic-first-letter-selection-clear-crash.html
new file mode 100644 (file)
index 0000000..1d71156
--- /dev/null
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This tests that we can clear selection properly on dynamic content with first letter.</title>
+<style>
+.floatClass::first-letter {
+  float: right;
+}
+
+#innerBody {
+ column-count: 2;
+}
+
+</style>
+</head>
+<body>
+<li id=li style="float: right;">Pass if<span style="float: right;"></span></li><body id=innerBody style="height: 100px"><span id=span style="display: none;"></span>no crash.</body>
+<script>
+  if (window.testRunner)
+    testRunner.dumpAsText();
+  innerBody.style.webkitWritingMode = "vertical-rl";
+  innerBody.className = "floatClass";
+
+  document.getSelection().setBaseAndExtent(span, 0, innerBody, innerBody.childNodes.length);
+  window.getSelection().modify("extend", "left", "documentboundary");
+  innerBody.scrollIntoViewIfNeeded(true);
+  li.style.cssText = "float: right; height: 40px; width: 40px;";
+  document.body.offsetHeight;
+  li.style.cssText = "";
+</script>
+</body>
+</html>
index 4ca4985..bb2d9a3 100644 (file)
@@ -1,3 +1,34 @@
+2016-10-24  Zalan Bujtas  <zalan@apple.com>
+
+        Do not update selection rect on dirty lineboxes.
+        https://bugs.webkit.org/show_bug.cgi?id=163862
+        <rdar://problem/28813156>
+
+        Reviewed by Simon Fraser.
+
+        In certain cases RenderBlock::updateFirstLetter() triggers
+        unwanted render tree mutation while the caller assumes intact renderers.
+        This patch ensures that no renderers gets destroyed while computing the preferred widths
+        when we are outside of layout context.
+
+        Test: fast/css-generated-content/dynamic-first-letter-selection-clear-crash.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::computePreferredLogicalWidths):
+        (WebCore::RenderBlock::updateFirstLetter):
+        * rendering/RenderBlock.h:
+        * rendering/RenderListItem.cpp:
+        (WebCore::RenderListItem::insertOrMoveMarkerRendererIfNeeded):
+        * rendering/RenderRubyRun.cpp:
+        (WebCore::RenderRubyRun::updateFirstLetter):
+        * rendering/RenderRubyRun.h:
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::updateFirstLetter):
+        * rendering/RenderTable.h:
+        * rendering/svg/RenderSVGText.cpp:
+        (WebCore::RenderSVGText::updateFirstLetter):
+        * rendering/svg/RenderSVGText.h:
+
 2016-10-24  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r207795.
index fede7dc..c747d75 100644 (file)
@@ -2755,7 +2755,9 @@ void RenderBlock::computePreferredLogicalWidths()
 {
     ASSERT(preferredLogicalWidthsDirty());
 
-    updateFirstLetter();
+    // FIXME: Do not even try to reshuffle first letter renderers when we are not in layout
+    // until after webkit.org/b/163848 is fixed.
+    updateFirstLetter(view().frameView().isInRenderTreeLayout() ? RenderTreeMutationIsAllowed::Yes : RenderTreeMutationIsAllowed::No);
 
     m_minPreferredLogicalWidth = 0;
     m_maxPreferredLogicalWidth = 0;
@@ -3305,7 +3307,7 @@ void RenderBlock::getFirstLetter(RenderObject*& firstLetter, RenderElement*& fir
         firstLetterContainer = nullptr;
 }
 
-void RenderBlock::updateFirstLetter()
+void RenderBlock::updateFirstLetter(RenderTreeMutationIsAllowed mutationAllowedOrNot)
 {
     RenderObject* firstLetterObj;
     RenderElement* firstLetterContainer;
@@ -3326,6 +3328,8 @@ void RenderBlock::updateFirstLetter()
     if (!is<RenderText>(*firstLetterObj))
         return;
 
+    if (mutationAllowedOrNot != RenderTreeMutationIsAllowed::Yes)
+        return;
     // Our layout state is not valid for the repaints we are going to trigger by
     // adding and removing children of firstLetterContainer.
     LayoutStateDisabler layoutStateDisabler(view());
index b7b506d..ddab760 100644 (file)
@@ -245,7 +245,8 @@ public:
     LayoutUnit collapsedMarginBeforeForChild(const RenderBox& child) const;
     LayoutUnit collapsedMarginAfterForChild(const RenderBox& child) const;
 
-    virtual void updateFirstLetter();
+    enum class RenderTreeMutationIsAllowed { Yes, No };
+    virtual void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes);
     void getFirstLetter(RenderObject*& firstLetter, RenderElement*& firstLetterContainer, RenderObject* skipObject = nullptr);
 
     virtual void scrollbarsChanged(bool /*horizontalScrollbarChanged*/, bool /*verticalScrollbarChanged*/) { }
index 7ca2748..a4987b0 100644 (file)
@@ -268,7 +268,7 @@ void RenderListItem::insertOrMoveMarkerRendererIfNeeded()
     if (!m_marker)
         return;
 
-    // FIXME: Do not even try reposition the marker when we are not in layout
+    // FIXME: Do not even try to reposition the marker when we are not in layout
     // until after we fixed webkit.org/b/163789.
     if (!view().frameView().isInRenderTreeLayout())
         return;
index 856a46a..2d8f6a0 100644 (file)
@@ -101,7 +101,7 @@ RenderBlock* RenderRubyRun::firstLineBlock() const
     return 0;
 }
 
-void RenderRubyRun::updateFirstLetter()
+void RenderRubyRun::updateFirstLetter(RenderTreeMutationIsAllowed)
 {
 }
 
index 12c0b36..ed9120c 100644 (file)
@@ -61,7 +61,7 @@ public:
     void removeChild(RenderObject&) override;
 
     RenderBlock* firstLineBlock() const override;
-    void updateFirstLetter() override;
+    void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) override;
 
     void getOverhang(bool firstLine, RenderObject* startRenderer, RenderObject* endRenderer, float& startOverhang, float& endOverhang) const;
 
index f034613..4c0b7b3 100644 (file)
@@ -1458,7 +1458,7 @@ RenderBlock* RenderTable::firstLineBlock() const
     return nullptr;
 }
 
-void RenderTable::updateFirstLetter()
+void RenderTable::updateFirstLetter(RenderTreeMutationIsAllowed)
 {
 }
 
index 63ec2bf..042792b 100644 (file)
@@ -303,7 +303,7 @@ private:
     void invalidateCachedColumnOffsets();
 
     RenderBlock* firstLineBlock() const final;
-    void updateFirstLetter() final;
+    void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) final;
     
     void updateLogicalWidth() final;
 
index 637c0c0..2bab7a7 100644 (file)
@@ -545,7 +545,7 @@ RenderBlock* RenderSVGText::firstLineBlock() const
 
 // Fix for <rdar://problem/8048875>. We should not render :first-letter CSS Style
 // in a SVG text element context.
-void RenderSVGText::updateFirstLetter()
+void RenderSVGText::updateFirstLetter(RenderTreeMutationIsAllowed)
 {
 }
 
index dd2ec30..c9ad2f8 100644 (file)
@@ -92,7 +92,7 @@ private:
     std::unique_ptr<RootInlineBox> createRootInlineBox() override;
 
     RenderBlock* firstLineBlock() const override;
-    void updateFirstLetter() override;
+    void updateFirstLetter(RenderTreeMutationIsAllowed = RenderTreeMutationIsAllowed::Yes) override;
 
     bool shouldHandleSubtreeMutations() const;