Title: [144639] trunk
Revision
144639
Author
[email protected]
Date
2013-03-04 09:29:20 -0800 (Mon, 04 Mar 2013)

Log Message

ASSERTION FAILED: m_clipRectsCache->m_respectingOverflowClip[clipRectsType] == (clipRectsContext.respectOverflowClip == RespectOverflowClip) in RenderLayer.
https://bugs.webkit.org/show_bug.cgi?id=108257

Reviewed by David Hyatt.

Source/WebCore:

With composited scrolling we paint both with and without respecting
overflow clip. To prevent collisions in the clip cache, and to prevent
throwing away cached clips unnecessarily, we keep two copies of the
clip cache -- one for when overflow clip is respected, and one for
when it isn't.

No new tests. Covered by existing tests (in debug):
  compositing/overflow/automatically-opt-into-composited-scrolling.html
  compositing/overflow/composited-scrolling-creates-a-stacking-container.html

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateClipRects):
  No longer asserts that our 'respect overflow clip' status is
  consistent. It also gets and sets the clip cache using the clip rect
  context rather than just the clip rect type.
(WebCore::RenderLayer::calculateClipRects):
(WebCore::RenderLayer::parentClipRects):
(WebCore::RenderLayer::clearClipRects):
(WebCore::ClipRectsCache::ClipRectsCache):
(WebCore::ClipRectsCache::getClipRects):
(WebCore::ClipRectsCache::setClipRects):
(WebCore::ClipRectsCache::getIndex):
  Get and set the cached clip rects using the context rather than
  type.
(WebCore::RenderLayer::clipRects):
  We now cache twice as many clip rects.
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateGraphicsLayerGeometry):
  ShouldRespectOverflowClip was moved out of RenderLayer.

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::updateClipRects):
(WebCore::RenderLayer::calculateClipRects):
(WebCore::RenderLayer::parentClipRects):
(WebCore::RenderLayer::clearClipRects):
* rendering/RenderLayer.h:
(WebCore::ClipRectsCache::ClipRectsCache):
(WebCore::ClipRectsCache::getClipRects):
(WebCore::ClipRectsCache::setClipRects):
(ClipRectsCache):
(WebCore::ClipRectsCache::getIndex):
(WebCore::RenderLayer::clipRects):
* rendering/RenderLayerBacking.cpp:
(WebCore::RenderLayerBacking::updateGraphicsLayerGeometry):

LayoutTests:

* platform/efl-wk2/TestExpectations:
* platform/mac/TestExpectations:
* platform/qt-5.0-wk2/TestExpectations:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (144638 => 144639)


--- trunk/LayoutTests/ChangeLog	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/LayoutTests/ChangeLog	2013-03-04 17:29:20 UTC (rev 144639)
@@ -1,3 +1,14 @@
+2013-03-04  Ian Vollick  <[email protected]>
+
+        ASSERTION FAILED: m_clipRectsCache->m_respectingOverflowClip[clipRectsType] == (clipRectsContext.respectOverflowClip == RespectOverflowClip) in RenderLayer.
+        https://bugs.webkit.org/show_bug.cgi?id=108257
+
+        Reviewed by David Hyatt.
+
+        * platform/efl-wk2/TestExpectations:
+        * platform/mac/TestExpectations:
+        * platform/qt-5.0-wk2/TestExpectations:
+
 2013-03-03  David Hyatt  <[email protected]>
 
         [New Multicolumn] Make sure region styling works for columns inside regions.

Modified: trunk/LayoutTests/platform/efl-wk2/TestExpectations (144638 => 144639)


--- trunk/LayoutTests/platform/efl-wk2/TestExpectations	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/LayoutTests/platform/efl-wk2/TestExpectations	2013-03-04 17:29:20 UTC (rev 144639)
@@ -224,9 +224,8 @@
 webkit.org/b/103883 css3/selectors3/xhtml/css3-modsel-1.xml [ Failure Pass ]
 
 # Assertion failure and incorrect results on our port after r140999 (webkit.org/b/106142)
-webkit.org/b/108257 compositing/overflow/automatically-opt-into-composited-scrolling.html [ Crash Failure ]
-webkit.org/b/108257 compositing/overflow/composited-scrolling-creates-a-stacking-container.html [ Crash Failure ]
-webkit.org/b/108257 compositing/overflow/clip-content-under-overflow-controls.html [ Crash Pass ]
+webkit.org/b/108257 compositing/overflow/automatically-opt-into-composited-scrolling.html [ Failure ]
+webkit.org/b/108257 compositing/overflow/composited-scrolling-creates-a-stacking-container.html [ Failure ]
 
 # Asserts when destroying WebCore::Database (possibly because r141207)
 webkit.org/b/108355 fast/js/caller-property.html [ Crash Pass ]

Modified: trunk/LayoutTests/platform/mac/TestExpectations (144638 => 144639)


--- trunk/LayoutTests/platform/mac/TestExpectations	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/LayoutTests/platform/mac/TestExpectations	2013-03-04 17:29:20 UTC (rev 144639)
@@ -1424,9 +1424,6 @@
 webkit.org/b/108370 editing/spelling/spelling-with-whitespace-selection.html [ Skip ]
 webkit.org/b/108370 editing/spelling/spelling-changed-text.html [ Skip ]
 
-webkit.org/b/108257 [ Debug ] compositing/overflow/composited-scrolling-creates-a-stacking-container.html [ Crash ]
-webkit.org/b/108257 [ Debug ] compositing/overflow/automatically-opt-into-composited-scrolling.html [ Crash ]
-
 # Rebaseline required after https://webkit.org/b/95772
 webkit.org/b/109209 fast/text/international/bidi-ignored-for-first-child-inline.html [ Failure ]
 

Modified: trunk/LayoutTests/platform/qt-5.0-wk2/TestExpectations (144638 => 144639)


--- trunk/LayoutTests/platform/qt-5.0-wk2/TestExpectations	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/LayoutTests/platform/qt-5.0-wk2/TestExpectations	2013-03-04 17:29:20 UTC (rev 144639)
@@ -730,9 +730,6 @@
 # [Qt][WK2] new fast/dom/Geolocation/cached-position-iframe.html fails since it was introduced in r136164
 webkit.org/b/103876 fast/dom/Geolocation/cached-position-iframe.html
 
-# [Qt][WK2] compositing/overflow/automatically-opt-into-composited-scrolling.html makes other tests fail
-webkit.org/b/105173 compositing/overflow/automatically-opt-into-composited-scrolling.html
-
 # [Qt][WK2] The new compositing/repaint/resize-repaint.html fails
 webkit.org/b/105182 compositing/repaint/resize-repaint.html
 

Modified: trunk/Source/WebCore/ChangeLog (144638 => 144639)


