Title: [184749] trunk
Revision
184749
Author
an...@apple.com
Date
2015-05-21 20:59:16 -0700 (Thu, 21 May 2015)

Log Message

MaskImageOperation code does not manage CachedImageClients correctly
https://bugs.webkit.org/show_bug.cgi?id=145276
Source/WebCore:

rdar://problem/20959822

Reviewed by Andreas Kling.

Test: css3/masking/mask-image-client-crash.html

* platform/graphics/MaskImageOperation.cpp:
(WebCore::MaskImageOperation::isMaskLoaded):
(WebCore::MaskImageOperation::setImage):

    If image changes transfer the clients to the new image.

(WebCore::MaskImageOperation::setRenderLayerImageClient):

    Always use setImage.

(WebCore::MaskImageOperation::notifyFinished):
* platform/graphics/MaskImageOperation.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::calculateClipRects):
* rendering/RenderLayer.h:
* rendering/RenderLayerMaskImageInfo.cpp:
(WebCore::RenderLayer::MaskImageInfo::~MaskImageInfo):
(WebCore::RenderLayer::MaskImageInfo::updateMaskImageClients):

    Store the MaskImageOperations where we added clients.

(WebCore::RenderLayer::MaskImageInfo::removeMaskImageClients):

    Make sure we remove the clients from the same MaskImageOperations we added them to.

* rendering/RenderLayerMaskImageInfo.h:

LayoutTests:


Reviewed by Andreas Kling.

* css3/masking/mask-image-client-crash-expected.txt: Added.
* css3/masking/mask-image-client-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (184748 => 184749)


--- trunk/LayoutTests/ChangeLog	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/LayoutTests/ChangeLog	2015-05-22 03:59:16 UTC (rev 184749)
@@ -1,3 +1,13 @@
+2015-05-21  Antti Koivisto  <an...@apple.com>
+
+        MaskImageOperation code does not manage CachedImageClients correctly
+        https://bugs.webkit.org/show_bug.cgi?id=145276
+
+        Reviewed by Andreas Kling.
+
+        * css3/masking/mask-image-client-crash-expected.txt: Added.
+        * css3/masking/mask-image-client-crash.html: Added.
+
 2015-05-21  Daniel Bates  <daba...@apple.com>
 
         Update Autofill button icon

Added: trunk/LayoutTests/css3/masking/mask-image-client-crash-expected.txt (0 => 184749)


--- trunk/LayoutTests/css3/masking/mask-image-client-crash-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/css3/masking/mask-image-client-crash-expected.txt	2015-05-22 03:59:16 UTC (rev 184749)
@@ -0,0 +1,2 @@
+This test passes if it doesn't crash or assert. 
+

Added: trunk/LayoutTests/css3/masking/mask-image-client-crash.html (0 => 184749)


--- trunk/LayoutTests/css3/masking/mask-image-client-crash.html	                        (rev 0)
+++ trunk/LayoutTests/css3/masking/mask-image-client-crash.html	2015-05-22 03:59:16 UTC (rev 184749)
@@ -0,0 +1,24 @@
+<!DOCTYPE HTML>
+<html id="webtest0">
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
+<head id="webtest1">
+<link id="webtest2" rel="stylesheet" href=""
+</head>
+<body>
+This test passes if it doesn't crash or assert.
+<a id="webtest4" href=""
+<br id="webtest12" clear="all"/>
+</body>
+
+<script>
+document.getElementById("webtest4").setAttribute("style","page-break-before:always;-webkit-mask-image:url(data:image/gif;base64,R0lGODdhAQACAPABAAD/AP///ywAAAAAAQACAAACAkQKADs=);");
+try {
+document.getElementById("webtest12").appendChild(document.createElement("applet")).setAttributeNodeNS(document.getElementById("webtest10"));
+} catch(e) {
+}
+document.getElementById("webtest4").setAttribute("style","text-rendering:auto;");
+</script>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (184748 => 184749)


