Regression(r129406): Fix the scope of the WidgetHierarchyUpdateSuspen…

…sionScope in Element::Attach(). https://bugs.webkit.org/show_bug.cgi?id=100803 Reviewed by Abhishek Arya. Source/WebCore: Ensures that the suspension scope has gone out of scope before calling into resumePostAttachCallbacks(). Test: fast/dom/adopt-node-crash-2.html * dom/Element.cpp: (WebCore::Element::attach): LayoutTests: * fast/dom/adopt-node-crash-2-expected.txt: Added. * fast/dom/adopt-node-crash-2.html: Added. git-svn-id: http://svn.webkit.org/repository/webkit/trunk@135914 268f45cc-cd09-0410-ab3c-d52691b4dbfc
ziswiler · Nov 27, 2012 · 8a16132 · 8a16132
1 parent 6ce3b60
commit 8a16132
Show file tree

Hide file tree

Showing 5 changed files with 78 additions and 21 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2012-11-27  Tom Sepez  <[email protected]>
+
+        Regression(r129406): Fix the scope of the WidgetHierarchyUpdateSuspensionScope in Element::Attach().
+        https://bugs.webkit.org/show_bug.cgi?id=100803
+
+        Reviewed by Abhishek Arya.
+
+        * fast/dom/adopt-node-crash-2-expected.txt: Added.
+        * fast/dom/adopt-node-crash-2.html: Added.
+
 2012-11-27  Tony Chang  <[email protected]>
 
         Remove hidden limiter div in the input slider shadow DOM

diff --git a/LayoutTests/fast/dom/adopt-node-crash-2-expected.txt b/LayoutTests/fast/dom/adopt-node-crash-2-expected.txt
@@ -0,0 +1,2 @@
+Tests for a crash due to adopting a DOM node during DOMFocusOut event. Test passes if it doesn't crash.
+
diff --git a/LayoutTests/fast/dom/adopt-node-crash-2.html b/LayoutTests/fast/dom/adopt-node-crash-2.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<html>
+<div>Tests for a crash due to adopting a DOM node during DOMFocusOut event. Test passes if it doesn't crash.</div>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+</script>
+<div id="div1"></div>
+<div id="div2">
+  <applet>
+    <iframe srcdoc="<iframe srcdoc=''>">
+    </iframe>
+  </applet>
+  <header id="header1">
+    <keygen autofocus>
+  </header>
+</div>
+<script>
+function doit()
+{
+    div2.addEventListener("DOMFocusOut", function () { document.implementation.createDocument("", "", null).adoptNode(div2); }, false);
+    div1.outerHTML = header1.outerHTML;
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+document.addEventListener("DOMContentLoaded", setTimeout("doit()", 1), false);
+</script>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2012-11-27  Tom Sepez  <[email protected]>
+
+        Regression(r129406): Fix the scope of the WidgetHierarchyUpdateSuspensionScope in Element::Attach().
+        https://bugs.webkit.org/show_bug.cgi?id=100803
+
+        Reviewed by Abhishek Arya.
+
+        Ensures that the suspension scope has gone out of scope before calling into
+        resumePostAttachCallbacks().
+
+        Test: fast/dom/adopt-node-crash-2.html
+
+        * dom/Element.cpp:
+        (WebCore::Element::attach):
+
 2012-11-27  Tony Chang  <[email protected]>
 
         Remove hidden limiter div in the input slider shadow DOM

diff --git a/Source/WebCore/dom/Element.cpp b/Source/WebCore/dom/Element.cpp
@@ -1198,34 +1198,34 @@ void Element::createRendererIfNeeded()
 void Element::attach()
 {
     suspendPostAttachCallbacks();
-    WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
-
-    createRendererIfNeeded();
+    {
+        WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+        createRendererIfNeeded();
 
-    StyleResolverParentPusher parentPusher(this);
+        StyleResolverParentPusher parentPusher(this);
 
-    if (parentElement() && parentElement()->isInCanvasSubtree())
-        setIsInCanvasSubtree(true);
+        if (parentElement() && parentElement()->isInCanvasSubtree())
+            setIsInCanvasSubtree(true);
 
-    // When a shadow root exists, it does the work of attaching the children.
-    if (ElementShadow* shadow = this->shadow()) {
-        parentPusher.push();
-        shadow->attach();
-    } else {
-        if (firstChild())
+        // When a shadow root exists, it does the work of attaching the children.
+        if (ElementShadow* shadow = this->shadow()) {
             parentPusher.push();
-    }
-    ContainerNode::attach();
+            shadow->attach();
+        } else {
+            if (firstChild())
+                parentPusher.push();
+        }
+        ContainerNode::attach();
 
-    if (hasRareData()) {   
-        ElementRareData* data = elementRareData();
-        if (data->needsFocusAppearanceUpdateSoonAfterAttach()) {
-            if (isFocusable() && document()->focusedNode() == this)
-                document()->updateFocusAppearanceSoon(false /* don't restore selection */);
-            data->setNeedsFocusAppearanceUpdateSoonAfterAttach(false);
+        if (hasRareData()) {   
+            ElementRareData* data = elementRareData();
+            if (data->needsFocusAppearanceUpdateSoonAfterAttach()) {
+                if (isFocusable() && document()->focusedNode() == this)
+                    document()->updateFocusAppearanceSoon(false /* don't restore selection */);
+                data->setNeedsFocusAppearanceUpdateSoonAfterAttach(false);
+            }
         }
     }
-
     resumePostAttachCallbacks();
 }
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Tests for a crash due to adopting a DOM node during DOMFocusOut event. Test passes if it doesn't crash.