Attachment 373718 Details for Bug 199613 – patch

[patch] patch

null-scrollport-crash-2.patch (text/plain), 8.36 KB, created by Antti Koivisto on 2019-07-09 07:51:34 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Antti Koivisto

Created: 2019-07-09 07:51:34 PDT

Size: 8.36 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 247255)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -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: Source/WebCore/css/CSSComputedStyleDeclaration.cpp
>===================================================================
>--- Source/WebCore/css/CSSComputedStyleDeclaration.cpp	(revision 247254)
>+++ Source/WebCore/css/CSSComputedStyleDeclaration.cpp	(working copy)
>@@ -790,12 +790,11 @@ static RefPtr<CSSValue> positionOffsetVa
>         }
>         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: Source/WebCore/rendering/RenderBox.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderBox.cpp	(revision 247254)
>+++ Source/WebCore/rendering/RenderBox.cpp	(working copy)
>@@ -4726,17 +4726,6 @@ RenderLayer* RenderBox::enclosingFloatPa
>     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: Source/WebCore/rendering/RenderBox.h
>===================================================================
>--- Source/WebCore/rendering/RenderBox.h	(revision 247254)
>+++ Source/WebCore/rendering/RenderBox.h	(working copy)
>@@ -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: Source/WebCore/rendering/RenderBoxModelObject.cpp
>===================================================================
>--- Source/WebCore/rendering/RenderBoxModelObject.cpp	(revision 247254)
>+++ Source/WebCore/rendering/RenderBoxModelObject.cpp	(working copy)
>@@ -441,13 +441,24 @@ LayoutPoint RenderBoxModelObject::adjust
>     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: Source/WebCore/rendering/RenderBoxModelObject.h
>===================================================================
>--- Source/WebCore/rendering/RenderBoxModelObject.h	(revision 247254)
>+++ Source/WebCore/rendering/RenderBoxModelObject.h	(working copy)
>@@ -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(); }
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 247254)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -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-08  Saam Barati  <sbarati@apple.com>
> 
>         [WHLSL Import 23 new JS reference spec tests
>Index: LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt
>===================================================================
>--- LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt	(nonexistent)
>+++ LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash-expected.txt	(working copy)
>@@ -0,0 +1 @@
>+This test passes if it doesn't crash
>Index: LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash.html
>===================================================================
>--- LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash.html	(nonexistent)
>+++ LayoutTests/fast/css/getComputedStyle/sticky-scroll-container-crash.html	(working copy)
>@@ -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>

Actions: View | Formatted Diff | Diff

Attachments on bug 199613: 373708 | 373718