- Revision
- 136045
- Author
- ach...@adobe.com
- Date
- 2012-11-28 12:39:52 -0800 (Wed, 28 Nov 2012)
Log Message
[CSS Regions] Crash when using hover and first-letter inside a flow-thread
https://bugs.webkit.org/show_bug.cgi?id=102957
Reviewed by David Hyatt.
Source/WebCore:
Some RenderObjects use a different path when they are destroyed. That's because they are dynamically
added just before layout happens and their parent is usually not their actual owner. In those cases the parent
will remove the object from the tree, but it's actually the owner that will destroy the object and all its
children.
RenderFlowThread maintains a RenderBoxRegionInfo object for each RenderObject that is rendered inside the
flow-thread. When the RenderObject is removed from the RenderFlowThread, the associated RenderBoxRegionInfo object
also needs to be removed.
In these special cases (list-marker, first-letter), the object itself was removed from the RenderFlowThread,
but its children were still left in the flow-thread. When the these special objects were later destroyed,
they will remove their own children. Removing their children means it will try to remove them from the
associated RenderFlowThread. However, in this cases there would be no link back to the parent flow-thread,
as the tree is now detached from the enclosing RenderFlowThread.
Added code that recursively removes the whole children tree from the RenderFlowThread when the root is removed.
Tests: fast/regions/firstletter-inside-flowthread.html
fast/regions/listmarker-inside-flowthread.html
* rendering/RenderObject.cpp:
(WebCore::RenderObject::willBeRemovedFromTree):
(WebCore::RenderObject::removeFromRenderFlowThread):
(WebCore):
(WebCore::RenderObject::removeFromRenderFlowThreadRecursive):
* rendering/RenderObject.h:
(RenderObject):
LayoutTests:
Added CSS Regions tests for the firstLetter and listMarker render objects that use
different destroy paths in the code.
* fast/regions/firstletter-inside-flowthread-expected.html: Added.
* fast/regions/firstletter-inside-flowthread.html: Added.
* fast/regions/listmarker-inside-flowthread-expected.html: Added.
* fast/regions/listmarker-inside-flowthread.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (136044 => 136045)
--- trunk/LayoutTests/ChangeLog 2012-11-28 20:32:37 UTC (rev 136044)
+++ trunk/LayoutTests/ChangeLog 2012-11-28 20:39:52 UTC (rev 136045)
@@ -1,3 +1,18 @@
+2012-11-28 Alexandru Chiculita <ach...@adobe.com>
+
+ [CSS Regions] Crash when using hover and first-letter inside a flow-thread
+ https://bugs.webkit.org/show_bug.cgi?id=102957
+
+ Reviewed by David Hyatt.
+
+ Added CSS Regions tests for the firstLetter and listMarker render objects that use
+ different destroy paths in the code.
+
+ * fast/regions/firstletter-inside-flowthread-expected.html: Added.
+ * fast/regions/firstletter-inside-flowthread.html: Added.
+ * fast/regions/listmarker-inside-flowthread-expected.html: Added.
+ * fast/regions/listmarker-inside-flowthread.html: Added.
+
2012-11-28 Tony Chang <t...@chromium.org>
Unreviewed, fix duplicate expectation.
Added: trunk/LayoutTests/fast/regions/firstletter-inside-flowthread-expected.html (0 => 136045)
--- trunk/LayoutTests/fast/regions/firstletter-inside-flowthread-expected.html (rev 0)
+++ trunk/LayoutTests/fast/regions/firstletter-inside-flowthread-expected.html 2012-11-28 20:39:52 UTC (rev 136045)
@@ -0,0 +1,20 @@
+<!doctype html>
+<html>
+ <head>
+ <title>Test for https://bugs.webkit.org/show_bug.cgi?id=102957</title>
+ <style>
+ .region {
+ color: green;
+ font-size: 50px;
+ font-family: Ahem;
+ height: 50px;
+ }
+ </style>
+ </head>
+ <body>
+ <p>Test case for <a href=""
+ <p>Testing that the removal of the first-letter pseudo render object will not crash the flow thread logic.</p>
+ <p>You should see a green rectangle. There should be no red or yellow.</p>
+ <div class="region">aaaa</div>
+ </body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/fast/regions/firstletter-inside-flowthread.html (0 => 136045)
--- trunk/LayoutTests/fast/regions/firstletter-inside-flowthread.html (rev 0)
+++ trunk/LayoutTests/fast/regions/firstletter-inside-flowthread.html 2012-11-28 20:39:52 UTC (rev 136045)
@@ -0,0 +1,39 @@
+<!doctype html>
+<html>
+ <head>
+ <title>Test for https://bugs.webkit.org/show_bug.cgi?id=102957</title>
+ <style>
+ .content {
+ -webkit-flow-into: flow;
+ font-size: 50px;
+ color: green;
+ font-family: Ahem;
+ }
+
+ .first-letter-div:first-letter {
+ color: red;
+ }
+
+ .region {
+ -webkit-flow-from: flow;
+ height: 50px;
+ }
+ </style>
+ <script>
+ function removeFirstLetter() {
+ document.body.offsetTop; // force layout
+ document.getElementById("target").style.display = "none";
+ }
+ </script>
+ </head>
+ <body _onload_="removeFirstLetter()">
+ <p>Test case for <a href=""
+ <p>Testing that the removal of the first-letter pseudo render object will not crash the flow thread logic.</p>
+ <p>You should see a green rectangle. There should be no red or yellow.</p>
+ <div class="content">
+ <div id="target" class="first-letter-div">aaaa</div>
+ aaaa
+ </div>
+ <div class="region"></div>
+ </body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/fast/regions/listmarker-inside-flowthread-expected.html (0 => 136045)
--- trunk/LayoutTests/fast/regions/listmarker-inside-flowthread-expected.html (rev 0)
+++ trunk/LayoutTests/fast/regions/listmarker-inside-flowthread-expected.html 2012-11-28 20:39:52 UTC (rev 136045)
@@ -0,0 +1,33 @@
+<!doctype html>
+<html>
+ <head>
+ <title>Test for https://bugs.webkit.org/show_bug.cgi?id=102957</title>
+ <style>
+ .region {
+ height: 50px;
+ }
+
+ .region ul {
+ margin: 0px;
+ }
+
+ #target {
+ font-size: 50px;
+ color: green;
+ font-family: Ahem;
+ color: green;
+ list-style-type: none;
+ }
+ </style>
+ </head>
+ <body _onload_="removeFirstLetter()">
+ <p>Test case for <a href=""
+ <p>Testing that the removal of the list-marker render object will not crash the flow thread logic.</p>
+ <p>You should see a green rectangle. There should be no red.</p>
+ <div class="region">
+ <ul>
+ <li id="target">aaaa</li>
+ </ul>
+ </div>
+ </body>
+</html>
\ No newline at end of file
Added: trunk/LayoutTests/fast/regions/listmarker-inside-flowthread.html (0 => 136045)
--- trunk/LayoutTests/fast/regions/listmarker-inside-flowthread.html (rev 0)
+++ trunk/LayoutTests/fast/regions/listmarker-inside-flowthread.html 2012-11-28 20:39:52 UTC (rev 136045)
@@ -0,0 +1,49 @@
+<!doctype html>
+<html>
+ <head>
+ <title>Test for https://bugs.webkit.org/show_bug.cgi?id=102957</title>
+ <style>
+ .content {
+ -webkit-flow-into: flow;
+ font-size: 50px;
+ color: green;
+ font-family: Ahem;
+ }
+
+ .content ul {
+ margin: 0px;
+ }
+
+ #target {
+ color: red;
+ }
+
+ #target.list-marker {
+ color: green;
+ list-style-type: none;
+ }
+
+ .region {
+ -webkit-flow-from: flow;
+ height: 50px;
+ }
+ </style>
+ <script>
+ function removeFirstLetter() {
+ document.body.offsetTop; // force layout
+ document.getElementById("target").classList.add("list-marker");
+ }
+ </script>
+ </head>
+ <body _onload_="removeFirstLetter()">
+ <p>Test case for <a href=""
+ <p>Testing that the removal of the list-marker render object will not crash the flow thread logic.</p>
+ <p>You should see a green rectangle. There should be no red.</p>
+ <div class="content">
+ <ul>
+ <li id="target">aaaa</li>
+ </ul>
+ </div>
+ <div class="region"></div>
+ </body>
+</html>
\ No newline at end of file
Modified: trunk/Source/WebCore/ChangeLog (136044 => 136045)
--- trunk/Source/WebCore/ChangeLog 2012-11-28 20:32:37 UTC (rev 136044)
+++ trunk/Source/WebCore/ChangeLog 2012-11-28 20:39:52 UTC (rev 136045)
@@ -1,5 +1,40 @@
2012-11-28 Alexandru Chiculita <ach...@adobe.com>
+ [CSS Regions] Crash when using hover and first-letter inside a flow-thread
+ https://bugs.webkit.org/show_bug.cgi?id=102957
+
+ Reviewed by David Hyatt.
+
+ Some RenderObjects use a different path when they are destroyed. That's because they are dynamically
+ added just before layout happens and their parent is usually not their actual owner. In those cases the parent
+ will remove the object from the tree, but it's actually the owner that will destroy the object and all its
+ children.
+
+ RenderFlowThread maintains a RenderBoxRegionInfo object for each RenderObject that is rendered inside the
+ flow-thread. When the RenderObject is removed from the RenderFlowThread, the associated RenderBoxRegionInfo object
+ also needs to be removed.
+
+ In these special cases (list-marker, first-letter), the object itself was removed from the RenderFlowThread,
+ but its children were still left in the flow-thread. When the these special objects were later destroyed,
+ they will remove their own children. Removing their children means it will try to remove them from the
+ associated RenderFlowThread. However, in this cases there would be no link back to the parent flow-thread,
+ as the tree is now detached from the enclosing RenderFlowThread.
+
+ Added code that recursively removes the whole children tree from the RenderFlowThread when the root is removed.
+
+ Tests: fast/regions/firstletter-inside-flowthread.html
+ fast/regions/listmarker-inside-flowthread.html
+
+ * rendering/RenderObject.cpp:
+ (WebCore::RenderObject::willBeRemovedFromTree):
+ (WebCore::RenderObject::removeFromRenderFlowThread):
+ (WebCore):
+ (WebCore::RenderObject::removeFromRenderFlowThreadRecursive):
+ * rendering/RenderObject.h:
+ (RenderObject):
+
+2012-11-28 Alexandru Chiculita <ach...@adobe.com>
+
[CSS Regions] Auto-height regions will not calculate the height correctly when the content changes dynamically
https://bugs.webkit.org/show_bug.cgi?id=102954
Modified: trunk/Source/WebCore/rendering/RenderObject.cpp (136044 => 136045)
--- trunk/Source/WebCore/rendering/RenderObject.cpp 2012-11-28 20:32:37 UTC (rev 136044)
+++ trunk/Source/WebCore/rendering/RenderObject.cpp 2012-11-28 20:39:52 UTC (rev 136045)
@@ -2447,10 +2447,8 @@
if (isOutOfFlowPositioned() && parent()->childrenInline())
parent()->dirtyLinesFromChangedChild(this);
- if (inRenderFlowThread()) {
- ASSERT(enclosingRenderFlowThread());
- enclosingRenderFlowThread()->removeFlowChildInfo(this);
- }
+ if (inRenderFlowThread())
+ removeFromRenderFlowThread();
if (RenderNamedFlowThread* containerFlowThread = parent()->enclosingRenderNamedFlowThread())
containerFlowThread->removeFlowChild(this);
@@ -2461,6 +2459,27 @@
#endif
}
+void RenderObject::removeFromRenderFlowThread()
+{
+ RenderFlowThread* renderFlowThread = enclosingRenderFlowThread();
+ ASSERT(renderFlowThread);
+ // Sometimes we remove the element from the flow, but it's not destroyed at that time.
+ // It's only until later when we actually destroy it and remove all the children from it.
+ // Currently, that happens for firstLetter elements and list markers.
+ // Pass in the flow thread so that we don't have to look it up for all the children.
+ removeFromRenderFlowThreadRecursive(renderFlowThread);
+}
+
+void RenderObject::removeFromRenderFlowThreadRecursive(RenderFlowThread* renderFlowThread)
+{
+ if (const RenderObjectChildList* children = virtualChildren()) {
+ for (RenderObject* child = children->firstChild(); child; child = child->nextSibling())
+ child->removeFromRenderFlowThreadRecursive(renderFlowThread);
+ }
+ renderFlowThread->removeFlowChildInfo(this);
+ setInRenderFlowThread(false);
+}
+
void RenderObject::destroyAndCleanupAnonymousWrappers()
{
// If the tree is destroyed, there is no need for a clean-up phase.
Modified: trunk/Source/WebCore/rendering/RenderObject.h (136044 => 136045)
--- trunk/Source/WebCore/rendering/RenderObject.h 2012-11-28 20:32:37 UTC (rev 136044)
+++ trunk/Source/WebCore/rendering/RenderObject.h 2012-11-28 20:39:52 UTC (rev 136045)
@@ -985,6 +985,9 @@
virtual void willBeRemovedFromTree();
private:
+ void removeFromRenderFlowThread();
+ void removeFromRenderFlowThreadRecursive(RenderFlowThread*);
+
RenderStyle* cachedFirstLineStyle() const;
StyleDifference adjustStyleDifference(StyleDifference, unsigned contextSensitiveProperties) const;
