Changeset 208370 in webkit

Timestamp:

Nov 3, 2016, 10:08:29 PM (8 years ago)

Author:

Antti Koivisto

Message:

Source/WebCore:
REGRESSION (r207669): Crash under media controls shadow root construction
​https://bugs.webkit.org/show_bug.cgi?id=164381
<rdar://problem/28935401>

Reviewed by Simon Fraser.

The problem is that we are running a script for media control UA shadow tree in HTMLMediaElement::insertedInto.
It is not safe to run scripts in insertedInto as the tree is in inconsistent state. Instead finishedInsertingSubtree
callback should be used.

Test: media/media-controls-shadow-construction-crash.html

Seen on ​https://www.theguardian.com/artanddesign/video/2013/oct/14/banksy-central-park-new-york-video

• html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::insertedInto):
(WebCore::HTMLMediaElement::finishedInsertingSubtree):

Move configureMediaControls() to finishedInsertingSubtree().

• html/HTMLMediaElement.h:
• style/StyleTreeResolver.cpp:

(WebCore::Style::TreeResolver::resolveComposedTree):

Add an assert to make the bad state easier to hit in tests.

LayoutTests:
REGRESSION (r207669): Crash under SVGRenderSupport::updateMaskedAncestorShouldIsolateBlending
​https://bugs.webkit.org/show_bug.cgi?id=164381
<rdar://problem/28935401>

Reviewed by Simon Fraser.

• media/media-controls-shadow-construction-crash-expected.txt: Added.
• media/media-controls-shadow-construction-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/media-controls-shadow-construction-crash-expected.txt (added)
• LayoutTests/media/media-controls-shadow-construction-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLMediaElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLMediaElement.h (modified) (1 diff)
• Source/WebCore/style/StyleTreeResolver.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2016-11-03  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r207669): Crash under SVGRenderSupport::updateMaskedAncestorShouldIsolateBlending
+        https://bugs.webkit.org/show_bug.cgi?id=164381
+        <rdar://problem/28935401>
+
+        Reviewed by Simon Fraser.
+
+        * media/media-controls-shadow-construction-crash-expected.txt: Added.
+        * media/media-controls-shadow-construction-crash.html: Added.
+
 2016-11-03  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-11-03  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION (r207669): Crash under media controls shadow root construction
+        https://bugs.webkit.org/show_bug.cgi?id=164381
+        <rdar://problem/28935401>
+
+        Reviewed by Simon Fraser.
+
+        The problem is that we are running a script for media control UA shadow tree in HTMLMediaElement::insertedInto.
+        It is not safe to run scripts in insertedInto as the tree is in inconsistent state. Instead finishedInsertingSubtree
+        callback should be used.
+
+        Test: media/media-controls-shadow-construction-crash.html
+
+        Seen on https://www.theguardian.com/artanddesign/video/2013/oct/14/banksy-central-park-new-york-video
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::insertedInto):
+        (WebCore::HTMLMediaElement::finishedInsertingSubtree):
+
+            Move configureMediaControls() to finishedInsertingSubtree().
+
+        * html/HTMLMediaElement.h:
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::resolveComposedTree):
+
+            Add an assert to make the bad state easier to hit in tests.
+
 2016-11-03  Ryosuke Niwa  <rniwa@webkit.org>
 

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -808 +808 @@
     }
 
+    return InsertionShouldCallFinishedInsertingSubtree;
+}
+
+void HTMLMediaElement::finishedInsertingSubtree()
+{
     configureMediaControls();
-    return InsertionDone;
 }
 

trunk/Source/WebCore/html/HTMLMediaElement.h

@@ -522 +522 @@
     bool childShouldCreateRenderer(const Node&) const override;
     InsertionNotificationRequest insertedInto(ContainerNode&) override;
+    void finishedInsertingSubtree() override;
     void removedFrom(ContainerNode&) override;
     void didRecalcStyle(Style::Change) override;

trunk/Source/WebCore/style/StyleTreeResolver.cpp

@@ -358 +358 @@
         auto& parent = this->parent();
 
+        ASSERT(node.inDocument());
         ASSERT(node.containingShadowRoot() == scope().shadowRoot);
         ASSERT(node.parentElement() == parent.element || is<ShadowRoot>(node.parentNode()) || node.parentElement()->shadowRoot());