--- trunk/Source/WebCore/ChangeLog	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/Source/WebCore/ChangeLog	2015-05-22 03:59:16 UTC (rev 184749)
@@ -1,3 +1,40 @@
+2015-05-21  Antti Koivisto  <an...@apple.com>
+
+        MaskImageOperation code does not manage CachedImageClients correctly
+        https://bugs.webkit.org/show_bug.cgi?id=145276
+        rdar://problem/20959822
+
+        Reviewed by Andreas Kling.
+
+        Test: css3/masking/mask-image-client-crash.html
+
+        * platform/graphics/MaskImageOperation.cpp:
+        (WebCore::MaskImageOperation::isMaskLoaded):
+        (WebCore::MaskImageOperation::setImage):
+
+            If image changes transfer the clients to the new image.
+
+        (WebCore::MaskImageOperation::setRenderLayerImageClient):
+
+            Always use setImage.
+
+        (WebCore::MaskImageOperation::notifyFinished):
+        * platform/graphics/MaskImageOperation.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::calculateClipRects):
+        * rendering/RenderLayer.h:
+        * rendering/RenderLayerMaskImageInfo.cpp:
+        (WebCore::RenderLayer::MaskImageInfo::~MaskImageInfo):
+        (WebCore::RenderLayer::MaskImageInfo::updateMaskImageClients):
+
+            Store the MaskImageOperations where we added clients.
+
+        (WebCore::RenderLayer::MaskImageInfo::removeMaskImageClients):
+
+            Make sure we remove the clients from the same MaskImageOperations we added them to.
+
+        * rendering/RenderLayerMaskImageInfo.h:
+
 2015-05-21  Daniel Bates  <daba...@apple.com>
 
         Update Autofill button icon

Modified: trunk/Source/WebCore/platform/graphics/MaskImageOperation.cpp (184748 => 184749)


--- trunk/Source/WebCore/platform/graphics/MaskImageOperation.cpp	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/Source/WebCore/platform/graphics/MaskImageOperation.cpp	2015-05-22 03:59:16 UTC (rev 184749)
@@ -129,6 +129,28 @@
     return false;
 }
 
+void MaskImageOperation::setImage(PassRefPtr<StyleImage> image)
+{
+    if (m_styleImage == image)
+        return;
+
+    if (m_styleImage) {
+        if (m_renderLayerImageClient && m_styleImage->cachedImage())
+            m_styleImage->cachedImage()->removeClient(m_renderLayerImageClient);
+        for (auto& client : m_rendererImageClients)
+            m_styleImage->removeClient(client.key);
+    }
+
+    m_styleImage = image;
+
+    if (m_styleImage) {
+        if (m_renderLayerImageClient && m_styleImage->cachedImage())
+            m_styleImage->cachedImage()->addClient(m_renderLayerImageClient);
+        for (auto& client : m_rendererImageClients)
+            m_styleImage->addClient(client.key);
+    }
+}
+
 void MaskImageOperation::setRenderLayerImageClient(CachedImageClient* client)
 {
     if (m_renderLayerImageClient == client)
@@ -205,11 +227,7 @@
         ASSERT(cachedSVGDocument->loader());
         if (SubresourceLoader* loader = cachedSVGDocument->loader()) {
             if (SharedBuffer* dataBuffer = loader->resourceData()) {
-                m_styleImage = StyleCachedImage::create(new CachedImage(cachedSVGDocument->resourceRequest(), cachedSVGDocument->sessionID()));
-                if (m_renderLayerImageClient)
-                    m_styleImage->cachedImage()->addClient(m_renderLayerImageClient);
-                for (auto itClient : m_rendererImageClients)
-                    m_styleImage->addClient(itClient.key);
+                setImage(StyleCachedImage::create(new CachedImage(cachedSVGDocument->resourceRequest(), cachedSVGDocument->sessionID())));
 
                 m_styleImage->cachedImage()->setResponse(cachedSVGDocument->response());
                 m_styleImage->cachedImage()->finishLoading(dataBuffer);

Modified: trunk/Source/WebCore/platform/graphics/MaskImageOperation.h (184748 => 184749)


--- trunk/Source/WebCore/platform/graphics/MaskImageOperation.h	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/Source/WebCore/platform/graphics/MaskImageOperation.h	2015-05-22 03:59:16 UTC (rev 184749)
@@ -63,7 +63,7 @@
     const String& fragment() const { return m_fragment; }
     bool isExternalDocument() const { return m_isExternalDocument; }
     StyleImage* image() const { return m_styleImage.get(); }
-    void setImage(PassRefPtr<StyleImage> image) { m_styleImage = image; }
+    void setImage(PassRefPtr<StyleImage>);
     void setRenderLayerImageClient(CachedImageClient*);
     void addRendererImageClient(RenderElement*);
     void removeRendererImageClient(RenderElement*);

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (184748 => 184749)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2015-05-22 03:59:16 UTC (rev 184749)
@@ -6708,7 +6708,7 @@
     updateBlendMode();
 #endif
     updateOrRemoveFilterClients();
-    updateOrRemoveMaskImageClients(oldStyle);
+    updateOrRemoveMaskImageClients();
 
     updateNeedsCompositedScrolling();
 
@@ -6863,17 +6863,12 @@
         filterInfo->removeReferenceFilterClients();
 }
 
-void RenderLayer::updateOrRemoveMaskImageClients(const RenderStyle* oldStyle)
+void RenderLayer::updateOrRemoveMaskImageClients()
 {
-    if (oldStyle && oldStyle->maskImage().get()) {
-        if (MaskImageInfo* maskImageInfo = MaskImageInfo::getIfExists(*this))
-            maskImageInfo->removeMaskImageClients(*oldStyle);
-    }
-
     if (renderer().style().maskImage().get())
         MaskImageInfo::get(*this).updateMaskImageClients();
     else if (MaskImageInfo* maskImageInfo = MaskImageInfo::getIfExists(*this))
-        maskImageInfo->removeMaskImageClients(renderer().style());
+        maskImageInfo->removeMaskImageClients();
 }
 
 void RenderLayer::updateOrRemoveFilterEffectRenderer()

Modified: trunk/Source/WebCore/rendering/RenderLayer.h (184748 => 184749)


--- trunk/Source/WebCore/rendering/RenderLayer.h	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/Source/WebCore/rendering/RenderLayer.h	2015-05-22 03:59:16 UTC (rev 184749)
@@ -920,9 +920,8 @@
 
     void updateOrRemoveFilterClients();
     void updateOrRemoveFilterEffectRenderer();
+    void updateOrRemoveMaskImageClients();
 
-    void updateOrRemoveMaskImageClients(const RenderStyle* oldStyle);
-
 #if ENABLE(CSS_COMPOSITING)
     void updateAncestorChainHasBlendingDescendants();
     void dirtyAncestorChainHasBlendingDescendants();

Modified: trunk/Source/WebCore/rendering/RenderLayerMaskImageInfo.cpp (184748 => 184749)


--- trunk/Source/WebCore/rendering/RenderLayerMaskImageInfo.cpp	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/Source/WebCore/rendering/RenderLayerMaskImageInfo.cpp	2015-05-22 03:59:16 UTC (rev 184749)
@@ -106,16 +106,18 @@
 
 RenderLayer::MaskImageInfo::~MaskImageInfo()
 {
-    removeMaskImageClients(m_layer.renderer().style());
+    removeMaskImageClients();
 }
 
 void RenderLayer::MaskImageInfo::updateMaskImageClients()
 {
-    removeMaskImageClients(m_layer.renderer().style());
+    removeMaskImageClients();
     
     for (auto* maskLayer = m_layer.renderer().style().maskLayers(); maskLayer; maskLayer = maskLayer->next()) {
-        const RefPtr<MaskImageOperation> maskImage = maskLayer->maskImage();
+        RefPtr<MaskImageOperation> maskImage = maskLayer->maskImage();
         maskImage->setRenderLayerImageClient(m_imageClient.get());
+        m_maskImageOperations.append(maskImage);
+
         CachedSVGDocumentReference* documentReference = maskImage->cachedSVGDocumentReference();
         CachedSVGDocument* cachedSVGDocument = documentReference ? documentReference->document() : nullptr;
         
@@ -134,13 +136,11 @@
     }
 }
 
-void RenderLayer::MaskImageInfo::removeMaskImageClients(const RenderStyle& oldStyle)
+void RenderLayer::MaskImageInfo::removeMaskImageClients()
 {
-    for (auto* maskLayer = oldStyle.maskLayers(); maskLayer; maskLayer = maskLayer->next()) {
-        if (auto& image = maskLayer->maskImage())
-            image->setRenderLayerImageClient(nullptr);
-    }
-    
+    for (auto& maskImage : m_maskImageOperations)
+        maskImage->setRenderLayerImageClient(nullptr);
+
     for (auto& externalSVGReference : m_externalSVGReferences)
         externalSVGReference->removeClient(m_svgDocumentClient.get());
     m_externalSVGReferences.clear();

Modified: trunk/Source/WebCore/rendering/RenderLayerMaskImageInfo.h (184748 => 184749)


--- trunk/Source/WebCore/rendering/RenderLayerMaskImageInfo.h	2015-05-22 02:54:44 UTC (rev 184748)
+++ trunk/Source/WebCore/rendering/RenderLayerMaskImageInfo.h	2015-05-22 03:59:16 UTC (rev 184749)
@@ -44,7 +44,7 @@
     ~MaskImageInfo();
 
     void updateMaskImageClients();
-    void removeMaskImageClients(const RenderStyle& oldStyle);
+    void removeMaskImageClients();
 
 private:
     static HashMap<const RenderLayer*, std::unique_ptr<MaskImageInfo>>& layerToMaskMap();
@@ -58,6 +58,7 @@
     Vector<CachedResourceHandle<CachedSVGDocument>> m_externalSVGReferences;
     std::unique_ptr<SVGDocumentClient> m_svgDocumentClient;
     std::unique_ptr<ImageClient> m_imageClient;
+    Vector<RefPtr<MaskImageOperation>> m_maskImageOperations;
 };
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to