Crash in the hit testing code via HTMLPlugInElement::isReplacementObscured()
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2019 23:49:56 +0000 (23:49 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Feb 2019 23:49:56 +0000 (23:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=194691

Reviewed by Simon Fraser.

Source/WebCore:

The crash was caused by HTMLPlugInElement::isReplacementObscured updating the document
without updating the layout of ancestor documents (i.e. documents in which frame owner
elements appear) even though it hit-tests against the top-level document's RenderView.

Fixed the bug by updating the layout of the top-level document as needed.

Test: plugins/unsupported-plugin-with-replacement-in-iframe-crash.html

* html/HTMLPlugInElement.cpp:
(WebCore::HTMLPlugInElement::isReplacementObscured):

LayoutTests:

Added a regression test. It hits the newly added debug assertion without the fix.

* platform/mac-wk1/TestExpectations: Skip the test since DumpRenderTree doesn't support
testRunner.setPluginSupportedMode.
* plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt: Added.
* plugins/unsupported-plugin-with-replacement-in-iframe-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk1/TestExpectations
LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt [new file with mode: 0644]
LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLPlugInElement.cpp

index 676ced2..c35b44e 100644 (file)
@@ -1,3 +1,17 @@
+2019-02-15  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Crash in the hit testing code via HTMLPlugInElement::isReplacementObscured()
+        https://bugs.webkit.org/show_bug.cgi?id=194691
+
+        Reviewed by Simon Fraser.
+
+        Added a regression test. It hits the newly added debug assertion without the fix.
+
+        * platform/mac-wk1/TestExpectations: Skip the test since DumpRenderTree doesn't support
+        testRunner.setPluginSupportedMode.
+        * plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt: Added.
+        * plugins/unsupported-plugin-with-replacement-in-iframe-crash.html: Added.
+
 2019-02-15  Nikita Vasilyev  <nvasilyev@apple.com>
 
         Web Inspector: Styles: valid values in style attributes are reported as unsupported property values
index d841cb4..4b593dd 100644 (file)
@@ -123,6 +123,7 @@ http/tests/plugins/supported-plugin-origin-specific-visibility.html [ Skip ]
 http/tests/plugins/supported-plugin-on-specific-origin.html [ Skip ]
 http/tests/plugins/unsupported-plugin-on-specific-origin.html [ Skip ]
 plugins/unsupported-plugin.html [ Skip ]
+plugins/unsupported-plugin-with-replacement-in-iframe-crash.html [ Skip ]
 
 # Color input is not yet implemented on Mac WK1. Currently, using it erroneously triggers an ASSERT_NOT_REACHED.
 webkit.org/b/119094 fast/forms/color/input-color-onchange-event.html [ Skip ]
diff --git a/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt b/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt
new file mode 100644 (file)
index 0000000..063842c
--- /dev/null
@@ -0,0 +1,8 @@
+CONSOLE MESSAGE: line 28: 1. Updating the layout with an embed object inside an iframe
+CONSOLE MESSAGE: line 22: 2. beforeload for the object fires and dirties the style tree
+CONSOLE MESSAGE: line 29: Tried to use an unsupported plug-in.
+CONSOLE MESSAGE: line 30: 3. Updated layout. The test passed.
+This tests entering HTMLPlugInElement::isReplacementObscured() while the top document's style tree is dirty.
+WebKit should update the layout of all documents and should not hit any debug assertions.
+
+PASS
diff --git a/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash.html b/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash.html
new file mode 100644 (file)
index 0000000..ce057cb
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests entering HTMLPlugInElement::isReplacementObscured() while the top document's style tree is dirty.<br>
+WebKit should update the layout of all documents and should not hit any debug assertions.</p>
+<style> input:focus { border: solid 5px black; } </style>
+<script>
+
+if (window.testRunner) {
+    testRunner.setPluginSupportedMode("none");
+    testRunner.dumpAsText();
+}
+
+document.body.getBoundingClientRect();
+
+const frame = document.createElement('iframe');
+document.body.appendChild(frame);
+const frameDocument = frame.contentDocument;
+frameDocument.body.innerHTML = '<object name="testPlugin" type="application/x-webkit-test-netscape" width="200" height="200"></object>';
+
+frameDocument.addEventListener('beforeload', () => {
+    console.log('2. beforeload for the object fires and dirties the style tree');
+    const input = document.createElement('input');
+    input.setAttribute('autofocus', '');
+    document.body.appendChild(input);
+}, true);
+
+console.log('1. Updating the layout with an embed object inside an iframe');
+const rect = frameDocument.querySelector('object').getBoundingClientRect();
+console.log('3. Updated layout. The test passed.');
+
+document.write('PASS');
+
+</script>
+</body>
+</html>
index 9258ebe..06bd2d0 100644 (file)
@@ -1,3 +1,21 @@
+2019-02-15  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Crash in the hit testing code via HTMLPlugInElement::isReplacementObscured()
+        https://bugs.webkit.org/show_bug.cgi?id=194691
+
+        Reviewed by Simon Fraser.
+
+        The crash was caused by HTMLPlugInElement::isReplacementObscured updating the document
+        without updating the layout of ancestor documents (i.e. documents in which frame owner
+        elements appear) even though it hit-tests against the top-level document's RenderView.
+
+        Fixed the bug by updating the layout of the top-level document as needed.
+
+        Test: plugins/unsupported-plugin-with-replacement-in-iframe-crash.html
+
+        * html/HTMLPlugInElement.cpp:
+        (WebCore::HTMLPlugInElement::isReplacementObscured):
+
 2019-02-15  Ross Kirsling  <ross.kirsling@sony.com>
 
         [WTF] Add environment variable helpers
index 1e04c5f..f619249 100644 (file)
@@ -425,13 +425,18 @@ bool HTMLPlugInElement::setReplacement(RenderEmbeddedObject::PluginUnavailabilit
 
 bool HTMLPlugInElement::isReplacementObscured()
 {
-    // We should always start hit testing a clean tree.
-    if (document().view())
-        document().view()->updateLayoutAndStyleIfNeededRecursive();
-    // Check if style recalc/layout destroyed the associated renderer.
-    auto* renderView = document().topDocument().renderView();
-    if (!document().view() || !renderView)
+    auto topDocument = makeRef(document().topDocument());
+    auto topFrameView = makeRefPtr(topDocument->view());
+    if (!topFrameView)
         return false;
+
+    topFrameView->updateLayoutAndStyleIfNeededRecursive();
+
+    // Updating the layout may have detached this document from the top document.
+    auto* renderView = topDocument->renderView();
+    if (!renderView || !document().view() || &document().topDocument() != topDocument.ptr())
+        return false;
+
     if (!renderer() || !is<RenderEmbeddedObject>(*renderer()))
         return false;
     auto& pluginRenderer = downcast<RenderEmbeddedObject>(*renderer());
@@ -457,6 +462,8 @@ bool HTMLPlugInElement::isReplacementObscured()
     HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::IgnoreClipping | HitTestRequest::DisallowUserAgentShadowContent | HitTestRequest::AllowChildFrameContent);
     HitTestResult result;
     HitTestLocation location = LayoutPoint(x + width / 2, y + height / 2);
+    ASSERT(!renderView->needsLayout());
+    ASSERT(!renderView->document().needsStyleRecalc());
     bool hit = renderView->hitTest(request, location, result);
     if (!hit || result.innerNode() != &pluginRenderer.frameOwnerElement())
         return true;