REGRESSION(r244906): Crash in WebCore::positionOffsetValue
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jul 2019 15:21:11 +0000 (15:21 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 9 Jul 2019 15:21:11 +0000 (15:21 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199613
<rdar://problem/51518172>

Reviewed by Zalan Bujtas.

Source/WebCore:

Test: fast/css/getComputedStyle/sticky-scroll-container-crash.html

* css/CSSComputedStyleDeclaration.cpp:
(WebCore::positionOffsetValue):
* rendering/RenderBox.cpp:
(WebCore::RenderBox::enclosingScrollportBox const): Deleted.

The client trivally hits nullptr when this is called for element without overflow scroll parent.

Fix by removing the whole function and using shared enclosingClippingBoxForStickyPosition instead.
It does the same ancestor walk more efficiently via layer tree.

* rendering/RenderBox.h:
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::enclosingClippingBoxForStickyPosition const):

Factor into function.

(WebCore::RenderBoxModelObject::computeStickyPositionConstraints const):
* rendering/RenderBoxModelObject.h:

LayoutTests:

* fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt: Added.
* fast/css/getComputedStyle/sticky-scroll-container-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/CSSComputedStyleDeclaration.cpp
Source/WebCore/rendering/RenderBox.cpp
Source/WebCore/rendering/RenderBox.h
Source/WebCore/rendering/RenderBoxModelObject.cpp
Source/WebCore/rendering/RenderBoxModelObject.h

index 698ba25..fa6c566 100644 (file)
@@ -1,3 +1,14 @@
+2019-07-09  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION(r244906): Crash in WebCore::positionOffsetValue
+        https://bugs.webkit.org/show_bug.cgi?id=199613
+        <rdar://problem/51518172>
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt: Added.
+        * fast/css/getComputedStyle/sticky-scroll-container-crash.html: Added.
+
 2019-07-09  Cathie Chen  <cathiechen@igalia.com>
 
         Support writing-mode and direction for scrollIntoViewOptions.
diff --git a/LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt b/LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt
new file mode 100644 (file)
index 0000000..2985393
--- /dev/null
@@ -0,0 +1 @@
+This test passes if it doesn't crash
diff --git a/LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash.html b/LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash.html
new file mode 100644 (file)
index 0000000..c08802f
--- /dev/null
@@ -0,0 +1,13 @@
+<style>
+#target {
+    position: -webkit-sticky;
+    top: 5%;
+}
+</style>
+<div id=target>This test passes if it doesn't crash</div>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+getComputedStyle(target).top;
+</script>
index 5e3f4fb..e564bd2 100644 (file)
@@ -1,3 +1,32 @@
+2019-07-09  Antti Koivisto  <antti@apple.com>
+
+        REGRESSION(r244906): Crash in WebCore::positionOffsetValue
+        https://bugs.webkit.org/show_bug.cgi?id=199613
+        <rdar://problem/51518172>
+
+        Reviewed by Zalan Bujtas.
+
+        Test: fast/css/getComputedStyle/sticky-scroll-container-crash.html
+
+        * css/CSSComputedStyleDeclaration.cpp:
+        (WebCore::positionOffsetValue):
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::enclosingScrollportBox const): Deleted.
+
+        The client trivally hits nullptr when this is called for element without overflow scroll parent.
+
+        Fix by removing the whole function and using shared enclosingClippingBoxForStickyPosition instead.
+        It does the same ancestor walk more efficiently via layer tree.
+
+        * rendering/RenderBox.h:
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::enclosingClippingBoxForStickyPosition const):
+
+        Factor into function.
+
+        (WebCore::RenderBoxModelObject::computeStickyPositionConstraints const):
+        * rendering/RenderBoxModelObject.h:
+
 2019-07-09  Cathie Chen  <cathiechen@igalia.com>
 
         Support writing-mode and direction for scrollIntoViewOptions.
