Title: [230434] releases/WebKitGTK/webkit-2.20
Revision
230434
Author
carlo...@webkit.org
Date
2018-04-09 08:47:10 -0700 (Mon, 09 Apr 2018)

Log Message

Merge r230313 - Folding anonymous blocks should not result in deleting content.
https://bugs.webkit.org/show_bug.cgi?id=184339
<rdar://problem/37327428>

Reviewed by Antti Koivisto.

Source/WebCore:

While folding multiple anonymous blocks (moving the children from next sibling over to previous sibling)
we should ensure that the block we are about to destroy does not gain new descendants.
In case of 4 sibling anonymous blocks (A B C D), while destroying B
1. we move C's children to A and destroy C.
2. While destroying C, we notice B and C as sibling anonymous blocks and we move
D's children over to B (even though B is going to be destroyed as we climb back on the stack).

In this patch, B is detached from the tree before we start moving renderers around so that a subsequent folding won't
find B anymore as a candidate.

Test: fast/block/crash-while-folding-anonymous-blocks.html

* rendering/updating/RenderTreeBuilderBlock.cpp:
(WebCore::RenderTreeBuilder::Block::detach):

LayoutTests:

* fast/block/crash-when-subtree-is-still-attached-expected.txt: Progressing. This test does not
intend to remove "foobar" text at all.
* fast/block/crash-while-folding-anonymous-blocks-expected.txt: Added.
* fast/block/crash-while-folding-anonymous-blocks.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog (230433 => 230434)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog	2018-04-09 15:47:01 UTC (rev 230433)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/ChangeLog	2018-04-09 15:47:10 UTC (rev 230434)
@@ -1,3 +1,16 @@
+2018-04-05  Zalan Bujtas  <za...@apple.com>
+
+        Folding anonymous blocks should not result in deleting content.
+        https://bugs.webkit.org/show_bug.cgi?id=184339
+        <rdar://problem/37327428>
+
+        Reviewed by Antti Koivisto.
+
+        * fast/block/crash-when-subtree-is-still-attached-expected.txt: Progressing. This test does not 
+        intend to remove "foobar" text at all.
+        * fast/block/crash-while-folding-anonymous-blocks-expected.txt: Added.
+        * fast/block/crash-while-folding-anonymous-blocks.html: Added.
+
 2018-03-27  Fujii Hironori  <hironori.fu...@sony.com>
 
         [GTK] Layout test editing/deleting/delete-surrogatepair.html crashing with CRITICAL **: enchant_dict_check: assertion 'g_utf8_validate(word, len, NULL)' failed

Modified: releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-when-subtree-is-still-attached-expected.txt (230433 => 230434)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-when-subtree-is-still-attached-expected.txt	2018-04-09 15:47:01 UTC (rev 230433)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-when-subtree-is-still-attached-expected.txt	2018-04-09 15:47:10 UTC (rev 230434)
@@ -1,3 +1,4 @@
 PASS if
 no crash.
+foobar
 

Added: releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-while-folding-anonymous-blocks-expected.txt (0 => 230434)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-while-folding-anonymous-blocks-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-while-folding-anonymous-blocks-expected.txt	2018-04-09 15:47:10 UTC (rev 230434)
@@ -0,0 +1 @@
+

Added: releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-while-folding-anonymous-blocks.html (0 => 230434)


--- releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-while-folding-anonymous-blocks.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.20/LayoutTests/fast/block/crash-while-folding-anonymous-blocks.html	2018-04-09 15:47:10 UTC (rev 230434)
@@ -0,0 +1,33 @@
+<span id=foo9><div id=foo7></div></span><div id=foo1></div><div id=foo2></div><div id=foo3></div><span id=foo12><div id=foo10></div></span><div id=foo4></div><div id=foo5></div><div id=foo6></div><span id=foo11><table><colgroup><col></colgroup></table><div id=foo8></div></span><script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+// Setting up 3 continuations in separate blocks.
+document.body.offsetHeight;
+
+let di = document.createElement("div");
+foo2.appendChild(di);
+foo2.style.display = "inline";
+document.body.offsetHeight;
+
+foo1.remove();
+foo3.remove();
+document.body.offsetHeight;
+
+foo2.remove();
+document.body.offsetHeight;
+
+let di2 = document.createElement("div");
+foo5.appendChild(di2);
+foo5.style.display = "inline";
+document.body.offsetHeight;
+
+foo4.remove();
+foo6.remove();
+foo5.remove();
+foo7.remove();
+foo9.remove();
+foo10.remove();
+document.body.offsetHeight;
+foo12.remove();
+document.body.offsetHeight;
+</script>
\ No newline at end of file

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog (230433 => 230434)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-04-09 15:47:01 UTC (rev 230433)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/ChangeLog	2018-04-09 15:47:10 UTC (rev 230434)
@@ -1,3 +1,26 @@
+2018-04-05  Zalan Bujtas  <za...@apple.com>
+
+        Folding anonymous blocks should not result in deleting content.
+        https://bugs.webkit.org/show_bug.cgi?id=184339
+        <rdar://problem/37327428>
+
+        Reviewed by Antti Koivisto.
+
+        While folding multiple anonymous blocks (moving the children from next sibling over to previous sibling)
+        we should ensure that the block we are about to destroy does not gain new descendants.
+        In case of 4 sibling anonymous blocks (A B C D), while destroying B
+        1. we move C's children to A and destroy C.
+        2. While destroying C, we notice B and C as sibling anonymous blocks and we move
+        D's children over to B (even though B is going to be destroyed as we climb back on the stack).
+        
+        In this patch, B is detached from the tree before we start moving renderers around so that a subsequent folding won't
+        find B anymore as a candidate.
+
+        Test: fast/block/crash-while-folding-anonymous-blocks.html
+
+        * rendering/updating/RenderTreeBuilderBlock.cpp:
+        (WebCore::RenderTreeBuilder::Block::detach):
+
 2018-04-03  Chris Dumez  <cdu...@apple.com>
 
         Make SecurityOrigin safe to create and use from any thread

Modified: releases/WebKitGTK/webkit-2.20/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp (230433 => 230434)


--- releases/WebKitGTK/webkit-2.20/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp	2018-04-09 15:47:01 UTC (rev 230433)
+++ releases/WebKitGTK/webkit-2.20/Source/WebCore/rendering/updating/RenderTreeBuilderBlock.cpp	2018-04-09 15:47:10 UTC (rev 230434)
@@ -280,9 +280,14 @@
 
     // If this child is a block, and if our previous and next siblings are both anonymous blocks
     // with inline content, then we can fold the inline content back together.
-    RenderObject* prev = oldChild.previousSibling();
-    RenderObject* next = oldChild.nextSibling();
-    bool canMergeAnonymousBlocks = canMergeContiguousAnonymousBlocks(oldChild, prev, next);
+    auto prev = makeWeakPtr(oldChild.previousSibling());
+    auto next = makeWeakPtr(oldChild.nextSibling());
+    bool canMergeAnonymousBlocks = canMergeContiguousAnonymousBlocks(oldChild, prev.get(), next.get());
+
+    parent.invalidateLineLayoutPath();
+
+    auto takenChild = m_builder.detachFromRenderElement(parent, oldChild);
+
     if (canMergeAnonymousBlocks && prev && next) {
         prev->setNeedsLayoutAndPrefWidthsRecalc();
         RenderBlock& nextBlock = downcast<RenderBlock>(*next);
@@ -320,15 +325,10 @@
             // Delete the now-empty block's lines and nuke it.
             nextBlock.deleteLines();
             m_builder.destroy(nextBlock);
-            next = nullptr;
         }
     }
 
-    parent.invalidateLineLayoutPath();
-
-    auto takenChild = m_builder.detachFromRenderElement(parent, oldChild);
-
-    RenderObject* child = prev ? prev : next;
+    RenderObject* child = prev ? prev.get() : next.get();
     if (canMergeAnonymousBlocks && child && !child->previousSibling() && !child->nextSibling() && parent.canDropAnonymousBlockChild()) {
         // The removal has knocked us down to containing only a single anonymous
         // box. We can pull the content right back up into our box.
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to