Title: [254598] branches/safari-609-branch
Revision
254598
Author
alanc...@apple.com
Date
2020-01-15 11:15:10 -0800 (Wed, 15 Jan 2020)

Log Message

Cherry-pick r254201. rdar://problem/58552859

    [Web Animations] Stop creating CSS Animations for <noscript> elements
    https://bugs.webkit.org/show_bug.cgi?id=205925
    <rdar://problem/58158479>

    Reviewed by Antti Koivisto.

    Source/WebCore:

    Test: webanimations/no-css-animation-on-noscript.html

    It makes no sense to create CSS Animations for a <noscript> element and it has the side effect of potential crashes.
    Indeed, AnimationTimeline::updateCSSAnimationsForElement() may be called without a currentStyle and so we never have
    a list of previously-applied animations to compare to the list of animations in afterChangeStyle. So on each call we
    end up creating a new CSSAnimation and the previous animation for the same name is never explicitly removed from the
    effect stack and is eventually destroyed and the WeakPtr for it in the stack ends up being null, which would cause a
    crash under KeyframeEffectStack::ensureEffectsAreSorted().

    We now prevent elements such as <noscript> from being considered for CSS Animations in TreeResolver::resolveElement().

    * dom/Element.cpp:
    (WebCore::Element::rendererIsNeeded):
    * dom/Element.h:
    (WebCore::Element::rendererIsEverNeeded):
    * html/HTMLElement.cpp:
    (WebCore::HTMLElement::rendererIsEverNeeded):
    (WebCore::HTMLElement::rendererIsNeeded): Deleted.
    * html/HTMLElement.h:
    * style/StyleTreeResolver.cpp:
    (WebCore::Style::TreeResolver::resolveElement):

    LayoutTests:

    Add a new test that checks that setting the `animation` property on a <noscript> element does not yield the creation of a CSSAnimation object.

    * webanimations/no-css-animation-on-noscript-expected.txt: Added.
    * webanimations/no-css-animation-on-noscript.html: Added.

    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254201 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Modified Paths

Added Paths

Diff

Modified: branches/safari-609-branch/LayoutTests/ChangeLog (254597 => 254598)


--- branches/safari-609-branch/LayoutTests/ChangeLog	2020-01-15 19:15:05 UTC (rev 254597)
+++ branches/safari-609-branch/LayoutTests/ChangeLog	2020-01-15 19:15:10 UTC (rev 254598)
@@ -1,5 +1,61 @@
 2020-01-14  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r254201. rdar://problem/58552859
+
+    [Web Animations] Stop creating CSS Animations for <noscript> elements
+    https://bugs.webkit.org/show_bug.cgi?id=205925
+    <rdar://problem/58158479>
+    
+    Reviewed by Antti Koivisto.
+    
+    Source/WebCore:
+    
+    Test: webanimations/no-css-animation-on-noscript.html
+    
+    It makes no sense to create CSS Animations for a <noscript> element and it has the side effect of potential crashes.
+    Indeed, AnimationTimeline::updateCSSAnimationsForElement() may be called without a currentStyle and so we never have
+    a list of previously-applied animations to compare to the list of animations in afterChangeStyle. So on each call we
+    end up creating a new CSSAnimation and the previous animation for the same name is never explicitly removed from the
+    effect stack and is eventually destroyed and the WeakPtr for it in the stack ends up being null, which would cause a
+    crash under KeyframeEffectStack::ensureEffectsAreSorted().
+    
+    We now prevent elements such as <noscript> from being considered for CSS Animations in TreeResolver::resolveElement().
+    
+    * dom/Element.cpp:
+    (WebCore::Element::rendererIsNeeded):
+    * dom/Element.h:
+    (WebCore::Element::rendererIsEverNeeded):
+    * html/HTMLElement.cpp:
+    (WebCore::HTMLElement::rendererIsEverNeeded):
+    (WebCore::HTMLElement::rendererIsNeeded): Deleted.
+    * html/HTMLElement.h:
+    * style/StyleTreeResolver.cpp:
+    (WebCore::Style::TreeResolver::resolveElement):
+    
+    LayoutTests:
+    
+    Add a new test that checks that setting the `animation` property on a <noscript> element does not yield the creation of a CSSAnimation object.
+    
+    * webanimations/no-css-animation-on-noscript-expected.txt: Added.
+    * webanimations/no-css-animation-on-noscript.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254201 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-01-08  Antoine Quint  <grao...@apple.com>
+
+            [Web Animations] Stop creating CSS Animations for <noscript> elements
+            https://bugs.webkit.org/show_bug.cgi?id=205925
+            <rdar://problem/58158479>
+
+            Reviewed by Antti Koivisto.
+
+            Add a new test that checks that setting the `animation` property on a <noscript> element does not yield the creation of a CSSAnimation object.
+
+            * webanimations/no-css-animation-on-noscript-expected.txt: Added.
+            * webanimations/no-css-animation-on-noscript.html: Added.
+
+2020-01-14  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r254157. rdar://problem/58549081
 
     REGRESSION: [ Mac wk2 ] http/wpt/service-workers/persistent-importScripts.html is a flaky failure

