Title: [127472] trunk
- Revision
- 127472
- Author
- [email protected]
- Date
- 2012-09-04 10:47:49 -0700 (Tue, 04 Sep 2012)
Log Message
[CSS Regions] Destroying a render named flow thread without unregistering left-over content nodes triggered an assertion.
https://bugs.webkit.org/show_bug.cgi?id=95645
Patch by Andrei Bucur <[email protected]> on 2012-09-04
Reviewed by Abhishek Arya.
Source/WebCore:
This patch cleans up the render named flow thread before destruction by unregistering left-over content nodes.
Tests: fast/regions/moved-content-node-crash.html
* rendering/RenderNamedFlowThread.cpp:
(WebCore::RenderNamedFlowThread::~RenderNamedFlowThread):
LayoutTests:
Simple test case that triggers an ASSERT in debug mode and causes a crash in release.
The ASSERT appears in RenderFlowThreadController::unregisterNamedFlowContentNode
ASSERT(it != m_mapNamedFlowContentNodes.end());
It happens because when a content node is added to an iframe document and then moved back, the iframe's RenderNamedFlowThread is destroyed
without calling unregisterNamedFlowContentNode on the node. This triggers the before mentioned assertion after a lazyAttach and a detach
in the parent document.
* fast/regions/moved-content-node-crash-expected.txt: Added.
* fast/regions/moved-content-node-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (127471 => 127472)
--- trunk/LayoutTests/ChangeLog 2012-09-04 16:50:47 UTC (rev 127471)
+++ trunk/LayoutTests/ChangeLog 2012-09-04 17:47:49 UTC (rev 127472)
@@ -1,3 +1,21 @@
+2012-09-04 Andrei Bucur <[email protected]>
+
+ [CSS Regions] Destroying a render named flow thread without unregistering left-over content nodes triggered an assertion.
+ https://bugs.webkit.org/show_bug.cgi?id=95645
+
+ Reviewed by Abhishek Arya.
+
+ Simple test case that triggers an ASSERT in debug mode and causes a crash in release.
+ The ASSERT appears in RenderFlowThreadController::unregisterNamedFlowContentNode
+ ASSERT(it != m_mapNamedFlowContentNodes.end());
+
+ It happens because when a content node is added to an iframe document and then moved back, the iframe's RenderNamedFlowThread is destroyed
+ without calling unregisterNamedFlowContentNode on the node. This triggers the before mentioned assertion after a lazyAttach and a detach
+ in the parent document.
+
+ * fast/regions/moved-content-node-crash-expected.txt: Added.
+ * fast/regions/moved-content-node-crash.html: Added.
+
2012-09-04 Andrei Poenaru <[email protected]>
Web Inspector: Protocol Extension: Add "regionLayoutUpdate" event
Added: trunk/LayoutTests/fast/regions/moved-content-node-crash-expected.txt (0 => 127472)
--- trunk/LayoutTests/fast/regions/moved-content-node-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/regions/moved-content-node-crash-expected.txt 2012-09-04 17:47:49 UTC (rev 127472)
@@ -0,0 +1,7 @@
+Test for WebKit Bug 95645 Crash
+
+The test passes if it does not crash or assert.
+
+PASS
+
+
Added: trunk/LayoutTests/fast/regions/moved-content-node-crash.html (0 => 127472)
--- trunk/LayoutTests/fast/regions/moved-content-node-crash.html (rev 0)
+++ trunk/LayoutTests/fast/regions/moved-content-node-crash.html 2012-09-04 17:47:49 UTC (rev 127472)
@@ -0,0 +1,27 @@
+<!doctype html>
+<html>
+<body>
+ <p>Test for <a href="" Bug 95645</a> Crash </p>
+ <p>The test passes if it does not crash or assert.</p>
+ <p>PASS</p>
+ <div id="DIV1">
+ <div id="DIV2">
+ <iframe id="FRAME1" style="-webkit-flow-into: flow;"></iframe>
+ <iframe id="FRAME2"></iframe>
+ <script>
+ if (window.testRunner)
+ window.testRunner.dumpAsText();
+
+ div1 = document.getElementById("DIV1");
+ div2 = document.getElementById("DIV2");
+ frame1 = document.getElementById("FRAME1");
+ frame2 = document.getElementById("FRAME2");
+ frame2.contentDocument.body.appendChild(frame1);
+
+ document.body.offsetTop;
+ div1.replaceChild(frame1, div2);
+ </script>
+ </div>
+ </div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (127471 => 127472)
--- trunk/Source/WebCore/ChangeLog 2012-09-04 16:50:47 UTC (rev 127471)
+++ trunk/Source/WebCore/ChangeLog 2012-09-04 17:47:49 UTC (rev 127472)
@@ -1,3 +1,17 @@
+2012-09-04 Andrei Bucur <[email protected]>
+
+ [CSS Regions] Destroying a render named flow thread without unregistering left-over content nodes triggered an assertion.
+ https://bugs.webkit.org/show_bug.cgi?id=95645
+
+ Reviewed by Abhishek Arya.
+
+ This patch cleans up the render named flow thread before destruction by unregistering left-over content nodes.
+
+ Tests: fast/regions/moved-content-node-crash.html
+
+ * rendering/RenderNamedFlowThread.cpp:
+ (WebCore::RenderNamedFlowThread::~RenderNamedFlowThread):
+
2012-09-04 Koji Ishii <[email protected]>
[chromium] OpenTypeVerticalData.cpp in both webcore_remaining and webcore_platform seems to break incremental linking on Windows Chromium
Modified: trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp (127471 => 127472)
--- trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp 2012-09-04 16:50:47 UTC (rev 127471)
+++ trunk/Source/WebCore/rendering/RenderNamedFlowThread.cpp 2012-09-04 17:47:49 UTC (rev 127472)
@@ -43,7 +43,11 @@
RenderNamedFlowThread::~RenderNamedFlowThread()
{
- // The RenderNamedFlowThread may be destroyed because the document is discarded. Leave the NamedFlow object in a consistent state by calling mark for destruction.
+ // The flow thread can be destroyed without unregistering the content nodes if the document is destroyed.
+ // This can lead to problems because the nodes are still marked as belonging to a flow thread.
+ clearContentNodes();
+
+ // Also leave the NamedFlow object in a consistent state by calling mark for destruction.
setMarkForDestruction();
}
@@ -51,6 +55,21 @@
{
return "RenderNamedFlowThread";
}
+
+void RenderNamedFlowThread::clearContentNodes()
+{
+ for (NamedFlowContentNodes::iterator it = m_contentNodes.begin(); it != m_contentNodes.end(); ++it) {
+ Node* contentNode = *it;
+
+ ASSERT(contentNode && contentNode->isElementNode());
+ ASSERT(contentNode->inNamedFlow());
+ ASSERT(contentNode->document() == document());
+
+ contentNode->clearInNamedFlow();
+ }
+
+ m_contentNodes.clear();
+}
RenderObject* RenderNamedFlowThread::nextRendererForNode(Node* node) const
{
@@ -299,6 +318,7 @@
void RenderNamedFlowThread::registerNamedFlowContentNode(Node* contentNode)
{
ASSERT(contentNode && contentNode->isElementNode());
+ ASSERT(contentNode->document() == document());
contentNode->setInNamedFlow();
@@ -321,6 +341,7 @@
ASSERT(contentNode && contentNode->isElementNode());
ASSERT(m_contentNodes.contains(contentNode));
ASSERT(contentNode->inNamedFlow());
+ ASSERT(contentNode->document() == document());
contentNode->clearInNamedFlow();
m_contentNodes.remove(contentNode);
Modified: trunk/Source/WebCore/rendering/RenderNamedFlowThread.h (127471 => 127472)
--- trunk/Source/WebCore/rendering/RenderNamedFlowThread.h 2012-09-04 16:50:47 UTC (rev 127471)
+++ trunk/Source/WebCore/rendering/RenderNamedFlowThread.h 2012-09-04 17:47:49 UTC (rev 127472)
@@ -87,6 +87,7 @@
void checkInvalidRegions();
bool canBeDestroyed() const { return m_regionList.isEmpty() && m_contentNodes.isEmpty(); }
void regionLayoutUpdateEventTimerFired(Timer<RenderNamedFlowThread>*);
+ void clearContentNodes();
private:
// Observer flow threads have invalid regions that depend on the state of this thread
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
