Attachment 339694 Details for Bug 185357 – Patch

[patch] Patch

bug-185357-20180506162759.patch (text/plain), 12.36 KB, created by zalan on 2018-05-06 16:28:00 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-05-06 16:28:00 PDT

Size: 12.36 KB

patch

obsolete

>Subversion Revision: 231395
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 293ccf0b6b21fd8c6f1c02b808d9f81094b47a9a..a56f351e43e91e0594ca6846549b168760fd1728 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,44 @@
>+2018-05-05  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Add assertions for stale Display::Box geometry
>+        https://bugs.webkit.org/show_bug.cgi?id=185357
>+
>+        Reviewed by Antti Koivisto.
>+
>+        Ensure that we don't access stale geometry of other boxes during layout.
>+        For example, in order to layout a block child we need the containing block's content box top/left and width (but not the height)
>+
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::invalidateTop):
>+        (WebCore::Display::Box::invalidateLeft):
>+        (WebCore::Display::Box::invalidateWidth):
>+        (WebCore::Display::Box::invalidateHeight):
>+        (WebCore::Display::Box::hasValidPosition const):
>+        (WebCore::Display::Box::hasValidSize const):
>+        (WebCore::Display::Box::hasValidGeometry const):
>+        (WebCore::Display::Box::invalidatePosition):
>+        (WebCore::Display::Box::invalidateSize):
>+        (WebCore::Display::Box::setHasValidPosition):
>+        (WebCore::Display::Box::setHasValidSize):
>+        (WebCore::Display::Box::setHasValidGeometry):
>+        (WebCore::Display::Box::rect const):
>+        (WebCore::Display::Box::top const):
>+        (WebCore::Display::Box::left const):
>+        (WebCore::Display::Box::bottom const):
>+        (WebCore::Display::Box::right const):
>+        (WebCore::Display::Box::topLeft const):
>+        (WebCore::Display::Box::bottomRight const):
>+        (WebCore::Display::Box::size const):
>+        (WebCore::Display::Box::width const):
>+        (WebCore::Display::Box::height const):
>+        (WebCore::Display::Box::setRect):
>+        (WebCore::Display::Box::setTopLeft):
>+        (WebCore::Display::Box::setTop):
>+        (WebCore::Display::Box::setLeft):
>+        (WebCore::Display::Box::setSize):
>+        (WebCore::Display::Box::setWidth):
>+        (WebCore::Display::Box::setHeight):
>+
> 2018-05-02  Dean Jackson  <dino@apple.com>
> 
>         Use IOSurfaces for CoreImage operations where possible
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.cpp b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>index eac3baf183ef49ae2bf5100afb593606d0fe061b..ccdf6483cd12ecd0cc538449f1f52f458e396e68 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>@@ -45,6 +45,7 @@ Box::~Box()
> 
> LayoutRect Box::marginBox() const
> {
>+    ASSERT(m_hasValidMargin);
>     auto marginBox = rect();
>     auto topLeftMargin = LayoutSize(m_marginLeft, m_marginTop);
>     marginBox.inflate(topLeftMargin);
>@@ -61,6 +62,7 @@ LayoutRect Box::borderBox() const
> 
> LayoutRect Box::paddingBox() const
> {
>+    ASSERT(m_hasValidBorder);
>     auto paddingBox = borderBox();
>     auto topLeftBorder = LayoutSize(m_borderLeft, m_borderTop);
>     paddingBox.inflate(-topLeftBorder);
>@@ -72,6 +74,7 @@ LayoutRect Box::paddingBox() const
> 
> LayoutRect Box::contentBox() const
> {
>+    ASSERT(m_hasValidPadding);
>     auto contentBox = paddingBox();
>     auto topLeftPadding = LayoutSize(m_paddingLeft, m_paddingTop);
>     contentBox.inflate(-topLeftPadding);
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index 73ea630599be9073d126e3c2f995726ad6928861..3661b802972d583f53079b18532c4d7b05c74a41 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -47,24 +47,24 @@ public:
> 
>     ~Box();
> 
>-    LayoutRect rect() const { return m_rect; }
>+    LayoutRect rect() const;
> 
>-    LayoutUnit top() const { return m_rect.y(); }
>-    LayoutUnit left() const { return m_rect.x(); }
>-    LayoutUnit bottom() const { return m_rect.maxY(); }
>-    LayoutUnit right() const { return m_rect.maxX(); }
>+    LayoutUnit top() const;
>+    LayoutUnit left() const;
>+    LayoutUnit bottom() const;
>+    LayoutUnit right() const;
> 
>-    LayoutPoint topLeft() const { return m_rect.location(); }
>-    LayoutPoint bottomRight() const { return m_rect.location(); }
>+    LayoutPoint topLeft() const;
>+    LayoutPoint bottomRight() const;
> 
>-    LayoutSize size() const { return m_rect.size(); }
>-    LayoutUnit width() const { return m_rect.width(); }
>-    LayoutUnit height() const { return m_rect.height(); }
>+    LayoutSize size() const;
>+    LayoutUnit width() const;
>+    LayoutUnit height() const;
> 
>-    LayoutUnit marginTop() const { return m_marginTop; }
>-    LayoutUnit marginLeft() const { return m_marginLeft; }
>-    LayoutUnit marginBottom() const { return m_marginBottom; }
>-    LayoutUnit marginRight() const { return m_marginRight; }
>+    LayoutUnit marginTop() const;
>+    LayoutUnit marginLeft() const;
>+    LayoutUnit marginBottom() const;
>+    LayoutUnit marginRight() const;
> 
>     LayoutRect marginBox() const;
>     LayoutRect borderBox() const;
>@@ -74,30 +74,44 @@ public:
> private:
>     Box();
> 
>-    void setRect(const LayoutRect& rect) { m_rect = rect; }
>-    void setTopLeft(const LayoutPoint& topLeft) { m_rect.setLocation(topLeft); }
>-    void setTop(LayoutUnit top) { m_rect.setY(top); }
>-    void setLeft(LayoutUnit left) { m_rect.setX(left); }
>-    void setSize(const LayoutSize& size) { m_rect.setSize(size); }
>-    void setWidth(LayoutUnit width) { m_rect.setWidth(width); }
>-    void setHeight(LayoutUnit height) { m_rect.setHeight(height); }
>-
>-    void setMarginTop(LayoutUnit marginTop) { m_marginTop = marginTop; }
>-    void setMarginLeft(LayoutUnit marginLeft) { m_marginLeft = marginLeft; }
>-    void setMarginBottom(LayoutUnit marginBottom) { m_marginBottom = marginBottom; }
>-    void setMarginRight(LayoutUnit marginRight) { m_marginRight = marginRight; }
>-
>-    void setBorderTop(LayoutUnit borderTop) { m_borderTop = borderTop; }
>-    void setBorderLeft(LayoutUnit borderLeft) { m_borderLeft = borderLeft; }
>-    void setBorderBottom(LayoutUnit borderBottom) { m_borderBottom = borderBottom; }
>-    void setBorderRight(LayoutUnit borderRight) { m_borderRight = borderRight; }
>-
>-    void setPaddingTop(LayoutUnit paddingTop) { m_paddingTop = paddingTop; }
>-    void setPaddingLeft(LayoutUnit paddingLeft) { m_paddingLeft = paddingLeft; }
>-    void setPaddingBottom(LayoutUnit paddingBottom) { m_paddingBottom = paddingBottom; }
>-    void setPaddingRight(LayoutUnit paddingRight) { m_paddingRight = paddingRight; }
>+    void setRect(const LayoutRect&);
>+    void setTopLeft(const LayoutPoint&);
>+    void setTop(LayoutUnit);
>+    void setLeft(LayoutUnit);
>+    void setSize(const LayoutSize&);
>+    void setWidth(LayoutUnit);
>+    void setHeight(LayoutUnit);
>+
>+    void setMargin(LayoutUnit marginTop, LayoutUnit marginLeft, LayoutUnit marginRight, LayoutUnit marginBottom);
>+    void setBorder(LayoutUnit borderTop, LayoutUnit borderLeft, LayoutUnit borderRight, LayoutUnit borderBottom);
>+    void setPadding(LayoutUnit paddingTop, LayoutUnit paddingLeft, LayoutUnit paddingRight, LayoutUnit paddingBottom);
>+
>+#if !ASSERT_DISABLED
>+    void invalidateTop() { m_hasValidTop = false; }
>+    void invalidateLeft() { m_hasValidLeft = false; }
>+    void invalidateWidth() { m_hasValidWidth = false; }
>+    void invalidateHeight() { m_hasValidHeight = false; }
>+    void invalidatePosition();
>+    void invalidateSize();
>+    void invalidateMargin() { m_hasValidMargin = false; }
>+    void invalidateBorder() { m_hasValidBorder = false; }
>+    void invalidatePadding() { m_hasValidPadding = false; }
>+
>+    bool hasValidPosition() const { return m_hasValidTop && m_hasValidLeft; }
>+    bool hasValidSize() const { return m_hasValidWidth && m_hasValidHeight; }
>+    bool hasValidGeometry() const { return hasValidPosition() && hasValidSize(); }
>+    
>+    void setHasValidPosition();
>+    void setHasValidSize();
>+    void setHasValidGeometry();
>+    
>+    void setHasValidMargin();
>+    void setHasValidBorder();
>+    void setHasValidPadding();
>+#endif
> 
>     LayoutRect m_rect;
>+
>     LayoutUnit m_marginTop;
>     LayoutUnit m_marginLeft;
>     LayoutUnit m_marginBottom;
>@@ -112,8 +126,223 @@ private:
>     LayoutUnit m_paddingLeft;
>     LayoutUnit m_paddingBottom;
>     LayoutUnit m_paddingRight;
>+
>+#if !ASSERT_DISABLED
>+    bool m_hasValidTop { false };
>+    bool m_hasValidLeft { false };
>+    bool m_hasValidWidth { false };
>+    bool m_hasValidHeight { false };
>+    bool m_hasValidMargin { false };
>+    bool m_hasValidBorder { false };
>+    bool m_hasValidPadding { false };
>+#endif
> };
> 
>+#if !ASSERT_DISABLED
>+inline void Box::invalidatePosition()
>+{
>+    invalidateTop();
>+    invalidateLeft();
>+}
>+
>+inline void Box::invalidateSize()
>+{
>+    invalidateWidth();
>+    invalidateHeight();
>+}
>+
>+inline void Box::setHasValidPosition()
>+{
>+    m_hasValidTop = true;
>+    m_hasValidLeft = true;
>+}
>+
>+inline void Box::setHasValidSize()
>+{
>+    m_hasValidWidth = true;
>+    m_hasValidHeight = true;
>+}
>+
>+inline void Box::setHasValidGeometry()
>+{
>+    setHasValidPosition();
>+    setHasValidSize();
>+}
>+#endif
>+
>+inline LayoutRect Box::rect() const
>+{
>+    ASSERT(hasValidGeometry());
>+    return m_rect;
>+}
>+
>+inline LayoutUnit Box::top() const
>+{
>+    ASSERT(m_hasValidTop);
>+    return m_rect.y();
>+}
>+
>+inline LayoutUnit Box::left() const
>+{
>+    ASSERT(m_hasValidLeft);
>+    return m_rect.x();
>+}
>+
>+inline LayoutUnit Box::bottom() const
>+{
>+    ASSERT(m_hasValidTop && m_hasValidHeight);
>+    return m_rect.maxY();
>+}
>+
>+inline LayoutUnit Box::right() const
>+{
>+    ASSERT(m_hasValidLeft && m_hasValidWidth);
>+    return m_rect.maxX();
>+}
>+
>+inline LayoutPoint Box::topLeft() const
>+{
>+    ASSERT(hasValidPosition());
>+    return m_rect.location();
>+}
>+
>+inline LayoutPoint Box::bottomRight() const
>+{
>+    ASSERT(hasValidGeometry());
>+    return m_rect.maxXMaxYCorner();
>+}
>+
>+inline LayoutSize Box::size() const
>+{
>+    ASSERT(hasValidSize());
>+    return m_rect.size();
>+}
>+
>+inline LayoutUnit Box::width() const
>+{
>+    ASSERT(m_hasValidWidth);
>+    return m_rect.width();
>+}
>+
>+inline LayoutUnit Box::height() const
>+{
>+    ASSERT(m_hasValidHeight);
>+    return m_rect.height();
>+}
>+
>+inline void Box::setRect(const LayoutRect& rect)
>+{
>+#if !ASSERT_DISABLED
>+    setHasValidGeometry();
>+#endif
>+    m_rect = rect;
>+}
>+
>+inline void Box::setTopLeft(const LayoutPoint& topLeft)
>+{
>+#if !ASSERT_DISABLED
>+    setHasValidPosition();
>+#endif
>+    m_rect.setLocation(topLeft);
>+}
>+
>+inline void Box::setTop(LayoutUnit top)
>+{
>+#if !ASSERT_DISABLED
>+    m_hasValidTop = true;
>+#endif
>+    m_rect.setY(top);
>+}
>+
>+inline void Box::setLeft(LayoutUnit left)
>+{
>+#if !ASSERT_DISABLED
>+    m_hasValidLeft = true;
>+#endif
>+    m_rect.setX(left);
>+}
>+
>+inline void Box::setSize(const LayoutSize& size)
>+{
>+#if !ASSERT_DISABLED
>+    setHasValidSize();
>+#endif
>+    m_rect.setSize(size);
>+}
>+
>+inline void Box::setWidth(LayoutUnit width)
>+{
>+#if !ASSERT_DISABLED
>+    m_hasValidWidth = true;
>+#endif
>+    m_rect.setWidth(width);
>+}
>+
>+inline void Box::setHeight(LayoutUnit height)
>+{
>+#if !ASSERT_DISABLED
>+    m_hasValidHeight = true;
>+#endif
>+    m_rect.setHeight(height);
>+}
>+
>+inline void Box::setMargin(LayoutUnit marginTop, LayoutUnit marginLeft, LayoutUnit marginRight, LayoutUnit marginBottom)
>+{
>+#if !ASSERT_DISABLED
>+    void setHasValidMargin();
>+#endif
>+    m_marginTop = marginTop;
>+    m_marginLeft = marginLeft;
>+    m_marginBottom = marginBottom;
>+    m_marginRight = marginRight;
>+}
>+
>+inline void Box::setBorder(LayoutUnit borderTop, LayoutUnit borderLeft, LayoutUnit borderRight, LayoutUnit borderBottom)
>+{
>+#if !ASSERT_DISABLED
>+    void setHasValidBorder();
>+#endif
>+    m_borderTop = borderTop;
>+    m_borderLeft = borderLeft;
>+    m_borderBottom = borderBottom;
>+    m_borderRight = borderRight;
>+}
>+
>+inline void Box::setPadding(LayoutUnit paddingTop, LayoutUnit paddingLeft, LayoutUnit paddingRight, LayoutUnit paddingBottom)
>+{
>+#if !ASSERT_DISABLED
>+    void setHasValidPadding();
>+#endif
>+    m_paddingTop = paddingTop;
>+    m_paddingLeft = paddingLeft;
>+    m_paddingBottom = paddingBottom;
>+    m_paddingRight = paddingRight;
>+}
>+
>+inline LayoutUnit Box::marginTop() const
>+{
>+    ASSERT(m_hasValidMargin);
>+    return m_marginTop;
>+}
>+
>+inline LayoutUnit Box::marginLeft() const
>+{
>+    ASSERT(m_hasValidMargin);
>+    return m_marginLeft;
>+}
>+
>+inline LayoutUnit Box::marginBottom() const
>+{
>+    ASSERT(m_hasValidMargin);
>+    return m_marginBottom;
>+}
>+
>+inline LayoutUnit Box::marginRight() const
>+{
>+    ASSERT(m_hasValidMargin);
>+    return m_marginRight;
>+}
>+
> }
> }
> #endif

Actions: View | Formatted Diff | Diff

Attachments on bug 185357: 339664 | 339669 | 339694