Make sure we don't mishandle HTMLFrameOwnerElement lifecycle
authorbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Apr 2016 23:38:09 +0000 (23:38 +0000)
committerbfulgham@apple.com <bfulgham@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Apr 2016 23:38:09 +0000 (23:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157040

Reviewed by Chris Dumez.

Source/WebCore:

Protect a couple of sites where event handling could result in the owning frame
being destroyed during execution.

Tested by fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html.

* inspector/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::didCommitLoad):
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible):

LayoutTests:

* fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash-expected.txt: Added.
* fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html: Added.
* fast/dom/HTMLAnchorElement/resources/iframe-with-anchor-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html [new file with mode: 0644]
LayoutTests/fast/dom/HTMLAnchorElement/resources/iframe-with-anchor-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorDOMAgent.cpp
Source/WebCore/rendering/RenderLayer.cpp

index 31d424a..f5feba9 100644 (file)
@@ -1,3 +1,14 @@
+2016-04-27  Brent Fulgham  <bfulgham@apple.com>
+
+        Make sure we don't mishandle HTMLFrameOwnerElement lifecycle
+        https://bugs.webkit.org/show_bug.cgi?id=157040
+
+        Reviewed by Chris Dumez.
+
+        * fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash-expected.txt: Added.
+        * fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html: Added.
+        * fast/dom/HTMLAnchorElement/resources/iframe-with-anchor-crash.html: Added.
+
 2016-04-28  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Add CSS Grid Layout runtime flag
diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash-expected.txt b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash-expected.txt
new file mode 100644 (file)
index 0000000..1fa2ac5
--- /dev/null
@@ -0,0 +1,4 @@
+This tests whether clicking on an anchor in an iframe with scrolling="no" will scroll to anchor. If clicking on the link below triggers a scroll, the test passes.
+
+
+PASS
diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html b/LayoutTests/fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html
new file mode 100644 (file)
index 0000000..3519a9f
--- /dev/null
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests whether clicking on an anchor in an iframe with scrolling="no" will scroll to anchor. If clicking on the link below triggers a scroll, the test passes.<p>
+<iframe id="target" src="resources/iframe-with-anchor-crash.html" width="100%" height="2000" scrolling="no" onload="setupTopLevel();"></iframe>
+<script>
+    var iframeTarget = document.getElementById('target');
+
+    function finish() {
+        var result = "PASS";
+        window.top.document.body.appendChild(document.createTextNode(result));
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }
+
+    function setupTopLevel() {
+        var scrollTarget = window.frames['target'].document.getElementById('might_scroll');
+
+        window.frames['target'].window.registerAction(function () {
+            iframeTarget.remove();
+            setTimeout(finish, 0);
+        });
+
+        window.frames['target'].window.run();
+    }
+</script>    
+</body>
+</html>
\ No newline at end of file
diff --git a/LayoutTests/fast/dom/HTMLAnchorElement/resources/iframe-with-anchor-crash.html b/LayoutTests/fast/dom/HTMLAnchorElement/resources/iframe-with-anchor-crash.html
new file mode 100644 (file)
index 0000000..5ae780d
--- /dev/null
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+    if (window.testRunner) {
+        testRunner.waitUntilDone();
+        testRunner.dumpAsText();
+    }
+
+    var workToDo;
+
+    function registerAction(action)
+    {
+        workToDo = action;
+    }
+
+    function run()
+    {
+        if (!window.eventSender)
+            return;
+
+        var anchor = document.getElementById("start");
+        var x = window.frameElement.offsetLeft + anchor.offsetLeft + 2;
+        var y = window.frameElement.offsetTop + anchor.offsetTop + 2;
+        eventSender.mouseMoveTo(x, y);
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+        workToDo();
+    }
+</script>
+</head>
+<body>
+    <a id="start" href="#anchor">Go to anchor</a>
+    <div id="might_scroll" style="height: 1000px"></div>
+    <a name="anchor">Anchor</a>
+</body>
+</html>
\ No newline at end of file
index fc68ef6..d805bfc 100644 (file)
@@ -1,3 +1,20 @@
+2016-04-27  Brent Fulgham  <bfulgham@apple.com>
+
+        Make sure we don't mishandle HTMLFrameOwnerElement lifecycle
+        https://bugs.webkit.org/show_bug.cgi?id=157040
+
+        Reviewed by Chris Dumez.
+
+        Protect a couple of sites where event handling could result in the owning frame
+        being destroyed during execution.
+
+        Tested by fast/dom/HTMLAnchorElement/anchor-in-noscroll-iframe-crash.html.
+
+        * inspector/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::didCommitLoad):
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollRectToVisible):
+
 2016-04-28  Manuel Rego Casasnovas  <rego@igalia.com>
 
         [css-grid] Add CSS Grid Layout runtime flag
index dd47aa2..4ac13b9 100644 (file)
@@ -1869,7 +1869,7 @@ void InspectorDOMAgent::mainFrameDOMContentLoaded()
 
 void InspectorDOMAgent::didCommitLoad(Document* document)
 {
-    Element* frameOwner = document->ownerElement();
+    RefPtr<Element> frameOwner = document->ownerElement();
     if (!frameOwner)
         return;
 
@@ -1878,12 +1878,12 @@ void InspectorDOMAgent::didCommitLoad(Document* document)
         return;
 
     // Re-add frame owner element together with its new children.
-    int parentId = m_documentNodeToIdMap.get(innerParentNode(frameOwner));
+    int parentId = m_documentNodeToIdMap.get(innerParentNode(frameOwner.get()));
     m_frontendDispatcher->childNodeRemoved(parentId, frameOwnerId);
-    unbind(frameOwner, &m_documentNodeToIdMap);
+    unbind(frameOwner.get(), &m_documentNodeToIdMap);
 
-    Ref<Inspector::Protocol::DOM::Node> value = buildObjectForNode(frameOwner, 0, &m_documentNodeToIdMap);
-    Node* previousSibling = innerPreviousSibling(frameOwner);
+    Ref<Inspector::Protocol::DOM::Node> value = buildObjectForNode(frameOwner.get(), 0, &m_documentNodeToIdMap);
+    Node* previousSibling = innerPreviousSibling(frameOwner.get());
     int prevId = previousSibling ? m_documentNodeToIdMap.get(previousSibling) : 0;
     m_frontendDispatcher->childNodeInserted(parentId, prevId, WTFMove(value));
 }
index c92cc43..198f985 100644 (file)
@@ -79,6 +79,7 @@
 #include "HitTestRequest.h"
 #include "HitTestResult.h"
 #include "Logging.h"
+#include "NoEventDispatchAssertion.h"
 #include "OverflowEvent.h"
 #include "OverlapTestRequestClient.h"
 #include "Page.h"
@@ -2519,6 +2520,9 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& rect, const ScrollAlignm
                 frameElementBase = downcast<HTMLFrameElementBase>(ownerElement);
 
             if (frameElementAndViewPermitScroll(frameElementBase, frameView)) {
+                // If this assertion fires we need to protect the ownerElement from being destroyed.
+                NoEventDispatchAssertion assertNoEventDispatch;
+
                 LayoutRect viewRect = frameView.visibleContentRect(LegacyIOSDocumentVisibleRect);
                 LayoutRect exposeRect = getRectToExpose(viewRect, viewRect, rect, alignX, alignY);