Changeset 225723 in webkit

Timestamp:

Dec 9, 2017 2:41:32 AM (6 years ago)

Author:

rniwa@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/ios/viewport-change-with-video-expected.txt (added)
• LayoutTests/media/ios/viewport-change-with-video.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLMetaElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLMetaElement.h (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2017-12-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        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  <zalan@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2017-12-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        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  <zalan@apple.com>
 

trunk/Source/WebCore/html/HTMLMetaElement.cpp

@@ -65 +65 @@
     HTMLElement::insertedIntoAncestor(insertionType, parentOfInsertedTree);
     if (insertionType.connectedToDocument)
-        process();
+        return InsertedIntoAncestorResult::NeedsPostInsertionCallback;
     return InsertedIntoAncestorResult::Done;
+}
+
+void HTMLMetaElement::didFinishInsertingNode()
+{
+    process();
 }
 

trunk/Source/WebCore/html/HTMLMetaElement.h

@@ -41 +41 @@
     void parseAttribute(const QualifiedName&, const AtomicString&) final;
     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
+    void didFinishInsertingNode();
 
     void process();