Attachment 341488 Details for Bug 186052 – Patch

[patch] Patch

bug-186052-20180529075007.patch (text/plain), 12.03 KB, created by zalan on 2018-05-29 07:50:08 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-05-29 07:50:08 PDT

Size: 12.03 KB

patch

obsolete

>Subversion Revision: 232251
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c097e127830359c7ba176f3fbc2a8ed5185e0006..d424c2c74c38caaaf4163571dd643d48414bd8e4 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,38 @@
>+2018-05-29  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Miscellaneous fixes to ensure no assertion in LayoutContext::layout
>+        https://bugs.webkit.org/show_bug.cgi?id=186052
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        With this patch, LayoutContext::layout() does not assert on <html><body><div></div></body></html> anymore.
>+
>+        * layout/LayoutContext.cpp:
>+        (WebCore::Layout::LayoutContext::initializeRoot): New context root is always a layout root.
>+        * layout/LayoutContext.h:
>+        * layout/Verification.cpp:
>+        (WebCore::Layout::outputMismatchingBoxInformationIfNeeded):
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::layout const): we need computed margin/border/padding for width computation  
>+        * layout/displaytree/DisplayBox.cpp: Add clone() method to be able to carry over the 'hasValid*' bits.
>+        (WebCore::Display::Box::Style::Style):
>+        (WebCore::Display::Box::borderBox const):
>+        (WebCore::Display::Box::contentBox const):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::Rect::operator LayoutRect const):
>+        (WebCore::Display::Box::setSize):
>+        (WebCore::Display::Box::setHasValidMargin):
>+        (WebCore::Display::Box::setHasValidBorder):
>+        (WebCore::Display::Box::setHasValidPadding):
>+        (WebCore::Display::Box::Rect::setHasValidPosition):
>+        (WebCore::Display::Box::Rect::setHasValidSize):
>+        (WebCore::Display::Box::Rect::setSize):
>+        (WebCore::Display::Box::Rect::clone const):
>+        (WebCore::Display::Box::setMargin):
>+        (WebCore::Display::Box::setBorder):
>+        (WebCore::Display::Box::setPadding):
>+        (WebCore::Display::Box::Rect::Rect): Deleted.
>+
> 2018-05-28  Zalan Bujtas  <zalan@apple.com>
> 
>         Unreviewed build fix.
>diff --git a/Source/WebCore/layout/LayoutContext.cpp b/Source/WebCore/layout/LayoutContext.cpp
>index 202b8e4cfaccd824ac114f27b6e10c6a65ad2ace..8f28e7ab8df71e8891475b125e6a1744492b3858 100644
>--- a/Source/WebCore/layout/LayoutContext.cpp
>+++ b/Source/WebCore/layout/LayoutContext.cpp
>@@ -51,12 +51,13 @@ LayoutContext::LayoutContext()
> 
> void LayoutContext::initializeRoot(const Container& root, const LayoutSize& containerSize)
> {
>+    ASSERT(root.establishesFormattingContext());
>+
>     m_root = makeWeakPtr(const_cast<Container&>(root));
>     auto& displayBox = createDisplayBox(root);
>     // Root is always at 0 0 with no margin 
>     displayBox.setTopLeft({ });
>-    displayBox.setWidth(containerSize.width());
>-    displayBox.setHeight(containerSize.height());
>+    displayBox.setSize(containerSize);
>     displayBox.setMargin({ });
> 
>     auto& style = root.style();
>@@ -66,14 +67,16 @@ void LayoutContext::initializeRoot(const Container& root, const LayoutSize& cont
>         style.borderLeft().width(),
>         style.borderBottom().width(),
>         style.borderRight().width()
>-
>     });
>+
>     displayBox.setPadding({
>         valueForLength(style.paddingTop(), containerSize.width()),
>         valueForLength(style.paddingLeft(), containerSize.width()),
>         valueForLength(style.paddingBottom(), containerSize.width()),
>         valueForLength(style.paddingRight(), containerSize.width())
>     });
>+
>+    m_formattingContextRootListForLayout.add(&root);
> }
> 
> void LayoutContext::updateLayout()
>diff --git a/Source/WebCore/layout/LayoutContext.h b/Source/WebCore/layout/LayoutContext.h
>index 703d83f2d2b881735d4b099289e8b31b9da7ea93..d02b6e549646e1c2815a7b8ccad68b2bd6bdf3ce 100644
>--- a/Source/WebCore/layout/LayoutContext.h
>+++ b/Source/WebCore/layout/LayoutContext.h
>@@ -77,12 +77,12 @@ public:
>     FormattingState& establishedFormattingState(const Box& formattingContextRoot, const FormattingContext&);
>     std::unique_ptr<FormattingContext> formattingContext(const Box& formattingContextRoot);
> 
>-    // For testing purposes only
>-    void verifyAndOutputMismatchingLayoutTree(const RenderView&) const;
>-
>     Display::Box& createDisplayBox(const Box&);
>     Display::Box* displayBoxForLayoutBox(const Box& layoutBox) const { return m_layoutToDisplayBox.get(&layoutBox); }
> 
>+    // For testing purposes only
>+    void verifyAndOutputMismatchingLayoutTree(const RenderView&) const;
>+
> private:
>     WeakPtr<Container> m_root;
>     HashSet<const Container*> m_formattingContextRootListForLayout;
>diff --git a/Source/WebCore/layout/Verification.cpp b/Source/WebCore/layout/Verification.cpp
>index 5f168414fd88c8aa844194421a7e4c15aaf8aad3..a7af7311631574b8f47788ce9c71906691e50540 100644
>--- a/Source/WebCore/layout/Verification.cpp
>+++ b/Source/WebCore/layout/Verification.cpp
>@@ -56,7 +56,7 @@ static void outputMismatchingBoxInformationIfNeeded(TextStream& stream, const La
>     auto* displayBox = context.displayBoxForLayoutBox(layoutBox);
>     ASSERT(displayBox);
> 
>-    if (renderer.marginBoxRect() != displayBox->marginBox())
>+    if (renderer.frameRect() != displayBox->rect())
>         outputRect("frameBox", renderer.frameRect(), displayBox->rect());
> 
>     if (renderer.marginBoxRect() != displayBox->marginBox())
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index bdea3ab0926d508ff6be166d4a99e0428bb47e37..5c06a65408a0b0b4f3256125f0599eac0c031f1a 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -73,10 +73,10 @@ void BlockFormattingContext::layout(LayoutContext& layoutContext, FormattingStat
>             auto& layoutBox = layoutPair.layoutBox;
>             auto& displayBox = layoutPair.displayBox;
>             
>-            computeWidth(layoutContext, layoutBox, displayBox);
>             computeMargin(layoutContext, layoutBox, displayBox);
>             computeBorderAndPadding(layoutContext, layoutBox, displayBox);
>             computeStaticPosition(layoutContext, layoutBox, displayBox);
>+            computeWidth(layoutContext, layoutBox, displayBox);
>             if (layoutBox.establishesFormattingContext()) {
>                 auto formattingContext = layoutContext.formattingContext(layoutBox);
>                 formattingContext->layout(layoutContext, layoutContext.establishedFormattingState(layoutBox, *formattingContext));
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.cpp b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>index f4f373935135f69c59bcba82e8ebcb806f52d9bd..d7d55847efcd85bb0154f496d1a01b5ce971b94a 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>@@ -48,7 +48,6 @@ Box::~Box()
> Box::Style::Style(const RenderStyle& style)
>     : boxSizing(style.boxSizing())
> {
>-
> }
> 
> Box::Rect Box::marginBox() const
>@@ -66,15 +65,17 @@ Box::Rect Box::marginBox() const
> 
> Box::Rect Box::borderBox() const
> {
>+    auto rect = m_rect.clone();
>+    rect.setTopLeft({ });
>+
>     if (m_style.boxSizing == BoxSizing::BorderBox)
>-        return Box::Rect( { }, size());
>+        return rect;
> 
>     // Width is content box.
>     ASSERT(m_hasValidBorder);
>     ASSERT(m_hasValidPadding);
>-    auto borderBoxSize = size();
>-    borderBoxSize.expand(borderLeft() + paddingLeft() + paddingRight() + borderRight(), borderTop() + paddingTop() + paddingBottom() + borderBottom());
>-    return Box::Rect( { }, borderBoxSize);
>+    rect.expand(borderLeft() + paddingLeft() + paddingRight() + borderRight(), borderTop() + paddingTop() + paddingBottom() + borderBottom());
>+    return rect;
> }
> 
> Box::Rect Box::paddingBox() const
>@@ -92,8 +93,11 @@ Box::Rect Box::paddingBox() const
> 
> Box::Rect Box::contentBox() const
> {
>-    if (m_style.boxSizing == BoxSizing::ContentBox)
>-        return Box::Rect(LayoutPoint(0, 0), size());
>+    if (m_style.boxSizing == BoxSizing::ContentBox) {
>+        auto rect = m_rect.clone();
>+        rect.setTopLeft({ });
>+        return rect;
>+    }
> 
>     // Width is border box.
>     ASSERT(m_hasValidPadding);
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index 6ae570ab70c9dc352261e567d011f4193d248393..8c1e261e4c01993a702ce22a9c0a5d5558850dea 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -55,7 +55,6 @@ public:
>     class Rect {
>     public:
>         Rect() = default;
>-        Rect(const LayoutPoint&, const LayoutSize&);
>         
>         LayoutUnit top() const;
>         LayoutUnit left() const;
>@@ -74,6 +73,7 @@ public:
>         void setTopLeft(const LayoutPoint&);
>         void setWidth(LayoutUnit);
>         void setHeight(LayoutUnit);
>+        void setSize(const LayoutSize&);
> 
>         void shiftLeftTo(LayoutUnit);
>         void shiftRightTo(LayoutUnit);
>@@ -82,7 +82,8 @@ public:
> 
>         void expand(LayoutUnit, LayoutUnit);
> 
>-        operator LayoutRect() const { return m_rect; }
>+        Rect clone() const;
>+        operator LayoutRect() const;
> 
>     private:
> #if !ASSERT_DISABLED
>@@ -97,6 +98,7 @@ public:
>         bool hasValidGeometry() const { return hasValidPosition() && hasValidSize(); }
>     
>         void setHasValidPosition();
>+        void setHasValidSize();
> 
>         bool m_hasValidTop { false };
>         bool m_hasValidLeft { false };
>@@ -156,6 +158,7 @@ private:
>     void setLeft(LayoutUnit left) { m_rect.setLeft(left); }
>     void setWidth(LayoutUnit width) { m_rect.setWidth(width); }
>     void setHeight(LayoutUnit height) { m_rect.setHeight(height); }
>+    void setSize(const LayoutSize& size) { m_rect.setSize(size); }
> 
>     struct Edges {
>         Edges() = default;
>@@ -180,9 +183,9 @@ private:
>     void invalidateBorder() { m_hasValidBorder = false; }
>     void invalidatePadding() { m_hasValidPadding = false; }
> 
>-    void setHasValidMargin();
>-    void setHasValidBorder();
>-    void setHasValidPadding();
>+    void setHasValidMargin() { m_hasValidMargin = true; }
>+    void setHasValidBorder() { m_hasValidBorder = true; }
>+    void setHasValidPadding() { m_hasValidPadding = true; }
> #endif
> 
>     const Style m_style;
>@@ -212,13 +215,13 @@ inline void Box::Rect::setHasValidPosition()
>     m_hasValidTop = true;
>     m_hasValidLeft = true;
> }
>-#endif
> 
>-inline Box::Rect::Rect(const LayoutPoint& topLeft, const LayoutSize& size)
>-    : m_rect(topLeft, size)
>+inline void Box::Rect::setHasValidSize()
> {
>-
>+    m_hasValidWidth = true;
>+    m_hasValidHeight = true;
> }
>+#endif
> 
> inline LayoutUnit Box::Rect::top() const
> {
>@@ -316,6 +319,14 @@ inline void Box::Rect::setHeight(LayoutUnit height)
>     m_rect.setHeight(height);
> }
> 
>+inline void Box::Rect::setSize(const LayoutSize& size)
>+{
>+#if !ASSERT_DISABLED
>+    setHasValidSize();
>+#endif
>+    m_rect.setSize(size);
>+}
>+
> inline void Box::Rect::shiftLeftTo(LayoutUnit left)
> {
>     ASSERT(m_hasValidLeft);
>@@ -346,10 +357,29 @@ inline void Box::Rect::expand(LayoutUnit width, LayoutUnit height)
>     m_rect.expand(width, height);
> }
> 
>+inline Box::Rect Box::Rect::clone() const
>+{
>+    Rect rect;
>+#if !ASSERT_DISABLED
>+    rect.m_hasValidTop = m_hasValidTop;
>+    rect.m_hasValidLeft = m_hasValidLeft;
>+    rect.m_hasValidWidth = m_hasValidWidth;
>+    rect.m_hasValidHeight  = m_hasValidHeight;
>+#endif 
>+    rect.m_rect = m_rect;
>+    return rect;
>+}
>+
>+inline Box::Rect::operator LayoutRect() const
>+{
>+    ASSERT(hasValidGeometry()); 
>+    return m_rect;
>+}
>+
> inline void Box::setMargin(Edges margin)
> {
> #if !ASSERT_DISABLED
>-    void setHasValidMargin();
>+    setHasValidMargin();
> #endif
>     m_margin = margin;
> }
>@@ -357,7 +387,7 @@ inline void Box::setMargin(Edges margin)
> inline void Box::setBorder(Edges border)
> {
> #if !ASSERT_DISABLED
>-    void setHasValidBorder();
>+    setHasValidBorder();
> #endif
>     m_border = border;
> }
>@@ -365,7 +395,7 @@ inline void Box::setBorder(Edges border)
> inline void Box::setPadding(Edges padding)
> {
> #if !ASSERT_DISABLED
>-    void setHasValidPadding();
>+    setHasValidPadding();
> #endif
>     m_padding = padding;
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 186052: 341488