[CSSRegions]Crash when moving anonymous block children inside a named flow
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 18:45:47 +0000 (18:45 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 18:45:47 +0000 (18:45 +0000)
https://bugs.webkit.org/show_bug.cgi?id=90865

Patch by Andrei Onea <onea@adobe.com> on 2012-08-23
Reviewed by Abhishek Arya.

Source/WebCore:

When an anonymous block's children are detached in RenderBlock::collapseAnonymousBoxChild, the reference
to their enclosingRenderFlowThread is lost and causes a crash in RenderObject::willBeRemovedFromTree.
Because of this, we now maintain the enclosingRenderFlowThread during the whole lifetime of the
RenderBlock::collapseAnonymousBoxChild function, using a CurrentRenderFlowThreadMaintainer local.

Test: fast/regions/move-anonymous-block-inside-named-flow-crash.html

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::collapseAnonymousBoxChild):
* rendering/RenderFlowThread.cpp:
(WebCore::CurrentRenderFlowThreadMaintainer::CurrentRenderFlowThreadMaintainer):
(WebCore):
(WebCore::CurrentRenderFlowThreadMaintainer::~CurrentRenderFlowThreadMaintainer):
Moved CurrentRenderFlowThreadMaintaner declaration from .cpp to .h, so that we can access it from
RenderBlock::collapseAnonymousBoxChild.
* rendering/RenderFlowThread.h:
(CurrentRenderFlowThreadMaintainer):
(WebCore):
* rendering/RenderObject.cpp:
(WebCore::RenderObject::willBeRemovedFromTree):

LayoutTests:

Added test for crash which happens when the children of an anonymous block
inside a flow thread are moved.

* fast/regions/move-anonymous-block-inside-named-flow-crash-expected.txt:
* fast/regions/move-anonymous-block-inside-named-flow-crash.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderBlock.cpp
Source/WebCore/rendering/RenderFlowThread.cpp
Source/WebCore/rendering/RenderFlowThread.h
Source/WebCore/rendering/RenderObject.cpp

index 52e2531..c7b11d1 100644 (file)
@@ -1,3 +1,16 @@
+2012-08-23  Andrei Onea  <onea@adobe.com>
+
+        [CSSRegions]Crash when moving anonymous block children inside a named flow
+        https://bugs.webkit.org/show_bug.cgi?id=90865
+
+        Reviewed by Abhishek Arya.
+
+        Added test for crash which happens when the children of an anonymous block
+        inside a flow thread are moved.
+
+        * fast/regions/move-anonymous-block-inside-named-flow-crash-expected.txt:
+        * fast/regions/move-anonymous-block-inside-named-flow-crash.html:
+
 2012-08-23  Li Yin  <li.yin@intel.com>
 
         Add test for decodeAudioData
diff --git a/LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash-expected.txt b/LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash-expected.txt
new file mode 100644 (file)
index 0000000..b0aae18
--- /dev/null
@@ -0,0 +1,2 @@
+Bug 90865:[CSSRegions]Crash when moving anonymous block children inside a named flow. Test passes if it does not CRASH or ASSERT.
+
diff --git a/LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash.html b/LayoutTests/fast/regions/move-anonymous-block-inside-named-flow-crash.html
new file mode 100644 (file)
index 0000000..d0d1d65
--- /dev/null
@@ -0,0 +1,27 @@
+<!doctype html>
+<html>
+<head>
+<style>
+.container { -webkit-column-count: 1; -webkit-flow-into: flow; }
+.columnSpan { -webkit-column-span: all; }
+.flow { -webkit-flow-from: flow; width: 100px; height: 100px; }
+</style>
+</head>
+<body>
+<div class="container">
+    <div id="test">
+        <div class="columnSpan"></div>
+    </div>
+</div>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+    var test = document.getElementById("test");
+    test.innerHTML = "Bug 90865:[CSSRegions]Crash when moving anonymous block children inside a named flow.\
+                      Test passes if it does not CRASH or ASSERT.";
+    var article = document.createElement("div");
+    article.setAttribute("class", "flow");
+    document.body.appendChild(article);
+</script>
+</body>
+</html>
index def7e69..d63ee7a 100644 (file)
@@ -1,3 +1,32 @@
+2012-08-23  Andrei Onea  <onea@adobe.com>
+
+        [CSSRegions]Crash when moving anonymous block children inside a named flow
+        https://bugs.webkit.org/show_bug.cgi?id=90865
+
+        Reviewed by Abhishek Arya.
+
+        When an anonymous block's children are detached in RenderBlock::collapseAnonymousBoxChild, the reference
+        to their enclosingRenderFlowThread is lost and causes a crash in RenderObject::willBeRemovedFromTree.
+        Because of this, we now maintain the enclosingRenderFlowThread during the whole lifetime of the
+        RenderBlock::collapseAnonymousBoxChild function, using a CurrentRenderFlowThreadMaintainer local.
+        
+
+        Test: fast/regions/move-anonymous-block-inside-named-flow-crash.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::collapseAnonymousBoxChild):
+        * rendering/RenderFlowThread.cpp:
+        (WebCore::CurrentRenderFlowThreadMaintainer::CurrentRenderFlowThreadMaintainer):
+        (WebCore):
+        (WebCore::CurrentRenderFlowThreadMaintainer::~CurrentRenderFlowThreadMaintainer):
+        Moved CurrentRenderFlowThreadMaintaner declaration from .cpp to .h, so that we can access it from
+        RenderBlock::collapseAnonymousBoxChild.
+        * rendering/RenderFlowThread.h:
+        (CurrentRenderFlowThreadMaintainer):
+        (WebCore):
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::willBeRemovedFromTree):
+
 2012-08-23  Kevin Ollivier  <kevino@theolliviers.com>
 
         [wx] Unreviewed build fix. Add wx to the list of platforms that use CoreText