Added: branches/safari-609-branch/LayoutTests/webanimations/no-css-animation-on-noscript-expected.txt (0 => 254598)


--- branches/safari-609-branch/LayoutTests/webanimations/no-css-animation-on-noscript-expected.txt	                        (rev 0)
+++ branches/safari-609-branch/LayoutTests/webanimations/no-css-animation-on-noscript-expected.txt	2020-01-15 19:15:10 UTC (rev 254598)
@@ -0,0 +1,3 @@
+
+PASS A CSS Animation cannot be created on a <noscript> element. 
+

Added: branches/safari-609-branch/LayoutTests/webanimations/no-css-animation-on-noscript.html (0 => 254598)


--- branches/safari-609-branch/LayoutTests/webanimations/no-css-animation-on-noscript.html	                        (rev 0)
+++ branches/safari-609-branch/LayoutTests/webanimations/no-css-animation-on-noscript.html	2020-01-15 19:15:10 UTC (rev 254598)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+
+noscript {
+    animation: animation 1s infinite;
+}
+
+@keyframes animation {
+    to { margin-left: "100px" };
+}
+
+</style>
+</head>
+<body>
+<script src=""
+<script src=""
+<noscript>Hello World</noscript>
+<script>
+
+test(() => {
+    assert_equals(document.querySelector("noscript").getAnimations().length, 0);
+}, "A CSS Animation cannot be created on a <noscript> element.");
+
+</script>
+</body>
+</html>

Modified: branches/safari-609-branch/Source/WebCore/ChangeLog (254597 => 254598)


--- branches/safari-609-branch/Source/WebCore/ChangeLog	2020-01-15 19:15:05 UTC (rev 254597)
+++ branches/safari-609-branch/Source/WebCore/ChangeLog	2020-01-15 19:15:10 UTC (rev 254598)
@@ -1,5 +1,78 @@
 2020-01-14  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r254201. rdar://problem/58552859
+
+    [Web Animations] Stop creating CSS Animations for <noscript> elements
+    https://bugs.webkit.org/show_bug.cgi?id=205925
+    <rdar://problem/58158479>
+    
+    Reviewed by Antti Koivisto.
+    
+    Source/WebCore:
+    
+    Test: webanimations/no-css-animation-on-noscript.html
+    
+    It makes no sense to create CSS Animations for a <noscript> element and it has the side effect of potential crashes.
+    Indeed, AnimationTimeline::updateCSSAnimationsForElement() may be called without a currentStyle and so we never have
+    a list of previously-applied animations to compare to the list of animations in afterChangeStyle. So on each call we
+    end up creating a new CSSAnimation and the previous animation for the same name is never explicitly removed from the
+    effect stack and is eventually destroyed and the WeakPtr for it in the stack ends up being null, which would cause a
+    crash under KeyframeEffectStack::ensureEffectsAreSorted().
+    
+    We now prevent elements such as <noscript> from being considered for CSS Animations in TreeResolver::resolveElement().
+    
+    * dom/Element.cpp:
+    (WebCore::Element::rendererIsNeeded):
+    * dom/Element.h:
+    (WebCore::Element::rendererIsEverNeeded):
+    * html/HTMLElement.cpp:
+    (WebCore::HTMLElement::rendererIsEverNeeded):
+    (WebCore::HTMLElement::rendererIsNeeded): Deleted.
+    * html/HTMLElement.h:
+    * style/StyleTreeResolver.cpp:
+    (WebCore::Style::TreeResolver::resolveElement):
+    
+    LayoutTests:
+    
+    Add a new test that checks that setting the `animation` property on a <noscript> element does not yield the creation of a CSSAnimation object.
+    
+    * webanimations/no-css-animation-on-noscript-expected.txt: Added.
+    * webanimations/no-css-animation-on-noscript.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254201 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-01-08  Antoine Quint  <grao...@apple.com>
+
+            [Web Animations] Stop creating CSS Animations for <noscript> elements
+            https://bugs.webkit.org/show_bug.cgi?id=205925
+            <rdar://problem/58158479>
+
+            Reviewed by Antti Koivisto.
+
+            Test: webanimations/no-css-animation-on-noscript.html
+
+            It makes no sense to create CSS Animations for a <noscript> element and it has the side effect of potential crashes.
+            Indeed, AnimationTimeline::updateCSSAnimationsForElement() may be called without a currentStyle and so we never have
+            a list of previously-applied animations to compare to the list of animations in afterChangeStyle. So on each call we
+            end up creating a new CSSAnimation and the previous animation for the same name is never explicitly removed from the
+            effect stack and is eventually destroyed and the WeakPtr for it in the stack ends up being null, which would cause a
+            crash under KeyframeEffectStack::ensureEffectsAreSorted().
+
+            We now prevent elements such as <noscript> from being considered for CSS Animations in TreeResolver::resolveElement().
+
+            * dom/Element.cpp:
+            (WebCore::Element::rendererIsNeeded):
+            * dom/Element.h:
+            (WebCore::Element::rendererIsEverNeeded):
+            * html/HTMLElement.cpp:
+            (WebCore::HTMLElement::rendererIsEverNeeded):
+            (WebCore::HTMLElement::rendererIsNeeded): Deleted.
+            * html/HTMLElement.h:
+            * style/StyleTreeResolver.cpp:
+            (WebCore::Style::TreeResolver::resolveElement):
+
+2020-01-14  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r254155. rdar://problem/58552864
 
     Add a move constructor to IDBResultData

