Use after free in WebCore::RenderNamedFlowFragment::restoreRegionObjectsOriginalStyle
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Feb 2015 18:43:48 +0000 (18:43 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Feb 2015 18:43:48 +0000 (18:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138366

Reviewed by Dave Hyatt.

This patch ensures that we clean up RenderNamedFlowFragment::m_renderObjectRegionStyle when embedded flow content is getting destroyed.

In m_renderObjectRegionStyle hash map, we store style information about the named flow's descendant children.
When a child is being detached from the tree, it removes itself from this hashmap.
We do it by traversing up on the ancestor chain and call removeFlowChildInfo() on the parent flow.
However in case of embedded flows (for example multicolumn content inside a region), we need to check whether the parent flow
is inside a flow too and continue the cleanup accordingly.

Source/WebCore:

Test: fast/regions/region-with-multicolumn-embedded-crash.html

* rendering/RenderObject.cpp:
(WebCore::RenderObject::removeFromRenderFlowThreadIncludingDescendants):

LayoutTests:

* fast/regions/region-with-multicolumn-embedded-crash-expected.txt: Added.
* fast/regions/region-with-multicolumn-embedded-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/regions/region-with-multicolumn-embedded-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/regions/region-with-multicolumn-embedded-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderObject.cpp

index 5e62abf..c9a10ae 100644 (file)
@@ -1,3 +1,21 @@
+2015-02-27  Zalan Bujtas  <zalan@apple.com>
+
+        Use after free in WebCore::RenderNamedFlowFragment::restoreRegionObjectsOriginalStyle
+        https://bugs.webkit.org/show_bug.cgi?id=138366
+
+        Reviewed by Dave Hyatt.
+
+        This patch ensures that we clean up RenderNamedFlowFragment::m_renderObjectRegionStyle when embedded flow content is getting destroyed.
+
+        In m_renderObjectRegionStyle hash map, we store style information about the named flow's descendant children.
+        When a child is being detached from the tree, it removes itself from this hashmap.
+        We do it by traversing up on the ancestor chain and call removeFlowChildInfo() on the parent flow.
+        However in case of embedded flows (for example multicolumn content inside a region), we need to check whether the parent flow
+        is inside a flow too and continue the cleanup accordingly.
+
+        * fast/regions/region-with-multicolumn-embedded-crash-expected.txt: Added.
+        * fast/regions/region-with-multicolumn-embedded-crash.html: Added.
+
 2015-02-27  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Add another batch of debug assert failures.
diff --git a/LayoutTests/fast/regions/region-with-multicolumn-embedded-crash-expected.txt b/LayoutTests/fast/regions/region-with-multicolumn-embedded-crash-expected.txt
new file mode 100644 (file)
index 0000000..6afeb43
--- /dev/null
@@ -0,0 +1,4 @@
+
+PASS if no crash or assert in debug.
+
+
diff --git a/LayoutTests/fast/regions/region-with-multicolumn-embedded-crash.html b/LayoutTests/fast/regions/region-with-multicolumn-embedded-crash.html
new file mode 100644 (file)
index 0000000..a60a01c
--- /dev/null
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<head>
+<title>This test that region context cleanup with embedded flow content is done properly.</title>
+<style>
+       #article1 { 
+        -webkit-flow-into: flow1;
+       }
+       
+       #region1 {
+        -webkit-flow-from: flow1;
+       }
+       
+       #p1 {
+        column-count: 2;
+       }
+
+       @-webkit-region #region1 {
+        #p1 {
+            color: red; 
+        }
+    }
+</style>
+<script>
+       if (window.testRunner)
+        testRunner.dumpAsText();
+</script>
+</head>
+<body>
+<p>PASS if no crash or assert in debug.</p>
+<div id="article1">
+       <p id="p1">X</p>
+</div>
+<div id="region1"></div>
+<script>
+       var iframe = document.createElement("iframe");
+       document.head.parentNode.insertBefore(iframe, document.head.nextSibling);
+       document.execCommand("SelectAll");
+       document.designMode = "on";
+       document.execCommand("JustifyCenter", false, null);
+       var elem = document.getElementById("article1"); 
+       elem.parentNode.removeChild(elem);
+</script>
+</body>
+</html>
index 27dcc39..3e81010 100644 (file)
@@ -1,3 +1,23 @@
+2015-02-27  Zalan Bujtas  <zalan@apple.com>
+
+        Use after free in WebCore::RenderNamedFlowFragment::restoreRegionObjectsOriginalStyle
+        https://bugs.webkit.org/show_bug.cgi?id=138366
+
+        Reviewed by Dave Hyatt.
+
+        This patch ensures that we clean up RenderNamedFlowFragment::m_renderObjectRegionStyle when embedded flow content is getting destroyed.
+
+        In m_renderObjectRegionStyle hash map, we store style information about the named flow's descendant children.
+        When a child is being detached from the tree, it removes itself from this hashmap.
+        We do it by traversing up on the ancestor chain and call removeFlowChildInfo() on the parent flow.
+        However in case of embedded flows (for example multicolumn content inside a region), we need to check whether the parent flow
+        is inside a flow too and continue the cleanup accordingly.
+
+        Test: fast/regions/region-with-multicolumn-embedded-crash.html
+
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::removeFromRenderFlowThreadIncludingDescendants):
+
 2015-02-27  Brady Eidson  <beidson@apple.com>
 
         Add API to remove a single content filter.
index e5095ea..ebb70a4 100644 (file)
@@ -1950,10 +1950,18 @@ void RenderObject::removeFromRenderFlowThreadIncludingDescendants(bool shouldUpd
 
     // We have to ask for our containing flow thread as it may be above the removed sub-tree.
     RenderFlowThread* flowThreadContainingBlock = this->flowThreadContainingBlock();
-    if (flowThreadContainingBlock)
+    while (flowThreadContainingBlock) {
         flowThreadContainingBlock->removeFlowChildInfo(this);
+        if (flowThreadContainingBlock->flowThreadState() == NotInsideFlowThread)
+            break;
+        RenderObject* parent = flowThreadContainingBlock->parent();
+        if (!parent)
+            break;
+        flowThreadContainingBlock = parent->flowThreadContainingBlock();
+    }
     if (is<RenderBlock>(*this))
         downcast<RenderBlock>(*this).setCachedFlowThreadContainingBlockNeedsUpdate();
+
     if (shouldUpdateState)
         setFlowThreadState(NotInsideFlowThread);
 }