<rdar://problem/7873647> Crash when updating hover state
authormitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Apr 2010 23:10:19 +0000 (23:10 +0000)
committermitz@apple.com <mitz@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Apr 2010 23:10:19 +0000 (23:10 +0000)
Reviewed by Simon Fraser.

WebCore:

Test: fast/dynamic/hover-style-recalc-crash.html

Updating the hover state of an element caused the document to need style
recalc, and then updating the hover state of a link caused style recalc,
which changed the render tree while updateHoverActiveState() was iterating
over it, leading to a crash.

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateHoverActiveState): Collect the nodes to be
updated into vectors, then update their active and hover states.

LayoutTests:

* fast/dynamic/hover-style-recalc-crash-expected.txt: Added.
* fast/dynamic/hover-style-recalc-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dynamic/hover-style-recalc-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/dynamic/hover-style-recalc-crash.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/rendering/RenderLayer.cpp

index ad03ff0..0a3bbbf 100644 (file)
@@ -1,3 +1,12 @@
+2010-04-16  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        <rdar://problem/7873647> Crash when updating hover state
+
+        * fast/dynamic/hover-style-recalc-crash-expected.txt: Added.
+        * fast/dynamic/hover-style-recalc-crash.html: Added.
+
 2010-04-16  Andrew Scherkus  <scherkus@chromium.org>
 
         Unreviewed, fixing Chromium test_expectations.txt due to removed tests in r57292.
diff --git a/LayoutTests/fast/dynamic/hover-style-recalc-crash-expected.txt b/LayoutTests/fast/dynamic/hover-style-recalc-crash-expected.txt
new file mode 100644 (file)
index 0000000..d35e72d
--- /dev/null
@@ -0,0 +1,5 @@
+Test for rdar://problem/7873647 Crash when updating hover state.
+
+Hover over the light blue square, then move down into the blue square. The browser should not crash.
+
+
diff --git a/LayoutTests/fast/dynamic/hover-style-recalc-crash.html b/LayoutTests/fast/dynamic/hover-style-recalc-crash.html
new file mode 100644 (file)
index 0000000..e86c13d
--- /dev/null
@@ -0,0 +1,26 @@
+<style>
+    div#a { width: 100px; height: 100px; background-color: lightblue; }
+    div#b { display: none; }
+    div#b a { display: block; width: 100px; height: 100px; background-color: blue; }
+    div#a:hover + div { display:block; }
+</style>
+<p>
+    Test for <i><a href="rdar://problem/7873647">rdar://problem/7873647</a>
+    Crash when updating hover state</i>.
+</p>
+<p>
+    Hover over the light blue square, then move down into the blue square. The browser should not crash.
+</p>
+<div id="a"></div>
+<div id="b">
+    <a></a>
+</div>
+<script>
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        document.body.offsetTop;
+        var y = document.getElementById("a").getBoundingClientRect().top;
+        eventSender.mouseMoveTo(50, y + 50);
+        eventSender.mouseMoveTo(50, y + 150);
+    }
+</script>
index 6979a33..ac6998a 100644 (file)
@@ -1,3 +1,20 @@
+2010-04-16  Dan Bernstein  <mitz@apple.com>
+
+        Reviewed by Simon Fraser.
+
+        <rdar://problem/7873647> Crash when updating hover state
+
+        Test: fast/dynamic/hover-style-recalc-crash.html
+
+        Updating the hover state of an element caused the document to need style
+        recalc, and then updating the hover state of a link caused style recalc,
+        which changed the render tree while updateHoverActiveState() was iterating
+        over it, leading to a crash.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateHoverActiveState): Collect the nodes to be
+        updated into vectors, then update their active and hover states.
+
 2010-04-16  Dumitru Daniliuc  <dumi@chromium.org>
 
         Reviewed by Alexey Proskuryakov.
index 2d868bb..286822c 100644 (file)
@@ -3197,22 +3197,33 @@ void RenderLayer::updateHoverActiveState(const HitTestRequest& request, HitTestR
     // Locate the common ancestor render object for the two renderers.
     RenderObject* ancestor = commonAncestor(oldHoverObj, newHoverObj);
 
+    Vector<Node*, 32> nodesToRemoveFromChain;
+    Vector<Node*, 32> nodesToAddToChain;
+
     if (oldHoverObj != newHoverObj) {
         // The old hover path only needs to be cleared up to (and not including) the common ancestor;
         for (RenderObject* curr = oldHoverObj; curr && curr != ancestor; curr = curr->hoverAncestor()) {
-            if (curr->node() && !curr->isText() && (!mustBeInActiveChain || curr->node()->inActiveChain())) {
-                curr->node()->setActive(false);
-                curr->node()->setHovered(false);
-            }
+            if (curr->node() && !curr->isText() && (!mustBeInActiveChain || curr->node()->inActiveChain()))
+                nodesToRemoveFromChain.append(curr->node());
         }
     }
 
     // Now set the hover state for our new object up to the root.
     for (RenderObject* curr = newHoverObj; curr; curr = curr->hoverAncestor()) {
-        if (curr->node() && !curr->isText() && (!mustBeInActiveChain || curr->node()->inActiveChain())) {
-            curr->node()->setActive(request.active());
-            curr->node()->setHovered(true);
-        }
+        if (curr->node() && !curr->isText() && (!mustBeInActiveChain || curr->node()->inActiveChain()))
+            nodesToAddToChain.append(curr->node());
+    }
+
+    size_t removeCount = nodesToRemoveFromChain.size();
+    for (size_t i = 0; i < removeCount; ++i) {
+        nodesToRemoveFromChain[i]->setActive(false);
+        nodesToRemoveFromChain[i]->setHovered(false);
+    }
+
+    size_t addCount = nodesToAddToChain.size();
+    for (size_t i = 0; i < addCount; ++i) {
+        nodesToAddToChain[i]->setActive(request.active());
+        nodesToAddToChain[i]->setHovered(true);
     }
 }