Title: [225723] trunk
- Revision
- 225723
- Author
- [email protected]
- Date
- 2017-12-09 02:41:32 -0800 (Sat, 09 Dec 2017)
Log Message
iOS: Crash in Document::updateLayout() via Document::processViewport
https://bugs.webkit.org/show_bug.cgi?id=180619
<rdar://problem/35717575>
Reviewed by Zalan Bujtas.
Source/WebCore:
The crash is caused by modern media controls updating the layout in the middle of insertedIntoAncestor
via HTMLMediaElement::setControllerJSProperty inside Document::pageScaleFactorChangedAndStable.
Fixed the crash by delaying the work to update the viewport configuration until didFinishInsertingNode
since updating the viewport configuration results in a lot of related code running in response,
and making sure all that code never tries to execute an author script is not attainable in the short term,
and a maintenance nightmare in the long term.
Test: media/ios/viewport-change-with-video.html
* html/HTMLMetaElement.cpp:
(WebCore::HTMLMetaElement::insertedIntoAncestor):
(WebCore::HTMLMetaElement::didFinishInsertingNode): Added.
* html/HTMLMetaElement.h:
LayoutTests:
Added a regression test for the crash.
* media/ios/viewport-change-with-video-expected.txt: Added.
* media/ios/viewport-change-with-video.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (225722 => 225723)
--- trunk/LayoutTests/ChangeLog 2017-12-09 07:39:47 UTC (rev 225722)
+++ trunk/LayoutTests/ChangeLog 2017-12-09 10:41:32 UTC (rev 225723)
@@ -1,3 +1,16 @@
+2017-12-09 Ryosuke Niwa <[email protected]>
+
+ iOS: Crash in Document::updateLayout() via Document::processViewport
+ https://bugs.webkit.org/show_bug.cgi?id=180619
+ <rdar://problem/35717575>
+
+ Reviewed by Zalan Bujtas.
+
+ Added a regression test for the crash.
+
+ * media/ios/viewport-change-with-video-expected.txt: Added.
+ * media/ios/viewport-change-with-video.html: Added.
+
2017-12-08 Zalan Bujtas <[email protected]>
Document::updateLayout() could destroy current frame.
Added: trunk/LayoutTests/media/ios/viewport-change-with-video-expected.txt (0 => 225723)
--- trunk/LayoutTests/media/ios/viewport-change-with-video-expected.txt (rev 0)
+++ trunk/LayoutTests/media/ios/viewport-change-with-video-expected.txt 2017-12-09 10:41:32 UTC (rev 225723)
@@ -0,0 +1,4 @@
+This tests changing the viewport after a video element. WebKit should not crash.
+
+
+PASS. WebKit didn't crash
Added: trunk/LayoutTests/media/ios/viewport-change-with-video.html (0 => 225723)
--- trunk/LayoutTests/media/ios/viewport-change-with-video.html (rev 0)
+++ trunk/LayoutTests/media/ios/viewport-change-with-video.html 2017-12-09 10:41:32 UTC (rev 225723)
@@ -0,0 +1,13 @@
+<!DOCTYPE html><!-- webkit-test-runner [ modernMediaControls=true useFlexibleViewport=true ] -->
+<html>
+<body>
+<p>This tests changing the viewport after a video element. WebKit should not crash.</p>
+<video src="" controls></video>
+<meta name="viewport" content="initial-scale=1, maximum-scale=1">
+<script>
+if (window.internals)
+ testRunner.dumpAsText();
+document.write("<div>PASS. WebKit didn't crash</div>");
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (225722 => 225723)
--- trunk/Source/WebCore/ChangeLog 2017-12-09 07:39:47 UTC (rev 225722)
+++ trunk/Source/WebCore/ChangeLog 2017-12-09 10:41:32 UTC (rev 225723)
@@ -1,3 +1,26 @@
+2017-12-09 Ryosuke Niwa <[email protected]>
+
+ iOS: Crash in Document::updateLayout() via Document::processViewport
+ https://bugs.webkit.org/show_bug.cgi?id=180619
+ <rdar://problem/35717575>
+
+ Reviewed by Zalan Bujtas.
+
+ The crash is caused by modern media controls updating the layout in the middle of insertedIntoAncestor
+ via HTMLMediaElement::setControllerJSProperty inside Document::pageScaleFactorChangedAndStable.
+
+ Fixed the crash by delaying the work to update the viewport configuration until didFinishInsertingNode
+ since updating the viewport configuration results in a lot of related code running in response,
+ and making sure all that code never tries to execute an author script is not attainable in the short term,
+ and a maintenance nightmare in the long term.
+
+ Test: media/ios/viewport-change-with-video.html
+
+ * html/HTMLMetaElement.cpp:
+ (WebCore::HTMLMetaElement::insertedIntoAncestor):
+ (WebCore::HTMLMetaElement::didFinishInsertingNode): Added.
+ * html/HTMLMetaElement.h:
+
2017-12-08 Zalan Bujtas <[email protected]>
Document::updateLayout() could destroy current frame.
Modified: trunk/Source/WebCore/html/HTMLMetaElement.cpp (225722 => 225723)
--- trunk/Source/WebCore/html/HTMLMetaElement.cpp 2017-12-09 07:39:47 UTC (rev 225722)
+++ trunk/Source/WebCore/html/HTMLMetaElement.cpp 2017-12-09 10:41:32 UTC (rev 225723)
@@ -64,10 +64,15 @@
{
HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
if (insertionType.connectedToDocument)
- process();
+ return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
return InsertedIntoAncestorResult::Done;
}
+void HTMLMetaElement::didFinishInsertingNode()
+{
+ process();
+}
+
void HTMLMetaElement::process()
{
// Changing a meta tag while it's not in the tree shouldn't have any effect on the document.
Modified: trunk/Source/WebCore/html/HTMLMetaElement.h (225722 => 225723)
--- trunk/Source/WebCore/html/HTMLMetaElement.h 2017-12-09 07:39:47 UTC (rev 225722)
+++ trunk/Source/WebCore/html/HTMLMetaElement.h 2017-12-09 10:41:32 UTC (rev 225723)
@@ -40,6 +40,7 @@
void parseAttribute(const QualifiedName&, const AtomicString&) final;
InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
+ void didFinishInsertingNode();
void process();
};
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