--- trunk/Source/WebCore/ChangeLog	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/Source/WebCore/ChangeLog	2013-03-04 17:29:20 UTC (rev 144639)
@@ -1,3 +1,55 @@
+2013-03-04  Ian Vollick  <[email protected]>
+
+        ASSERTION FAILED: m_clipRectsCache->m_respectingOverflowClip[clipRectsType] == (clipRectsContext.respectOverflowClip == RespectOverflowClip) in RenderLayer.
+        https://bugs.webkit.org/show_bug.cgi?id=108257
+
+        Reviewed by David Hyatt.
+
+        With composited scrolling we paint both with and without respecting
+        overflow clip. To prevent collisions in the clip cache, and to prevent
+        throwing away cached clips unnecessarily, we keep two copies of the
+        clip cache -- one for when overflow clip is respected, and one for
+        when it isn't.
+
+        No new tests. Covered by existing tests (in debug):
+          compositing/overflow/automatically-opt-into-composited-scrolling.html
+          compositing/overflow/composited-scrolling-creates-a-stacking-container.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateClipRects):
+          No longer asserts that our 'respect overflow clip' status is
+          consistent. It also gets and sets the clip cache using the clip rect
+          context rather than just the clip rect type.
+        (WebCore::RenderLayer::calculateClipRects):
+        (WebCore::RenderLayer::parentClipRects):
+        (WebCore::RenderLayer::clearClipRects):
+        (WebCore::ClipRectsCache::ClipRectsCache):
+        (WebCore::ClipRectsCache::getClipRects):
+        (WebCore::ClipRectsCache::setClipRects):
+        (WebCore::ClipRectsCache::getIndex):
+          Get and set the cached clip rects using the context rather than
+          type.
+        (WebCore::RenderLayer::clipRects):
+          We now cache twice as many clip rects.
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry):
+          ShouldRespectOverflowClip was moved out of RenderLayer.
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::updateClipRects):
+        (WebCore::RenderLayer::calculateClipRects):
+        (WebCore::RenderLayer::parentClipRects):
+        (WebCore::RenderLayer::clearClipRects):
+        * rendering/RenderLayer.h:
+        (WebCore::ClipRectsCache::ClipRectsCache):
+        (WebCore::ClipRectsCache::getClipRects):
+        (WebCore::ClipRectsCache::setClipRects):
+        (ClipRectsCache):
+        (WebCore::ClipRectsCache::getIndex):
+        (WebCore::RenderLayer::clipRects):
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::updateGraphicsLayerGeometry):
+
 2013-03-04  Aaron Colwell  <[email protected]>
 
         Remove unused return value from SourceBufferPrivate::abort() and WebSourceBuffer::abort().

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (144638 => 144639)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2013-03-04 17:29:20 UTC (rev 144639)
@@ -4896,9 +4896,8 @@
 {
     ClipRectsType clipRectsType = clipRectsContext.clipRectsType;
     ASSERT(clipRectsType < NumCachedClipRectsTypes);
-    if (m_clipRectsCache && m_clipRectsCache->m_clipRects[clipRectsType]) {
+    if (m_clipRectsCache && m_clipRectsCache->getClipRects(clipRectsType, clipRectsContext.respectOverflowClip)) {
         ASSERT(clipRectsContext.rootLayer == m_clipRectsCache->m_clipRectsRoot[clipRectsType]);
-        ASSERT(m_clipRectsCache->m_respectingOverflowClip[clipRectsType] == (clipRectsContext.respectOverflowClip == RespectOverflowClip));
         ASSERT(m_clipRectsCache->m_scrollbarRelevancy[clipRectsType] == clipRectsContext.overlayScrollbarSizeRelevancy);
         
 #ifdef CHECK_CACHED_CLIP_RECTS
@@ -4907,7 +4906,7 @@
         tempContext.clipRectsType = TemporaryClipRects;
         ClipRects clipRects;
         calculateClipRects(tempContext, clipRects);
-        ASSERT(clipRects == *m_clipRectsCache->m_clipRects[clipRectsType].get());
+        ASSERT(clipRects == *m_clipRectsCache->getClipRects(clipRectsType, clipRectsContext.respectOverflowClip).get());
 #endif
         return; // We have the correct cached value.
     }
@@ -4924,14 +4923,13 @@
     if (!m_clipRectsCache)
         m_clipRectsCache = adoptPtr(new ClipRectsCache);
 
-    if (parentLayer && parentLayer->clipRects(clipRectsType) && clipRects == *parentLayer->clipRects(clipRectsType))
-        m_clipRectsCache->m_clipRects[clipRectsType] = parentLayer->clipRects(clipRectsType);
+    if (parentLayer && parentLayer->clipRects(clipRectsContext) && clipRects == *parentLayer->clipRects(clipRectsContext))
+        m_clipRectsCache->setClipRects(clipRectsType, clipRectsContext.respectOverflowClip, parentLayer->clipRects(clipRectsContext));
     else
-        m_clipRectsCache->m_clipRects[clipRectsType] = ClipRects::create(clipRects);
+        m_clipRectsCache->setClipRects(clipRectsType, clipRectsContext.respectOverflowClip, ClipRects::create(clipRects));
 
 #ifndef NDEBUG
     m_clipRectsCache->m_clipRectsRoot[clipRectsType] = clipRectsContext.rootLayer;
-    m_clipRectsCache->m_respectingOverflowClip[clipRectsType] = clipRectsContext.respectOverflowClip == RespectOverflowClip;
     m_clipRectsCache->m_scrollbarRelevancy[clipRectsType] = clipRectsContext.overlayScrollbarSizeRelevancy;
 #endif
 }
@@ -4953,8 +4951,8 @@
     
     // Ensure that our parent's clip has been calculated so that we can examine the values.
     if (parentLayer) {
-        if (useCached && parentLayer->clipRects(clipRectsType))
-            clipRects = *parentLayer->clipRects(clipRectsType);
+        if (useCached && parentLayer->clipRects(clipRectsContext))
+            clipRects = *parentLayer->clipRects(clipRectsContext);
         else {
             ClipRectsContext parentContext(clipRectsContext);
             parentContext.overlayScrollbarSizeRelevancy = IgnoreOverlayScrollbarSize; // FIXME: why?
@@ -5015,7 +5013,7 @@
     }
 
     parent()->updateClipRects(clipRectsContext);
-    clipRects = *parent()->clipRects(clipRectsContext.clipRectsType);
+    clipRects = *parent()->clipRects(clipRectsContext);
 }
 
 static inline ClipRect backgroundClipRectForPosition(const ClipRects& parentRects, EPosition position)
@@ -5448,7 +5446,9 @@
         m_clipRectsCache = nullptr;
     else {
         ASSERT(typeToClear < NumCachedClipRectsTypes);
-        m_clipRectsCache->m_clipRects[typeToClear] = nullptr;
+        RefPtr<ClipRects> dummy;
+        m_clipRectsCache->setClipRects(typeToClear, RespectOverflowClip, dummy);
+        m_clipRectsCache->setClipRects(typeToClear, IgnoreOverflowClip, dummy);
     }
 }
 

Modified: trunk/Source/WebCore/rendering/RenderLayer.h (144638 => 144639)


--- trunk/Source/WebCore/rendering/RenderLayer.h	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/Source/WebCore/rendering/RenderLayer.h	2013-03-04 17:29:20 UTC (rev 144639)
@@ -231,6 +231,11 @@
     TemporaryClipRects
 };
 
