Crash in WebCore::RenderLayer::FilterInfo::updateReferenceFilterClients
authorjhoneycutt@apple.com <jhoneycutt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Apr 2014 02:48:14 +0000 (02:48 +0000)
committerjhoneycutt@apple.com <jhoneycutt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Apr 2014 02:48:14 +0000 (02:48 +0000)
<https://bugs.webkit.org/show_bug.cgi?id=121887>
<rdar://problem/15073043>

Reviewed by Dean Jackson.

Source/WebCore:

Test: svg/filters/first-letter-crash.html

* rendering/FilterEffectRenderer.cpp:
(WebCore::FilterEffectRenderer::buildReferenceFilter):
Added a null check to prevent crashes for anonymous RenderObjects.

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::filterNeedsRepaint):
Get the enclosing element, if there is one, and recalculate its style.
We use the enclosing element so that we recalculate style for the
ancestor of an anonymous RenderElement.
(WebCore::RenderLayer::enclosingElement):
Remove an assertion; we may now reach this condition if loading a
cached SVG document results in RenderLayer::filterNeedsRepaint() being
called before the object has been inserted into the render tree.

* rendering/RenderLayerFilterInfo.cpp:
(WebCore::RenderLayer::FilterInfo::notifyFinished):
Tell the RenderLayer that the filter needs repainting.
(WebCore::RenderLayer::FilterInfo::updateReferenceFilterClients):
Get the Element from the renderer rather than asking the renderer's
Element, which will be null for anonymous RenderObjects.

* rendering/RenderLayerFilterInfo.h:
Removed declaration for the old workaround function, layerElement().

LayoutTests:

* svg/filters/first-letter-crash-expected.txt: Added.
* svg/filters/first-letter-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/svg/filters/first-letter-crash-expected.txt [new file with mode: 0644]
LayoutTests/svg/filters/first-letter-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/FilterEffectRenderer.cpp
Source/WebCore/rendering/RenderLayer.cpp
Source/WebCore/rendering/RenderLayerFilterInfo.cpp
Source/WebCore/rendering/RenderLayerFilterInfo.h

index 821a719..e780417 100644 (file)
@@ -1,3 +1,15 @@
+2014-04-01  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        Crash in WebCore::RenderLayer::FilterInfo::updateReferenceFilterClients
+
+        <https://bugs.webkit.org/show_bug.cgi?id=121887>
+        <rdar://problem/15073043>
+
+        Reviewed by Dean Jackson.
+
+        * svg/filters/first-letter-crash-expected.txt: Added.
+        * svg/filters/first-letter-crash.html: Added.
+
 2014-04-01  Zoltan Horvath  <zoltan@webkit.org>
 
         [CSS Exclusions] Remove exclusions parsing support
