Attachment 359641 Details for Bug 193621 – Patch

[patch] Patch

bug-193621-20190120120038.patch (text/plain), 12.76 KB, created by Simon Fraser (smfr) on 2019-01-20 12:00:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-01-20 12:00:39 PST

Size: 12.76 KB

patch

obsolete

>Subversion Revision: 240194
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c041dc2d1645f44974f0bd49b44573c0b63763d6..8314a70d53969bd6341f3c187f1cf6518ba96d2f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,37 @@
>+2019-01-20  Simon Fraser  <simon.fraser@apple.com>
>+
>+        On RenderBox, make client sizing be derived from padding box sizing
>+        https://bugs.webkit.org/show_bug.cgi?id=193621
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        I never liked how clientWidth/Height, an IE-originated term, was used as the basis
>+        for various RenderBox geometry functions.
>+
>+        Fix by adding some functions which return the dimensions of the padding box (which
>+        is the inside of the border and any scrollbar), and define clientWidth/Height in
>+        terms of them.
>+
>+        Also add paddingBoxRectIncludingScrollbar() function that is used by compositing code.
>+
>+        * rendering/RenderBox.cpp:
>+        (WebCore::RenderBox::clientWidth const):
>+        (WebCore::RenderBox::clientHeight const):
>+        * rendering/RenderBox.h:
>+        (WebCore::RenderBox::borderBoxRect const):
>+        (WebCore::RenderBox::computedCSSContentBoxRect const):
>+        (WebCore::RenderBox::contentWidth const):
>+        (WebCore::RenderBox::contentHeight const):
>+        (WebCore::RenderBox::paddingBoxWidth const):
>+        (WebCore::RenderBox::paddingBoxHeight const):
>+        (WebCore::RenderBox::paddingBoxRect const):
>+        (WebCore::RenderBox::paddingBoxRectIncludingScrollbar const):
>+        (WebCore::RenderBox::hasHorizontalOverflow const):
>+        (WebCore::RenderBox::hasVerticalOverflow const):
>+        * rendering/RenderLayerBacking.cpp:
>+        (WebCore::RenderLayerBacking::computeParentGraphicsLayerRect const):
>+        (WebCore::RenderLayerBacking::updateGeometry):
>+
> 2019-01-18  Eric Carlson  <eric.carlson@apple.com>
> 
>         Revert r238815, it broke WK1 video fullscreen on Mac
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index 06aebaf65d5df19b7b02df2aef6124e6a64f442c..d38720c9159f0be34ee6d2eac6aafe8a25ecbf53 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -526,12 +526,12 @@ void RenderBox::layout()
> // excluding border and scrollbar.
> LayoutUnit RenderBox::clientWidth() const
> {
>-    return width() - borderLeft() - borderRight() - verticalScrollbarWidth();
>+    return paddingBoxWidth();
> }
> 
> LayoutUnit RenderBox::clientHeight() const
> {
>-    return height() - borderTop() - borderBottom() - horizontalScrollbarHeight();
>+    return paddingBoxHeight();
> }
> 
> int RenderBox::scrollWidth() const
>diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
>index 5510685294294abf8f5d6eb2a0cac2106f6a6c12..9fcd239911af3a3008e01526320030d9781a67ce 100644
>--- a/Source/WebCore/rendering/RenderBox.h
>+++ b/Source/WebCore/rendering/RenderBox.h
>@@ -151,7 +151,6 @@ public:
>         return LayoutRect(-marginLeft, -marginTop, size().width() + marginLeft + marginRight, size().height() + marginTop + marginBottom);
>     }
>     LayoutRect borderBoxRect() const { return LayoutRect(LayoutPoint(), size()); }
>-    LayoutRect paddingBoxRect() const { return LayoutRect(borderLeft(), borderTop(), contentWidth() + paddingLeft() + paddingRight(), contentHeight() + paddingTop() + paddingBottom()); }
>     LayoutRect borderBoundingBox() const final { return borderBoxRect(); }
> 
>     WEBCORE_EXPORT RoundedRect::Radii borderRadii() const;
>@@ -167,7 +166,7 @@ public:
> 
>     // This returns the content area of the box (excluding padding and border). The only difference with contentBoxRect is that computedCSSContentBoxRect
>     // does include the intrinsic padding in the content box as this is what some callers expect (like getComputedStyle).
>-    LayoutRect computedCSSContentBoxRect() const { return LayoutRect(borderLeft() + computedCSSPaddingLeft(), borderTop() + computedCSSPaddingTop(), clientWidth() - computedCSSPaddingLeft() - computedCSSPaddingRight(), clientHeight() - computedCSSPaddingTop() - computedCSSPaddingBottom()); }
>+    LayoutRect computedCSSContentBoxRect() const { return LayoutRect(borderLeft() + computedCSSPaddingLeft(), borderTop() + computedCSSPaddingTop(), paddingBoxWidth() - computedCSSPaddingLeft() - computedCSSPaddingRight(), paddingBoxHeight() - computedCSSPaddingTop() - computedCSSPaddingBottom()); }
> 
>     // Bounds of the outline box in absolute coords. Respects transforms
>     LayoutRect outlineBoundsForRepaint(const RenderLayerModelObject* /*repaintContainer*/, const RenderGeometryMap*) const final;
>@@ -213,11 +212,16 @@ public:
>     void updateLayerTransform();
> 
>     LayoutSize contentSize() const { return { contentWidth(), contentHeight() }; }
>-    LayoutUnit contentWidth() const { return clientWidth() - paddingLeft() - paddingRight(); }
>-    LayoutUnit contentHeight() const { return clientHeight() - paddingTop() - paddingBottom(); }
>+    LayoutUnit contentWidth() const { return paddingBoxWidth() - paddingLeft() - paddingRight(); }
>+    LayoutUnit contentHeight() const { return paddingBoxHeight() - paddingTop() - paddingBottom(); }
>     LayoutUnit contentLogicalWidth() const { return style().isHorizontalWritingMode() ? contentWidth() : contentHeight(); }
>     LayoutUnit contentLogicalHeight() const { return style().isHorizontalWritingMode() ? contentHeight() : contentWidth(); }
> 
>+    LayoutUnit paddingBoxWidth() const { return width() - borderLeft() - borderRight() - verticalScrollbarWidth(); }
>+    LayoutUnit paddingBoxHeight() const { return height() - borderTop() - borderBottom() - horizontalScrollbarHeight(); }
>+    LayoutRect paddingBoxRect() const { return LayoutRect(borderLeft(), borderTop(), paddingBoxWidth(), paddingBoxHeight()); }
>+    LayoutRect paddingBoxRectIncludingScrollbar() const { return LayoutRect(borderLeft(), borderTop(), width() - borderLeft() - borderRight(), height() - borderTop() - borderBottom()); }
>+
>     // IE extensions. Used to calculate offsetWidth/Height.  Overridden by inlines (RenderFlow)
>     // to return the remaining width on a given line (and the height of a single line).
>     LayoutUnit offsetWidth() const override { return width(); }
>@@ -447,8 +451,8 @@ override;
>     LayoutUnit availableWidth() const { return style().isHorizontalWritingMode() ? availableLogicalWidth() : availableLogicalHeight(IncludeMarginBorderPadding); }
>     LayoutUnit availableHeight() const { return style().isHorizontalWritingMode() ? availableLogicalHeight(IncludeMarginBorderPadding) : availableLogicalWidth(); }
> 
>-    virtual int verticalScrollbarWidth() const;
>-    int horizontalScrollbarHeight() const;
>+    WEBCORE_EXPORT virtual int verticalScrollbarWidth() const;
>+    WEBCORE_EXPORT int horizontalScrollbarHeight() const;
>     int intrinsicScrollbarLogicalWidth() const;
>     int scrollbarLogicalWidth() const { return style().isHorizontalWritingMode() ? verticalScrollbarWidth() : horizontalScrollbarHeight(); }
>     int scrollbarLogicalHeight() const { return style().isHorizontalWritingMode() ? horizontalScrollbarHeight() : verticalScrollbarWidth(); }
>@@ -472,8 +476,8 @@ override;
>     bool scrollsOverflowX() const { return hasOverflowClip() && (style().overflowX() == Overflow::Scroll || style().overflowX() == Overflow::Auto); }
>     bool scrollsOverflowY() const { return hasOverflowClip() && (style().overflowY() == Overflow::Scroll || style().overflowY() == Overflow::Auto); }
> 
>-    bool hasHorizontalOverflow() const { return scrollWidth() != roundToInt(clientWidth()); }
>-    bool hasVerticalOverflow() const { return scrollHeight() != roundToInt(clientHeight()); }
>+    bool hasHorizontalOverflow() const { return scrollWidth() != roundToInt(paddingBoxWidth()); }
>+    bool hasVerticalOverflow() const { return scrollHeight() != roundToInt(paddingBoxHeight()); }
> 
>     bool hasScrollableOverflowX() const { return scrollsOverflowX() && hasHorizontalOverflow(); }
>     bool hasScrollableOverflowY() const { return scrollsOverflowY() && hasVerticalOverflow(); }
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index 99afd882b7f282d3df62b618ccefb839fff73be9..31c326f9f1826f9ed54ae119fcd94877fcf02a8a 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -950,9 +950,9 @@ LayoutRect RenderLayerBacking::computeParentGraphicsLayerRect(RenderLayer* compo
>     if (compositedAncestor->hasCompositedScrollableOverflow()) {
>         LayoutRect ancestorCompositedBounds = ancestorBackingLayer->compositedBounds();
>         auto& renderBox = downcast<RenderBox>(compositedAncestor->renderer());
>-        LayoutRect paddingBox(renderBox.borderLeft(), renderBox.borderTop(), renderBox.width() - renderBox.borderLeft() - renderBox.borderRight(), renderBox.height() - renderBox.borderTop() - renderBox.borderBottom());
>+        LayoutRect paddingBoxIncludingScrollbar = renderBox.paddingBoxRectIncludingScrollbar();
>         ScrollOffset scrollOffset = compositedAncestor->scrollOffset();
>-        parentGraphicsLayerRect = LayoutRect((paddingBox.location() - toLayoutSize(ancestorCompositedBounds.location()) - toLayoutSize(scrollOffset)), paddingBox.size());
>+        parentGraphicsLayerRect = LayoutRect((paddingBoxIncludingScrollbar.location() - toLayoutSize(ancestorCompositedBounds.location()) - toLayoutSize(scrollOffset)), paddingBoxIncludingScrollbar.size());
>     }
> #endif
> 
>@@ -1175,16 +1175,16 @@ void RenderLayerBacking::updateGeometry()
>     if (m_scrollingLayer) {
>         ASSERT(m_scrollingContentsLayer);
>         auto& renderBox = downcast<RenderBox>(renderer());
>-        LayoutRect paddingBox(renderBox.borderLeft(), renderBox.borderTop(), renderBox.width() - renderBox.borderLeft() - renderBox.borderRight(), renderBox.height() - renderBox.borderTop() - renderBox.borderBottom());
>+        LayoutRect paddingBoxIncludingScrollbar = renderBox.paddingBoxRectIncludingScrollbar();
>         ScrollOffset scrollOffset = m_owningLayer.scrollOffset();
> 
>         // FIXME: need to do some pixel snapping here.
>-        m_scrollingLayer->setPosition(FloatPoint(paddingBox.location() - compositedBounds().location()));
>+        m_scrollingLayer->setPosition(FloatPoint(paddingBoxIncludingScrollbar.location() - compositedBounds().location()));
> 
>         m_scrollingLayer->setSize(roundedIntSize(LayoutSize(renderBox.clientWidth(), renderBox.clientHeight())));
> #if PLATFORM(IOS_FAMILY)
>         FloatSize oldScrollingLayerOffset = m_scrollingLayer->offsetFromRenderer();
>-        m_scrollingLayer->setOffsetFromRenderer(FloatPoint() - paddingBox.location());
>+        m_scrollingLayer->setOffsetFromRenderer(FloatPoint() - paddingBoxIncludingScrollbar.location());
>         bool paddingBoxOffsetChanged = oldScrollingLayerOffset != m_scrollingLayer->offsetFromRenderer();
> 
>         if (m_owningLayer.isInUserScroll()) {
>@@ -1207,17 +1207,17 @@ void RenderLayerBacking::updateGeometry()
>         m_scrollingContentsLayer->setSize(scrollSize);
>         // Scrolling the content layer does not need to trigger a repaint. The offset will be compensated away during painting.
>         m_scrollingContentsLayer->setScrollOffset(scrollOffset, GraphicsLayer::DontSetNeedsDisplay);
>-        m_scrollingContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBox.location()), GraphicsLayer::DontSetNeedsDisplay);
>+        m_scrollingContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBoxIncludingScrollbar.location()), GraphicsLayer::DontSetNeedsDisplay);
> #else
>         m_scrollingContentsLayer->setPosition(-scrollOffset);
> 
>         FloatSize oldScrollingLayerOffset = m_scrollingLayer->offsetFromRenderer();
>-        m_scrollingLayer->setOffsetFromRenderer(-toFloatSize(paddingBox.location()));
>+        m_scrollingLayer->setOffsetFromRenderer(-toFloatSize(paddingBoxIncludingScrollbar.location()));
> 
>         if (m_childClippingMaskLayer) {
>             m_childClippingMaskLayer->setPosition(m_scrollingLayer->position());
>             m_childClippingMaskLayer->setSize(m_scrollingLayer->size());
>-            m_childClippingMaskLayer->setOffsetFromRenderer(toFloatSize(paddingBox.location()));
>+            m_childClippingMaskLayer->setOffsetFromRenderer(toFloatSize(paddingBoxIncludingScrollbar.location()));
>         }
> 
>         bool paddingBoxOffsetChanged = oldScrollingLayerOffset != m_scrollingLayer->offsetFromRenderer();
>@@ -1228,7 +1228,7 @@ void RenderLayerBacking::updateGeometry()
> 
>         m_scrollingContentsLayer->setSize(scrollSize);
>         m_scrollingContentsLayer->setScrollOffset(scrollOffset, GraphicsLayer::DontSetNeedsDisplay);
>-        m_scrollingContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBox.location()), GraphicsLayer::DontSetNeedsDisplay);
>+        m_scrollingContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBoxIncludingScrollbar.location()), GraphicsLayer::DontSetNeedsDisplay);
> #endif
> 
>         if (m_foregroundLayer) {

Actions: View | Formatted Diff | Diff

Attachments on bug 193621: 359641