+enum ShouldRespectOverflowClip {
+    IgnoreOverflowClip,
+    RespectOverflowClip
+};
+
 struct ClipRectsCache {
     WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -239,18 +244,29 @@
 #ifndef NDEBUG
         for (int i = 0; i < NumCachedClipRectsTypes; ++i) {
             m_clipRectsRoot[i] = 0;
-            m_respectingOverflowClip[i] = false;
             m_scrollbarRelevancy[i] = IgnoreOverlayScrollbarSize;
         }
 #endif
     }
 
-    RefPtr<ClipRects> m_clipRects[NumCachedClipRectsTypes];
+    PassRefPtr<ClipRects> getClipRects(ClipRectsType clipRectsType, ShouldRespectOverflowClip respectOverflow) { return m_clipRects[getIndex(clipRectsType, respectOverflow)]; }
+    void setClipRects(ClipRectsType clipRectsType, ShouldRespectOverflowClip respectOverflow, PassRefPtr<ClipRects> clipRects) { m_clipRects[getIndex(clipRectsType, respectOverflow)] = clipRects; }
+
 #ifndef NDEBUG
     const RenderLayer* m_clipRectsRoot[NumCachedClipRectsTypes];
-    bool m_respectingOverflowClip[NumCachedClipRectsTypes];
     OverlayScrollbarSizeRelevancy m_scrollbarRelevancy[NumCachedClipRectsTypes];
 #endif
+
+private:
+    int getIndex(ClipRectsType clipRectsType, ShouldRespectOverflowClip respectOverflow)
+    {
+        int index = static_cast<int>(clipRectsType);
+        if (respectOverflow == RespectOverflowClip)
+            index += static_cast<int>(NumCachedClipRectsTypes);
+        return index;
+    }
+
+    RefPtr<ClipRects> m_clipRects[NumCachedClipRectsTypes * 2];
 };
 
 struct LayerFragment {
@@ -621,8 +637,6 @@
     bool hitTest(const HitTestRequest&, const HitTestLocation&, HitTestResult&);
     void paintOverlayScrollbars(GraphicsContext*, const LayoutRect& damageRect, PaintBehavior, RenderObject* paintingRoot = 0);
 
-    enum ShouldRespectOverflowClip { IgnoreOverflowClip, RespectOverflowClip };
-
     struct ClipRectsContext {
         ClipRectsContext(const RenderLayer* inRootLayer, RenderRegion* inRegion, ClipRectsType inClipRectsType, OverlayScrollbarSizeRelevancy inOverlayScrollbarSizeRelevancy = IgnoreOverlayScrollbarSize, ShouldRespectOverflowClip inRespectOverflowClip = RespectOverflowClip)
             : rootLayer(inRootLayer)
@@ -651,10 +665,10 @@
     // (rather than computing them all from scratch up the parent chain).
     void calculateClipRects(const ClipRectsContext&, ClipRects&) const;
 
-    ClipRects* clipRects(ClipRectsType type) const
+    ClipRects* clipRects(const ClipRectsContext& context) const
     {
-        ASSERT(type < NumCachedClipRectsTypes);
-        return m_clipRectsCache ? m_clipRectsCache->m_clipRects[type].get() : 0;
+        ASSERT(context.clipRectsType < NumCachedClipRectsTypes);
+        return m_clipRectsCache ? m_clipRectsCache->getClipRects(context.clipRectsType, context.respectOverflowClip).get() : 0;
     }
 
     LayoutRect childrenClipRect() const; // Returns the foreground clip rect of the layer in the document's coordinate space.

Modified: trunk/Source/WebCore/rendering/RenderLayerBacking.cpp (144638 => 144639)


--- trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2013-03-04 17:24:42 UTC (rev 144638)
+++ trunk/Source/WebCore/rendering/RenderLayerBacking.cpp	2013-03-04 17:29:20 UTC (rev 144639)
@@ -641,7 +641,7 @@
         // Call calculateRects to get the backgroundRect which is what is used to clip the contents of this
         // layer. Note that we call it with temporaryClipRects = true because normally when computing clip rects
         // for a compositing layer, rootLayer is the layer itself.
-        RenderLayer::ClipRectsContext clipRectsContext(compAncestor, 0, TemporaryClipRects, IgnoreOverlayScrollbarSize, RenderLayer::IgnoreOverflowClip);
+        RenderLayer::ClipRectsContext clipRectsContext(compAncestor, 0, TemporaryClipRects, IgnoreOverlayScrollbarSize, IgnoreOverflowClip);
         IntRect parentClipRect = pixelSnappedIntRect(m_owningLayer->backgroundClipRect(clipRectsContext).rect()); // FIXME: Incorrect for CSS regions.
         ASSERT(parentClipRect != PaintInfo::infiniteRect());
         m_ancestorClippingLayer->setPosition(FloatPoint(parentClipRect.location() - graphicsLayerParentLocation));
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to