Modified: branches/safari-609-branch/Source/WebCore/dom/Element.cpp (254597 => 254598)


--- branches/safari-609-branch/Source/WebCore/dom/Element.cpp	2020-01-15 19:15:05 UTC (rev 254597)
+++ branches/safari-609-branch/Source/WebCore/dom/Element.cpp	2020-01-15 19:15:10 UTC (rev 254598)
@@ -2114,7 +2114,7 @@
 
 bool Element::rendererIsNeeded(const RenderStyle& style)
 {
-    return style.display() != DisplayType::None && style.display() != DisplayType::Contents;
+    return rendererIsEverNeeded() && style.display() != DisplayType::None && style.display() != DisplayType::Contents;
 }
 
 RenderPtr<RenderElement> Element::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)

Modified: branches/safari-609-branch/Source/WebCore/dom/Element.h (254597 => 254598)


--- branches/safari-609-branch/Source/WebCore/dom/Element.h	2020-01-15 19:15:05 UTC (rev 254597)
+++ branches/safari-609-branch/Source/WebCore/dom/Element.h	2020-01-15 19:15:10 UTC (rev 254598)
@@ -285,6 +285,7 @@
 
     virtual RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&);
     virtual bool rendererIsNeeded(const RenderStyle&);
+    virtual bool rendererIsEverNeeded() { return true; }
 
     WEBCORE_EXPORT ShadowRoot* shadowRoot() const;
     ShadowRoot* shadowRootForBindings(JSC::JSGlobalObject&) const;

Modified: branches/safari-609-branch/Source/WebCore/html/HTMLElement.cpp (254597 => 254598)


--- branches/safari-609-branch/Source/WebCore/html/HTMLElement.cpp	2020-01-15 19:15:05 UTC (rev 254597)
+++ branches/safari-609-branch/Source/WebCore/html/HTMLElement.cpp	2020-01-15 19:15:10 UTC (rev 254598)
@@ -733,7 +733,7 @@
     setAttributeWithoutSynchronization(translateAttr, enable ? "yes" : "no");
 }
 
-bool HTMLElement::rendererIsNeeded(const RenderStyle& style)
+bool HTMLElement::rendererIsEverNeeded()
 {
     if (hasTagName(noscriptTag)) {
         RefPtr<Frame> frame = document().frame();
@@ -744,7 +744,7 @@
         if (frame && frame->loader().subframeLoader().allowPlugins())
             return false;
     }
-    return StyledElement::rendererIsNeeded(style);
+    return StyledElement::rendererIsEverNeeded();
 }
 
 RenderPtr<RenderElement> HTMLElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)

Modified: branches/safari-609-branch/Source/WebCore/html/HTMLElement.h (254597 => 254598)


--- branches/safari-609-branch/Source/WebCore/html/HTMLElement.h	2020-01-15 19:15:05 UTC (rev 254597)
+++ branches/safari-609-branch/Source/WebCore/html/HTMLElement.h	2020-01-15 19:15:10 UTC (rev 254598)
@@ -70,8 +70,8 @@
 
     void accessKeyAction(bool sendMouseEvents) override;
 
-    bool rendererIsNeeded(const RenderStyle&) override;
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) override;
+    bool rendererIsEverNeeded() final;
 
     WEBCORE_EXPORT virtual HTMLFormElement* form() const;
 

Modified: branches/safari-609-branch/Source/WebCore/style/StyleTreeResolver.cpp (254597 => 254598)


--- branches/safari-609-branch/Source/WebCore/style/StyleTreeResolver.cpp	2020-01-15 19:15:05 UTC (rev 254597)
+++ branches/safari-609-branch/Source/WebCore/style/StyleTreeResolver.cpp	2020-01-15 19:15:10 UTC (rev 254598)
@@ -198,6 +198,9 @@
         return { };
     }
 
+    if (!element.rendererIsEverNeeded())
+        return { };
+
     auto newStyle = styleForElement(element, parent().style);
 
     if (!affectsRenderedSubtree(element, *newStyle))
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to