Title: [242008] releases/WebKitGTK/webkit-2.22
Revision
242008
Author
ape...@igalia.com
Date
2019-02-23 17:07:20 -0800 (Sat, 23 Feb 2019)

Log Message

Merged r241626 - Crash in the hit testing code via HTMLPlugInElement::isReplacementObscured()
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.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog (242007 => 242008)


--- releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog	2019-02-24 01:07:11 UTC (rev 242007)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/ChangeLog	2019-02-24 01:07:20 UTC (rev 242008)
@@ -1,3 +1,17 @@
+2019-02-15  Ryosuke Niwa  <rn...@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-13  Ryosuke Niwa  <rn...@webkit.org>
 
         Crash in DOMTimer::fired

Modified: releases/WebKitGTK/webkit-2.22/LayoutTests/platform/mac-wk1/TestExpectations (242007 => 242008)


--- releases/WebKitGTK/webkit-2.22/LayoutTests/platform/mac-wk1/TestExpectations	2019-02-24 01:07:11 UTC (rev 242007)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/platform/mac-wk1/TestExpectations	2019-02-24 01:07:20 UTC (rev 242008)
@@ -121,6 +121,7 @@
 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 ]

Added: releases/WebKitGTK/webkit-2.22/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt (0 => 242008)


--- releases/WebKitGTK/webkit-2.22/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash-expected.txt	2019-02-24 01:07:20 UTC (rev 242008)
@@ -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

Added: releases/WebKitGTK/webkit-2.22/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash.html (0 => 242008)


--- releases/WebKitGTK/webkit-2.22/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-2.22/LayoutTests/plugins/unsupported-plugin-with-replacement-in-iframe-crash.html	2019-02-24 01:07:20 UTC (rev 242008)
@@ -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>

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog (242007 => 242008)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog	2019-02-24 01:07:11 UTC (rev 242007)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/ChangeLog	2019-02-24 01:07:20 UTC (rev 242008)
@@ -1,3 +1,21 @@
+2019-02-15  Ryosuke Niwa  <rn...@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-13  Ryosuke Niwa  <rn...@webkit.org>
 
         Crash in DOMTimer::fired

Modified: releases/WebKitGTK/webkit-2.22/Source/WebCore/html/HTMLPlugInElement.cpp (242007 => 242008)


--- releases/WebKitGTK/webkit-2.22/Source/WebCore/html/HTMLPlugInElement.cpp	2019-02-24 01:07:11 UTC (rev 242007)
+++ releases/WebKitGTK/webkit-2.22/Source/WebCore/html/HTMLPlugInElement.cpp	2019-02-24 01:07:20 UTC (rev 242008)
@@ -425,13 +425,18 @@
 
 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 @@
     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;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to