index cceb78e..4c4bef6 100644 (file)
@@ -790,12 +790,11 @@ static RefPtr<CSSValue> positionOffsetValue(const RenderStyle& style, CSSPropert
         }
         LayoutUnit containingBlockSize;
         if (box.isStickilyPositioned()) {
-            const RenderBox& enclosingScrollportBox =
-                box.enclosingScrollportBox();
-            if (isVerticalProperty == enclosingScrollportBox.isHorizontalWritingMode())
-                containingBlockSize = enclosingScrollportBox.contentLogicalHeight();
+            auto& enclosingClippingBox = box.enclosingClippingBoxForStickyPosition();
+            if (isVerticalProperty == enclosingClippingBox.isHorizontalWritingMode())
+                containingBlockSize = enclosingClippingBox.contentLogicalHeight();
             else
-                containingBlockSize = enclosingScrollportBox.contentLogicalWidth();
+                containingBlockSize = enclosingClippingBox.contentLogicalWidth();
         } else {
             if (isVerticalProperty == containingBlock->isHorizontalWritingMode()) {
                 containingBlockSize = box.isOutOfFlowPositioned()
index 21deb88..e20fc20 100644 (file)
@@ -4726,17 +4726,6 @@ RenderLayer* RenderBox::enclosingFloatPaintingLayer() const
     return nullptr;
 }
 
-const RenderBlock& RenderBox::enclosingScrollportBox() const
-{
-    const RenderBlock* ancestor = containingBlock();
-    for (; ancestor; ancestor = ancestor->containingBlock()) {
-        if (ancestor->hasOverflowClip())
-            return *ancestor;
-    }
-    ASSERT_NOT_REACHED();
-    return *ancestor;
-}
-
 LayoutRect RenderBox::logicalVisualOverflowRectForPropagation(const RenderStyle* parentStyle) const
 {
     LayoutRect rect = visualOverflowRectForPropagation(parentStyle);
index b2afd00..14a5522 100644 (file)
@@ -532,8 +532,6 @@ override;
     void removeFloatingOrPositionedChildFromBlockLists();
     
     RenderLayer* enclosingFloatPaintingLayer() const;
-
-    const RenderBlock& enclosingScrollportBox() const;
     
     virtual Optional<int> firstLineBaseline() const { return Optional<int>(); }
     virtual Optional<int> inlineBlockBaseline(LineDirectionMode) const { return Optional<int>(); } // Returns empty if we should skip this box when computing the baseline of an inline-block.
index 803d984..42430df 100644 (file)
@@ -441,13 +441,24 @@ LayoutPoint RenderBoxModelObject::adjustedPositionRelativeToOffsetParent(const L
     return referencePoint;
 }
 
+const RenderBox& RenderBoxModelObject::enclosingClippingBoxForStickyPosition(const RenderLayer** enclosingClippingLayer) const
+{
+    ASSERT(isStickilyPositioned());
+
+    auto* clipLayer = layer()->enclosingOverflowClipLayer(ExcludeSelf);
+    if (enclosingClippingLayer)
+        *enclosingClippingLayer = clipLayer;
+
+    return clipLayer ? downcast<RenderBox>(clipLayer->renderer()) : view();
+}
+
 void RenderBoxModelObject::computeStickyPositionConstraints(StickyPositionViewportConstraints& constraints, const FloatRect& constrainingRect) const
 {
     constraints.setConstrainingRectAtLastLayout(constrainingRect);
 
     RenderBlock* containingBlock = this->containingBlock();
-    RenderLayer* enclosingClippingLayer = layer()->enclosingOverflowClipLayer(ExcludeSelf);
-    RenderBox& enclosingClippingBox = enclosingClippingLayer ? downcast<RenderBox>(enclosingClippingLayer->renderer()) : view();
+    const RenderLayer* enclosingClippingLayer = nullptr;
+    auto& enclosingClippingBox = enclosingClippingBoxForStickyPosition(&enclosingClippingLayer);
 
     LayoutRect containerContentRect;
     if (!enclosingClippingLayer || (containingBlock != &enclosingClippingBox))
index 3c1b3a2..b59bf0a 100644 (file)
@@ -111,6 +111,7 @@ public:
     LayoutSize relativePositionLogicalOffset() const { return style().isHorizontalWritingMode() ? relativePositionOffset() : relativePositionOffset().transposedSize(); }
 
     FloatRect constrainingRectForStickyPosition() const;
+    const RenderBox& enclosingClippingBoxForStickyPosition(const RenderLayer** enclosingClippingLayer = nullptr) const;
     void computeStickyPositionConstraints(StickyPositionViewportConstraints&, const FloatRect& constrainingRect) const;
     LayoutSize stickyPositionOffset() const;
     LayoutSize stickyPositionLogicalOffset() const { return style().isHorizontalWritingMode() ? stickyPositionOffset() : stickyPositionOffset().transposedSize(); }