diff --git a/LayoutTests/svg/filters/first-letter-crash-expected.txt b/LayoutTests/svg/filters/first-letter-crash-expected.txt
new file mode 100644 (file)
index 0000000..db0ecd1
--- /dev/null
@@ -0,0 +1,2 @@
+PASS.
+WebKit bug #121887: Crash when applying SVG filter to first-letter pseudo element. This test passes if it does not crash.
diff --git a/LayoutTests/svg/filters/first-letter-crash.html b/LayoutTests/svg/filters/first-letter-crash.html
new file mode 100644 (file)
index 0000000..c620c09
--- /dev/null
@@ -0,0 +1,16 @@
+<head>
+    <style>
+        div:first-letter { -webkit-filter: url(#blurFirstLetter); }
+    </style>
+</head>
+
+<div>PASS.</div>
+
+<p>
+    WebKit bug #<a href="https://bugs.webkit.org/show_bug.cgi?id=121887">121887</a>: Crash when applying SVG filter to first-letter pseudo element. This test passes if it does not crash.
+</p>
+
+<script>
+    if (window.testRunner)
+        window.testRunner.dumpAsText();
+</script>
index 9ce44b2..77b2935 100644 (file)
@@ -1,3 +1,38 @@
+2014-04-01  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        Crash in WebCore::RenderLayer::FilterInfo::updateReferenceFilterClients
+
+        <https://bugs.webkit.org/show_bug.cgi?id=121887>
+        <rdar://problem/15073043>
+
+        Reviewed by Dean Jackson.
+
+        Test: svg/filters/first-letter-crash.html
+
+        * rendering/FilterEffectRenderer.cpp:
+        (WebCore::FilterEffectRenderer::buildReferenceFilter):
+        Added a null check to prevent crashes for anonymous RenderObjects.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::filterNeedsRepaint):
+        Get the enclosing element, if there is one, and recalculate its style.
+        We use the enclosing element so that we recalculate style for the
+        ancestor of an anonymous RenderElement.
+        (WebCore::RenderLayer::enclosingElement):
+        Remove an assertion; we may now reach this condition if loading a
+        cached SVG document results in RenderLayer::filterNeedsRepaint() being
+        called before the object has been inserted into the render tree.
+
+        * rendering/RenderLayerFilterInfo.cpp:
+        (WebCore::RenderLayer::FilterInfo::notifyFinished):
+        Tell the RenderLayer that the filter needs repainting.
+        (WebCore::RenderLayer::FilterInfo::updateReferenceFilterClients):
+        Get the Element from the renderer rather than asking the renderer's
+        Element, which will be null for anonymous RenderObjects.
+
+        * rendering/RenderLayerFilterInfo.h:
+        Removed declaration for the old workaround function, layerElement().
+
 2014-04-01  Ryuan Choi  <ryuan.choi@samsung.com>
 
         Build break when disabled VIDEO since r166261
index 0f3c144..e13ffac 100644 (file)
@@ -104,9 +104,10 @@ PassRefPtr<FilterEffect> FilterEffectRenderer::buildReferenceFilter(RenderElemen
 
     Element* filter = document->getElementById(filterOperation->fragment());
     if (!filter) {
-        // Although we did not find the referenced filter, it might exist later
-        // in the document
-        document->accessSVGExtensions()->addPendingResource(filterOperation->fragment(), renderer->element());
+        // Although we did not find the referenced filter, it might exist later in the document.
+        // FIXME: This skips anonymous RenderObjects. <https://webkit.org/b/131085>
+        if (Element* element = renderer->element())
+            document->accessSVGExtensions()->addPendingResource(filterOperation->fragment(), element);
         return 0;
     }
 
index c4ca6ec..287eddf 100644 (file)
@@ -4650,7 +4650,6 @@ Element* RenderLayer::enclosingElement() const
         if (Element* e = r->element())
             return e;
     }
-    ASSERT_NOT_REACHED();
     return 0;
 }
 
@@ -6804,7 +6803,9 @@ void RenderLayer::updateOrRemoveFilterEffectRenderer()
 
 void RenderLayer::filterNeedsRepaint()
 {
-    renderer().element()->setNeedsStyleRecalc(SyntheticStyleChange);
+    // We use the enclosing element so that we recalculate style for the ancestor of an anonymous object.
+    if (Element* element = enclosingElement())
+        element->setNeedsStyleRecalc(SyntheticStyleChange);
     renderer().repaint();
 }
 
index 397eafe..52906e7 100644 (file)
@@ -93,16 +93,9 @@ void RenderLayer::FilterInfo::setRenderer(PassRefPtr<FilterEffectRenderer> rende
 
 void RenderLayer::FilterInfo::notifyFinished(CachedResource*)
 {
-    m_layer.renderer().element()->setNeedsStyleRecalc(SyntheticStyleChange);
-    m_layer.renderer().repaint();
+    m_layer.filterNeedsRepaint();
 }
-
-// FIXME: Remove this helper function when <rdar://problem/16230015> is fixed.
-NEVER_INLINE Element* RenderLayer::FilterInfo::layerElement() const
-{
-    return m_layer.renderer().element();
-}
-
+    
 void RenderLayer::FilterInfo::updateReferenceFilterClients(const FilterOperations& operations)
 {
     removeReferenceFilterClients();
@@ -121,7 +114,7 @@ void RenderLayer::FilterInfo::updateReferenceFilterClients(const FilterOperation
         } else {
             // Reference is internal; add layer as a client so we can trigger
             // filter repaint on SVG attribute change.
-            Element* filter = layerElement()->document().getElementById(referenceFilterOperation->fragment());
+            Element* filter = m_layer.renderer().document().getElementById(referenceFilterOperation->fragment());
 
             if (!filter || !filter->renderer() || !filter->renderer()->isSVGResourceFilter())
                 continue;
index 0eb3353..71e80e3 100644 (file)
@@ -63,8 +63,6 @@ public:
     void removeReferenceFilterClients();
 
 private:
-    Element* layerElement() const;
-
     friend void WTF::deleteOwnedPtr<FilterInfo>(FilterInfo*);
 
     virtual void notifyFinished(CachedResource*) override;