Title: [232591] trunk
- Revision
- 232591
- Author
- [email protected]
- Date
- 2018-06-07 11:38:40 -0700 (Thu, 07 Jun 2018)
Log Message
Release assert in Document::updateLayout() in WebPage::determinePrimarySnapshottedPlugIn()
https://bugs.webkit.org/show_bug.cgi?id=186383
<rdar://problem/40849498>
Reviewed by Jon Lee.
Source/WebKit:
The release assert was hit because the descendent elemenet iterator, which instantiates ScriptDisallowedScope,
was alive as determinePrimarySnapshottedPlugIn invoked Document::updateLayout. Avoid this by copying
the list of plugin image elements into a vector first.
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::determinePrimarySnapshottedPlugIn): Fixed the release assert, and deployed Ref and RefPtr
to make this code safe.
LayoutTests:
Added a regression test.
* plugins/snapshotting/determine-primary-snapshotted-plugin-crash-expected.txt: Added.
* plugins/snapshotting/determine-primary-snapshotted-plugin-crash.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (232590 => 232591)
--- trunk/LayoutTests/ChangeLog 2018-06-07 18:36:47 UTC (rev 232590)
+++ trunk/LayoutTests/ChangeLog 2018-06-07 18:38:40 UTC (rev 232591)
@@ -1,3 +1,16 @@
+2018-06-07 Ryosuke Niwa <[email protected]>
+
+ Release assert in Document::updateLayout() in WebPage::determinePrimarySnapshottedPlugIn()
+ https://bugs.webkit.org/show_bug.cgi?id=186383
+ <rdar://problem/40849498>
+
+ Reviewed by Jon Lee.
+
+ Added a regression test.
+
+ * plugins/snapshotting/determine-primary-snapshotted-plugin-crash-expected.txt: Added.
+ * plugins/snapshotting/determine-primary-snapshotted-plugin-crash.html: Added.
+
2018-06-07 Thibault Saunier <[email protected]>
[GTK][WPE] Start implementing MediaStream API
Added: trunk/LayoutTests/plugins/snapshotting/determine-primary-snapshotted-plugin-crash-expected.txt (0 => 232591)
--- trunk/LayoutTests/plugins/snapshotting/determine-primary-snapshotted-plugin-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/plugins/snapshotting/determine-primary-snapshotted-plugin-crash-expected.txt 2018-06-07 18:38:40 UTC (rev 232591)
@@ -0,0 +1,4 @@
+This tests determining the primary snapshotted plugin does not hit an assertion.
+
+PASS
+
Added: trunk/LayoutTests/plugins/snapshotting/determine-primary-snapshotted-plugin-crash.html (0 => 232591)
--- trunk/LayoutTests/plugins/snapshotting/determine-primary-snapshotted-plugin-crash.html (rev 0)
+++ trunk/LayoutTests/plugins/snapshotting/determine-primary-snapshotted-plugin-crash.html 2018-06-07 18:38:40 UTC (rev 232591)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests determining the primary snapshotted plugin does not hit an assertion.</p>
+<div id="result">FAIL</div>
+<embed id="testPlugin" src="" type="application/x-webkit-test-netscape"></embed>
+<script>
+if (!window.testRunner)
+ result.textContent = 'This test requires WebKitTestRunner.';
+else {
+ internals.settings.setPlugInSnapshottingEnabled(true);
+ internals.settings.setMaximumPlugInSnapshotAttempts(0);
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+ setTimeout(() => {
+ const testPlugin = document.getElementById("testPlugin");
+ if (internals.isPluginSnapshotted(testPlugin))
+ result.textContent = 'PASS';
+ else
+ result.textContent = 'FAIL - not snapshot';
+ testRunner.notifyDone();
+ }, 500);
+}
+
+</script>
Modified: trunk/Source/WebKit/ChangeLog (232590 => 232591)
--- trunk/Source/WebKit/ChangeLog 2018-06-07 18:36:47 UTC (rev 232590)
+++ trunk/Source/WebKit/ChangeLog 2018-06-07 18:38:40 UTC (rev 232591)
@@ -1,3 +1,19 @@
+2018-06-07 Ryosuke Niwa <[email protected]>
+
+ Release assert in Document::updateLayout() in WebPage::determinePrimarySnapshottedPlugIn()
+ https://bugs.webkit.org/show_bug.cgi?id=186383
+ <rdar://problem/40849498>
+
+ Reviewed by Jon Lee.
+
+ The release assert was hit because the descendent elemenet iterator, which instantiates ScriptDisallowedScope,
+ was alive as determinePrimarySnapshottedPlugIn invoked Document::updateLayout. Avoid this by copying
+ the list of plugin image elements into a vector first.
+
+ * WebProcess/WebPage/WebPage.cpp:
+ (WebKit::WebPage::determinePrimarySnapshottedPlugIn): Fixed the release assert, and deployed Ref and RefPtr
+ to make this code safe.
+
2018-06-07 Don Olmstead <[email protected]>
[CoordGraphics] Fix compilation errors around USE(COORDINATED_GRAPHICS)
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (232590 => 232591)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-06-07 18:36:47 UTC (rev 232590)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp 2018-06-07 18:38:40 UTC (rev 232591)
@@ -5371,12 +5371,9 @@
layoutIfNeeded();
- auto& mainFrame = corePage()->mainFrame();
- if (!mainFrame.view())
+ RefPtr<FrameView> mainFrameView = corePage()->mainFrame().view();
+ if (!mainFrameView)
return;
- if (!mainFrame.view()->renderView())
- return;
- RenderView& mainRenderView = *mainFrame.view()->renderView();
IntRect searchRect = IntRect(IntPoint(), corePage()->mainFrame().view()->contentsSize());
searchRect.intersect(IntRect(IntPoint(), IntSize(primarySnapshottedPlugInSearchLimit, primarySnapshottedPlugInSearchLimit)));
@@ -5383,32 +5380,40 @@
HitTestRequest request(HitTestRequest::ReadOnly | HitTestRequest::Active | HitTestRequest::AllowChildFrameContent | HitTestRequest::IgnoreClipping | HitTestRequest::DisallowUserAgentShadowContent);
- HTMLPlugInImageElement* candidatePlugIn = nullptr;
+ RefPtr<HTMLPlugInImageElement> candidatePlugIn;
unsigned candidatePlugInArea = 0;
- for (Frame* frame = &mainFrame; frame; frame = frame->tree().traverseNextRendered()) {
+ for (RefPtr<Frame> frame = &corePage()->mainFrame(); frame; frame = frame->tree().traverseNextRendered()) {
if (!frame->loader().subframeLoader().containsPlugins())
continue;
if (!frame->document() || !frame->view())
continue;
+
+ Vector<Ref<HTMLPlugInImageElement>> nonPlayingPlugInImageElements;
for (auto& plugInImageElement : descendantsOfType<HTMLPlugInImageElement>(*frame->document())) {
if (plugInImageElement.displayState() == HTMLPlugInElement::Playing)
continue;
+ nonPlayingPlugInImageElements.append(plugInImageElement);
+ }
- auto pluginRenderer = plugInImageElement.renderer();
+ for (auto& plugInImageElement : nonPlayingPlugInImageElements) {
+ auto pluginRenderer = plugInImageElement->renderer();
if (!pluginRenderer || !pluginRenderer->isBox())
continue;
auto& pluginRenderBox = downcast<RenderBox>(*pluginRenderer);
- if (!plugInIntersectsSearchRect(plugInImageElement))
+ if (!plugInIntersectsSearchRect(plugInImageElement.get()))
continue;
- IntRect plugInRectRelativeToView = plugInImageElement.clientRect();
- ScrollPosition scrollPosition = mainFrame.view()->documentScrollPositionRelativeToViewOrigin();
+ IntRect plugInRectRelativeToView = plugInImageElement->clientRect();
+ ScrollPosition scrollPosition = mainFrameView->documentScrollPositionRelativeToViewOrigin();
IntRect plugInRectRelativeToTopDocument(plugInRectRelativeToView.location() + scrollPosition, plugInRectRelativeToView.size());
HitTestResult hitTestResult(plugInRectRelativeToTopDocument.center());
- mainRenderView.hitTest(request, hitTestResult);
- Element* element = hitTestResult.targetElement();
+ if (!mainFrameView->renderView())
+ return;
+ mainFrameView->renderView()->hitTest(request, hitTestResult);
+
+ RefPtr<Element> element = hitTestResult.targetElement();
if (!element)
continue;
@@ -5420,7 +5425,7 @@
inflatedPluginRect.inflateX(xOffset);
inflatedPluginRect.inflateY(yOffset);
- if (element != &plugInImageElement) {
+ if (element != plugInImageElement.ptr()) {
if (!(is<HTMLImageElement>(*element)
&& inflatedPluginRect.contains(elementRectRelativeToTopDocument)
&& elementRectRelativeToTopDocument.width() > pluginRenderBox.width() * minimumOverlappingImageToPluginDimensionScale
@@ -5427,11 +5432,11 @@
&& elementRectRelativeToTopDocument.height() > pluginRenderBox.height() * minimumOverlappingImageToPluginDimensionScale))
continue;
LOG(Plugins, "Primary Plug-In Detection: Plug-in is hidden by an image that is roughly aligned with it, autoplaying regardless of whether or not it's actually the primary plug-in.");
- plugInImageElement.restartSnapshottedPlugIn();
+ plugInImageElement->restartSnapshottedPlugIn();
}
if (plugInIsPrimarySize(plugInImageElement, candidatePlugInArea))
- candidatePlugIn = &plugInImageElement;
+ candidatePlugIn = WTFMove(plugInImageElement);
}
}
if (!candidatePlugIn) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