index a4158f4..91a141f 100755 (executable)
@@ -1143,6 +1143,8 @@ void RenderBlock::collapseAnonymousBoxChild(RenderBlock* parent, RenderObject* c
     RenderObject* nextSibling = child->nextSibling();
 
     RenderFlowThread* childFlowThread = child->enclosingRenderFlowThread();
+    CurrentRenderFlowThreadMaintainer flowThreadMaintainer(childFlowThread);
+    
     RenderBlock* anonBlock = toRenderBlock(parent->children()->removeChildNode(parent, child, child->hasLayer()));
     anonBlock->moveAllChildrenTo(parent, nextSibling, child->hasLayer());
     // Delete the now-empty block's lines and nuke it.
index 429dad3..f3e47df 100644 (file)
@@ -107,26 +107,6 @@ void RenderFlowThread::removeRegionFromThread(RenderRegion* renderRegion)
     checkRegionsWithStyling();
 }
 
-class CurrentRenderFlowThreadMaintainer {
-    WTF_MAKE_NONCOPYABLE(CurrentRenderFlowThreadMaintainer);
-public:
-    CurrentRenderFlowThreadMaintainer(RenderFlowThread* renderFlowThread)
-        : m_renderFlowThread(renderFlowThread)
-    {
-        RenderView* view = m_renderFlowThread->view();
-        ASSERT(!view->flowThreadController()->currentRenderFlowThread());
-        view->flowThreadController()->setCurrentRenderFlowThread(m_renderFlowThread);
-    }
-    ~CurrentRenderFlowThreadMaintainer()
-    {
-        RenderView* view = m_renderFlowThread->view();
-        ASSERT(view->flowThreadController()->currentRenderFlowThread() == m_renderFlowThread);
-        view->flowThreadController()->setCurrentRenderFlowThread(0);
-    }
-private:
-    RenderFlowThread* m_renderFlowThread;
-};
-
 class CurrentRenderFlowThreadDisabler {
     WTF_MAKE_NONCOPYABLE(CurrentRenderFlowThreadDisabler);
 public:
@@ -793,4 +773,24 @@ bool RenderFlowThread::objectInFlowRegion(const RenderObject* object, const Rend
     return false;
 }
 
+CurrentRenderFlowThreadMaintainer::CurrentRenderFlowThreadMaintainer(RenderFlowThread* renderFlowThread)
+        : m_renderFlowThread(renderFlowThread)
+{
+    if (!m_renderFlowThread)
+        return;
+    RenderView* view = m_renderFlowThread->view();
+    ASSERT(!view->flowThreadController()->currentRenderFlowThread());
+    view->flowThreadController()->setCurrentRenderFlowThread(m_renderFlowThread);
+}
+
+CurrentRenderFlowThreadMaintainer::~CurrentRenderFlowThreadMaintainer()
+{
+    if (!m_renderFlowThread)
+        return;
+    RenderView* view = m_renderFlowThread->view();
+    ASSERT(view->flowThreadController()->currentRenderFlowThread() == m_renderFlowThread);
+    view->flowThreadController()->setCurrentRenderFlowThread(0);
+}
+
+
 } // namespace WebCore
index 4730be8..34dec69 100644 (file)
@@ -197,6 +197,15 @@ inline const RenderFlowThread* toRenderFlowThread(const RenderObject* object)
 // This will catch anyone doing an unnecessary cast.
 void toRenderFlowThread(const RenderFlowThread*);
 
+class CurrentRenderFlowThreadMaintainer {
+    WTF_MAKE_NONCOPYABLE(CurrentRenderFlowThreadMaintainer);
+public:
+    CurrentRenderFlowThreadMaintainer(RenderFlowThread*);
+    ~CurrentRenderFlowThreadMaintainer();
+private:
+    RenderFlowThread* m_renderFlowThread;
+};
+
 } // namespace WebCore
 
 #endif // RenderFlowThread_h
index 391c119..473d4b5 100755 (executable)
@@ -2435,9 +2435,8 @@ void RenderObject::willBeRemovedFromTree()
         parent()->dirtyLinesFromChangedChild(this);
 
     if (inRenderFlowThread()) {
-        if (isBox())
-            enclosingRenderFlowThread()->removeRenderBoxRegionInfo(toRenderBox(this));
-        enclosingRenderFlowThread()->clearRenderObjectCustomStyle(this);
+        ASSERT(enclosingRenderFlowThread());
+        enclosingRenderFlowThread()->removeFlowChildInfo(this);
     }
 
     if (RenderNamedFlowThread* containerFlowThread = parent()->enclosingRenderNamedFlowThread())