[CSS Regions] Destroying a render named flow thread without unregistering...

[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 <abucur@adobe.com> 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.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@127472 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 11128bf8
2012-09-04 Andrei Bucur <abucur@adobe.com>
[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 <poenaru@adobe.com>
Web Inspector: Protocol Extension: Add "regionLayoutUpdate" event
Test for WebKit Bug 95645 Crash
The test passes if it does not crash or assert.
PASS
<!doctype html>
<html>
<body>
<p>Test for <a href="https://bugs.webkit.org/show_bug.cgi?id=95645">WebKit 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>
2012-09-04 Andrei Bucur <abucur@adobe.com>
[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 <kojiishi@gmail.com>
[chromium] OpenTypeVerticalData.cpp in both webcore_remaining and webcore_platform seems to break incremental linking on Windows Chromium
......@@ -43,7 +43,11 @@ RenderNamedFlowThread::RenderNamedFlowThread(Node* node, PassRefPtr<WebKitNamedF
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 @@ const char* RenderNamedFlowThread::renderName() const
{
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::pushDependencies(RenderNamedFlowThreadList& list)
void RenderNamedFlowThread::registerNamedFlowContentNode(Node* contentNode)
{
ASSERT(contentNode && contentNode->isElementNode());
ASSERT(contentNode->document() == document());
contentNode->setInNamedFlow();
......@@ -321,6 +341,7 @@ void RenderNamedFlowThread::unregisterNamedFlowContentNode(Node* contentNode)
ASSERT(contentNode && contentNode->isElementNode());
ASSERT(m_contentNodes.contains(contentNode));
ASSERT(contentNode->inNamedFlow());
ASSERT(contentNode->document() == document());
contentNode->clearInNamedFlow();
m_contentNodes.remove(contentNode);
......
......@@ -87,6 +87,7 @@ private:
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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment