[TextureMapper] Scrolling through 01.org/dleyna crashes WebKitWebProcess
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Sep 2016 09:59:52 +0000 (09:59 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Sep 2016 09:59:52 +0000 (09:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=162020

Reviewed by Žan Doberšek.

The problem is that we are trying to clone a ReferenceFilterOperation, which is not expected to be cloned, from
FilterAnimationValue copy constructor, and FilterOperations are never expected to be nullptr, so we end up
crashing. We just need to validate the filters before setting then and before creating a TextureMapperAnimation
for them.

* platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:
(WebCore::GraphicsLayerTextureMapper::filtersCanBeComposited): Return false if there are reference filters or no
filters at all. I don't know if we really support other filters, but at least we won't crash for the others.
(WebCore::GraphicsLayerTextureMapper::addAnimation): Check if filters can be composited before creating a
TextureMapperAnimation.
(WebCore::GraphicsLayerTextureMapper::setFilters): Check if filters can be composited before setting them.
* platform/graphics/texmap/GraphicsLayerTextureMapper.h:
* platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
(WebCore::CoordinatedGraphicsLayer::filtersCanBeComposited): Return false if there are reference filters or no
filters at all. I don't know if we really support other filters, but at least we won't crash for the others.
(WebCore::CoordinatedGraphicsLayer::setFilters): Check if filters can be composited before setting them.
(WebCore::CoordinatedGraphicsLayer::addAnimation): Check if filters can be composited before creating a
TextureMapperAnimation.
* platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.cpp
Source/WebCore/platform/graphics/texmap/GraphicsLayerTextureMapper.h
Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp
Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h

index b3ee9cb..721ffe4 100644 (file)
@@ -1,3 +1,30 @@
+2016-09-16  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [TextureMapper] Scrolling through 01.org/dleyna crashes WebKitWebProcess
+        https://bugs.webkit.org/show_bug.cgi?id=162020
+
+        Reviewed by Žan Doberšek.
+
+        The problem is that we are trying to clone a ReferenceFilterOperation, which is not expected to be cloned, from
+        FilterAnimationValue copy constructor, and FilterOperations are never expected to be nullptr, so we end up
+        crashing. We just need to validate the filters before setting then and before creating a TextureMapperAnimation
+        for them.
+
+        * platform/graphics/texmap/GraphicsLayerTextureMapper.cpp:
+        (WebCore::GraphicsLayerTextureMapper::filtersCanBeComposited): Return false if there are reference filters or no
+        filters at all. I don't know if we really support other filters, but at least we won't crash for the others.
+        (WebCore::GraphicsLayerTextureMapper::addAnimation): Check if filters can be composited before creating a
+        TextureMapperAnimation.
+        (WebCore::GraphicsLayerTextureMapper::setFilters): Check if filters can be composited before setting them.
+        * platform/graphics/texmap/GraphicsLayerTextureMapper.h:
+        * platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:
+        (WebCore::CoordinatedGraphicsLayer::filtersCanBeComposited): Return false if there are reference filters or no
+        filters at all. I don't know if we really support other filters, but at least we won't crash for the others.
+        (WebCore::CoordinatedGraphicsLayer::setFilters): Check if filters can be composited before setting them.
+        (WebCore::CoordinatedGraphicsLayer::addAnimation): Check if filters can be composited before creating a
+        TextureMapperAnimation.
+        * platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:
+
 2016-09-16  Youenn Fablet  <youenn@apple.com>
 
         CachedFont do not need to be updated according Origin/Fetch mode
index 738a93f..c7222b3 100644 (file)
@@ -560,6 +560,19 @@ bool GraphicsLayerTextureMapper::shouldHaveBackingStore() const
     return drawsContent() && contentsAreVisible() && !m_size.isEmpty();
 }
 
+bool GraphicsLayerTextureMapper::filtersCanBeComposited(const FilterOperations& filters) const
+{
+    if (!filters.size())
+        return false;
+
+    for (const auto& filterOperation : filters.operations()) {
+        if (filterOperation->type() == FilterOperation::REFERENCE)
+            return false;
+    }
+
+    return true;
+}
+
 bool GraphicsLayerTextureMapper::addAnimation(const KeyframeValueList& valueList, const FloatSize& boxSize, const Animation* anim, const String& keyframesName, double timeOffset)
 {
     ASSERT(!keyframesName.isEmpty());
@@ -567,6 +580,16 @@ bool GraphicsLayerTextureMapper::addAnimation(const KeyframeValueList& valueList
     if (!anim || anim->isEmptyOrZeroDuration() || valueList.size() < 2 || (valueList.property() != AnimatedPropertyTransform && valueList.property() != AnimatedPropertyOpacity))
         return false;
 
+    if (valueList.property() == AnimatedPropertyFilter) {
+        int listIndex = validateFilterOperations(valueList);
+        if (listIndex < 0)
+            return false;
+
+        const auto& filters = static_cast<const FilterAnimationValue&>(valueList.at(listIndex)).value();
+        if (!filtersCanBeComposited(filters))
+            return false;
+    }
+
     bool listsMatch = false;
     bool hasBigRotation;
 
@@ -604,11 +627,23 @@ void GraphicsLayerTextureMapper::removeAnimation(const String& animationName)
 
 bool GraphicsLayerTextureMapper::setFilters(const FilterOperations& filters)
 {
-    TextureMapper* textureMapper = m_layer.textureMapper();
-    if (!textureMapper)
+    if (!m_layer.textureMapper())
         return false;
-    notifyChange(FilterChange);
-    return GraphicsLayer::setFilters(filters);
+
+    bool canCompositeFilters = filtersCanBeComposited(filters);
+    if (GraphicsLayer::filters() == filters)
+        return canCompositeFilters;
+
+    if (canCompositeFilters) {
+        if (!GraphicsLayer::setFilters(filters))
+            return false;
+        notifyChange(FilterChange);
+    } else if (GraphicsLayer::filters().size()) {
+        clearFilters();
+        notifyChange(FilterChange);
+    }
+
+    return canCompositeFilters;
 }
 
 void GraphicsLayerTextureMapper::setFixedToViewport(bool fixed)
index 4ccd38e..6c1ecf9 100644 (file)
@@ -117,6 +117,8 @@ private:
     void prepareBackingStoreIfNeeded();
     bool shouldHaveBackingStore() const;
 
+    bool filtersCanBeComposited(const FilterOperations&) const;
+
     // This set of flags help us defer which properties of the layer have been
     // modified by the compositor, so we can know what to look for in the next flush.
     enum ChangeMask {
index 73510ac..748ad8e 100644 (file)
@@ -426,18 +426,37 @@ void CoordinatedGraphicsLayer::setContentsToPlatformLayer(PlatformLayer* platfor
 #endif
 }
 
-bool CoordinatedGraphicsLayer::setFilters(const FilterOperations& newFilters)
+bool CoordinatedGraphicsLayer::filtersCanBeComposited(const FilterOperations& filters) const
 {
-    if (filters() == newFilters)
-        return true;
-
-    if (!GraphicsLayer::setFilters(newFilters))
+    if (!filters.size())
         return false;
 
-    didChangeFilters();
+    for (const auto& filterOperation : filters.operations()) {
+        if (filterOperation->type() == FilterOperation::REFERENCE)
+            return false;
+    }
+
     return true;
 }
 
+bool CoordinatedGraphicsLayer::setFilters(const FilterOperations& newFilters)
+{
+    bool canCompositeFilters = filtersCanBeComposited(newFilters);
+    if (filters() == newFilters)
+        return canCompositeFilters;
+
+    if (canCompositeFilters) {
+        if (!GraphicsLayer::setFilters(newFilters))
+            return false;
+        didChangeFilters();
+    } else if (filters().size()) {
+        clearFilters();
+        didChangeFilters();
+    }
+
+    return canCompositeFilters;
+}
+
 void CoordinatedGraphicsLayer::setContentsToSolidColor(const Color& color)
 {
     if (m_layerState.solidColor == color)
@@ -1163,6 +1182,16 @@ bool CoordinatedGraphicsLayer::addAnimation(const KeyframeValueList& valueList,
     if (!anim || anim->isEmptyOrZeroDuration() || valueList.size() < 2 || (valueList.property() != AnimatedPropertyTransform && valueList.property() != AnimatedPropertyOpacity && valueList.property() != AnimatedPropertyFilter))
         return false;
 
+    if (valueList.property() == AnimatedPropertyFilter) {
+        int listIndex = validateFilterOperations(valueList);
+        if (listIndex < 0)
+            return false;
+
+        const auto& filters = static_cast<const FilterAnimationValue&>(valueList.at(listIndex)).value();
+        if (!filtersCanBeComposited(filters))
+            return false;
+    }
+
     bool listsMatch = false;
     bool ignoredHasBigRotation;
 
index aeac198..3799939 100644 (file)
@@ -207,6 +207,8 @@ private:
 
     void animationStartedTimerFired();
 
+    bool filtersCanBeComposited(const FilterOperations&) const;
+
     CoordinatedLayerID m_id;
     CoordinatedGraphicsLayerState m_layerState;
     GraphicsLayerTransform m_layerTransform;