Title: [144344] trunk/Source/WebCore
Revision
144344
Author
[email protected]
Date
2013-02-28 11:59:23 -0800 (Thu, 28 Feb 2013)

Log Message

Remove the quirk margin bits from RenderObject and put them back in RenderBlock.
https://bugs.webkit.org/show_bug.cgi?id=111089

Reviewed by Dan Bernstein.

This patch removes the marginBeforeQuirk and marginAfterQuirk bits from RenderObject
and puts them into RenderBlock instead. I also did some renaming and clean-up after
moving them, e.g., to hasMarginBeforeQuirk and hasMarginAfterQuirk.
        
Even though it's pretty irrelevant, I also made the code writing-mode-correct so that
the correct child margin quirk is propagated across differing writing mode
boundaries.

* rendering/RenderBlock.cpp:
(WebCore::RenderBlock::MarginInfo::MarginInfo):
(WebCore::RenderBlock::RenderBlock):
(WebCore::RenderBlock::layoutBlock):
(WebCore::RenderBlock::collapseMargins):
(WebCore::RenderBlock::marginBeforeEstimateForChild):
(WebCore::RenderBlock::setCollapsedBottomMargin):
(WebCore::RenderBlock::handleAfterSideOfBlock):
(WebCore::RenderBlock::hasMarginBeforeQuirk):
(WebCore):
(WebCore::RenderBlock::hasMarginAfterQuirk):
* rendering/RenderBlock.h:
(WebCore::RenderBlock::setHasMarginBeforeQuirk):
(WebCore::RenderBlock::setHasMarginAfterQuirk):
(RenderBlock):
(WebCore::RenderBlock::hasMarginBeforeQuirk):
(WebCore::RenderBlock::hasMarginAfterQuirk):
(MarginInfo):
(WebCore::RenderBlock::MarginInfo::setHasMarginBeforeQuirk):
(WebCore::RenderBlock::MarginInfo::setHasMarginAfterQuirk):
(WebCore::RenderBlock::MarginInfo::hasMarginBeforeQuirk):
(WebCore::RenderBlock::MarginInfo::hasMarginAfterQuirk):
* rendering/RenderObject.h:
(RenderObject):
(WebCore::RenderObject::RenderObjectBitfields::RenderObjectBitfields):
(RenderObjectBitfields):
* rendering/style/RenderStyle.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (144343 => 144344)


--- trunk/Source/WebCore/ChangeLog	2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/ChangeLog	2013-02-28 19:59:23 UTC (rev 144344)
@@ -1,3 +1,46 @@
+2013-02-28  David Hyatt  <[email protected]>
+
+        Remove the quirk margin bits from RenderObject and put them back in RenderBlock.
+        https://bugs.webkit.org/show_bug.cgi?id=111089
+
+        Reviewed by Dan Bernstein.
+
+        This patch removes the marginBeforeQuirk and marginAfterQuirk bits from RenderObject
+        and puts them into RenderBlock instead. I also did some renaming and clean-up after
+        moving them, e.g., to hasMarginBeforeQuirk and hasMarginAfterQuirk.
+        
+        Even though it's pretty irrelevant, I also made the code writing-mode-correct so that
+        the correct child margin quirk is propagated across differing writing mode
+        boundaries.
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::RenderBlock::MarginInfo::MarginInfo):
+        (WebCore::RenderBlock::RenderBlock):
+        (WebCore::RenderBlock::layoutBlock):
+        (WebCore::RenderBlock::collapseMargins):
+        (WebCore::RenderBlock::marginBeforeEstimateForChild):
+        (WebCore::RenderBlock::setCollapsedBottomMargin):
+        (WebCore::RenderBlock::handleAfterSideOfBlock):
+        (WebCore::RenderBlock::hasMarginBeforeQuirk):
+        (WebCore):
+        (WebCore::RenderBlock::hasMarginAfterQuirk):
+        * rendering/RenderBlock.h:
+        (WebCore::RenderBlock::setHasMarginBeforeQuirk):
+        (WebCore::RenderBlock::setHasMarginAfterQuirk):
+        (RenderBlock):
+        (WebCore::RenderBlock::hasMarginBeforeQuirk):
+        (WebCore::RenderBlock::hasMarginAfterQuirk):
+        (MarginInfo):
+        (WebCore::RenderBlock::MarginInfo::setHasMarginBeforeQuirk):
+        (WebCore::RenderBlock::MarginInfo::setHasMarginAfterQuirk):
+        (WebCore::RenderBlock::MarginInfo::hasMarginBeforeQuirk):
+        (WebCore::RenderBlock::MarginInfo::hasMarginAfterQuirk):
+        * rendering/RenderObject.h:
+        (RenderObject):
+        (WebCore::RenderObject::RenderObjectBitfields::RenderObjectBitfields):
+        (RenderObjectBitfields):
+        * rendering/style/RenderStyle.h:
+
 2013-02-28  Sheriff Bot  <[email protected]>
 
         Unreviewed, rolling out r144126 and r144176.

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (144343 => 144344)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2013-02-28 19:59:23 UTC (rev 144344)
@@ -167,8 +167,8 @@
 RenderBlock::MarginInfo::MarginInfo(RenderBlock* block, LayoutUnit beforeBorderPadding, LayoutUnit afterBorderPadding)
     : m_atBeforeSideOfBlock(true)
     , m_atAfterSideOfBlock(false)
-    , m_marginBeforeQuirk(false)
-    , m_marginAfterQuirk(false)
+    , m_hasMarginBeforeQuirk(false)
+    , m_hasMarginAfterQuirk(false)
     , m_determinedMarginBeforeQuirk(false)
     , m_discardMargin(false)
 {
@@ -201,6 +201,8 @@
 RenderBlock::RenderBlock(ContainerNode* node)
     : RenderBox(node)
     , m_lineHeight(-1)
+    , m_hasMarginBeforeQuirk(false)
+    , m_hasMarginAfterQuirk(false)
     , m_beingDestroyed(false)
     , m_hasMarkupTruncation(false)
     , m_hasBorderOrPaddingLogicalWidthChanged(false)
@@ -1553,8 +1555,8 @@
     if (!isCell) {
         initMaxMarginValues();
         
-        setMarginBeforeQuirk(styleToUse->isMarginBeforeQuirk());
-        setMarginAfterQuirk(styleToUse->isMarginAfterQuirk());
+        setHasMarginBeforeQuirk(styleToUse->hasMarginBeforeQuirk());
+        setHasMarginAfterQuirk(styleToUse->hasMarginAfterQuirk());
         setPaginationStrut(0);
     }
 
@@ -1989,7 +1991,7 @@
     
     // See if the top margin is quirky. We only care if this child has
     // margins that will collapse with us.
-    bool topQuirk = child->isMarginBeforeQuirk();
+    bool topQuirk = hasMarginBeforeQuirk(child);
 
     if (marginInfo.canCollapseWithMarginBefore()) {
         if (!childDiscardMarginBefore && !marginInfo.discardMargin()) {
@@ -2004,7 +2006,7 @@
             // has an example of this, a <dt> with 0.8em author-specified inside
             // a <dl> inside a <td>.
             if (!marginInfo.determinedMarginBeforeQuirk() && !topQuirk && (posTop - negTop)) {
-                setMarginBeforeQuirk(false);
+                setHasMarginBeforeQuirk(false);
                 marginInfo.setDeterminedMarginBeforeQuirk(true);
             }
 
@@ -2014,7 +2016,7 @@
                 // This deals with the <td><div><p> case.
                 // Don't do this for a block that split two inlines though. You do
                 // still apply margins in this case.
-                setMarginBeforeQuirk(true);
+                setHasMarginBeforeQuirk(true);
         } else
             // The before margin of the container will also discard all the margins it is collapsing with.
             setMustDiscardMarginBefore();
@@ -2027,7 +2029,7 @@
     }
 
     if (marginInfo.quirkContainer() && marginInfo.atBeforeSideOfBlock() && (posTop - negTop))
-        marginInfo.setMarginBeforeQuirk(topQuirk);
+        marginInfo.setHasMarginBeforeQuirk(topQuirk);
 
     LayoutUnit beforeCollapseLogicalTop = logicalHeight();
     LayoutUnit logicalTop = beforeCollapseLogicalTop;
@@ -2065,7 +2067,7 @@
             logicalTop = logicalHeight();
         } else if (!marginInfo.discardMargin() && (!marginInfo.atBeforeSideOfBlock()
             || (!marginInfo.canCollapseMarginBeforeWithChildren()
-            && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginBeforeQuirk())))) {
+            && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.hasMarginBeforeQuirk())))) {
             // We're collapsing with a previous sibling's margins and not
             // with the top of the block.
             setLogicalHeight(logicalHeight() + max(marginInfo.positiveMargin(), posTop) - max(marginInfo.negativeMargin(), negTop));
@@ -2081,7 +2083,7 @@
             marginInfo.clearMargin();
 
         if (marginInfo.margin())
-            marginInfo.setMarginAfterQuirk(child->isMarginAfterQuirk());
+            marginInfo.setHasMarginAfterQuirk(hasMarginAfterQuirk(child));
     }
     
     // If margins would pull us past the top of the next page, then we need to pull back and pretend like the margins
@@ -2181,7 +2183,7 @@
     // Give up if in quirks mode and we're a body/table cell and the top margin of the child box is quirky.
     // Give up if the child specified -webkit-margin-collapse: separate that prevents collapsing.
     // FIXME: Use writing mode independent accessor for marginBeforeCollapse.
-    if ((document()->inQuirksMode() && child->isMarginBeforeQuirk() && (isTableCell() || isBody())) || child->style()->marginBeforeCollapse() == MSEPARATE)
+    if ((document()->inQuirksMode() && hasMarginAfterQuirk(child) && (isTableCell() || isBody())) || child->style()->marginBeforeCollapse() == MSEPARATE)
         return;
 
     // The margins are discarded by a child that specified -webkit-margin-collapse: discard.
@@ -2221,8 +2223,11 @@
     // Make sure to update the block margins now for the grandchild box so that we're looking at current values.
     if (grandchildBox->needsLayout()) {
         grandchildBox->computeAndSetBlockDirectionMargins(this);
-        grandchildBox->setMarginBeforeQuirk(grandchildBox->style()->isMarginBeforeQuirk());
-        grandchildBox->setMarginAfterQuirk(grandchildBox->style()->isMarginAfterQuirk());
+        if (grandchildBox->isRenderBlock()) {
+            RenderBlock* grandchildBlock = toRenderBlock(grandchildBox);
+            grandchildBlock->setHasMarginBeforeQuirk(grandchildBox->style()->hasMarginBeforeQuirk());
+            grandchildBlock->setHasMarginAfterQuirk(grandchildBox->style()->hasMarginAfterQuirk());
+        }
     }
 
     // Collapse the margin of the grandchild box with our own to produce an estimate.
@@ -2338,14 +2343,14 @@
         // with our children.
         setMaxMarginAfterValues(max(maxPositiveMarginAfter(), marginInfo.positiveMargin()), max(maxNegativeMarginAfter(), marginInfo.negativeMargin()));
 
-        if (!marginInfo.marginAfterQuirk())
-            setMarginAfterQuirk(false);
+        if (!marginInfo.hasMarginAfterQuirk())
+            setHasMarginAfterQuirk(false);
 
-        if (marginInfo.marginAfterQuirk() && marginAfter() == 0)
+        if (marginInfo.hasMarginAfterQuirk() && !marginAfter())
             // We have no bottom margin and our last child has a quirky margin.
             // We will pick up this quirky margin and pass it through.
             // This deals with the <td><div><p> case.
-            setMarginAfterQuirk(true);
+            setHasMarginAfterQuirk(true);
     }
 }
 
@@ -2358,7 +2363,7 @@
     // its margin.
     if (!marginInfo.discardMargin() && (!marginInfo.canCollapseWithMarginAfter() && !marginInfo.canCollapseWithMarginBefore()
         && (!isAnonymousBlock() || isAnonymousColumnsBlock() || isAnonymousColumnSpanBlock())
-        && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.marginAfterQuirk())))
+        && (!document()->inQuirksMode() || !marginInfo.quirkContainer() || !marginInfo.hasMarginAfterQuirk())))
         setLogicalHeight(logicalHeight() + marginInfo.margin());
         
     // Now add in our bottom border/padding.
@@ -7639,6 +7644,40 @@
     return marginAfterForChild(child);
 }
 
+bool RenderBlock::hasMarginBeforeQuirk(const RenderBox* child) const
+{
+    // If the child has the same directionality as we do, then we can just return its
+    // margin quirk.
+    if (!child->isWritingModeRoot())
+        return child->isRenderBlock() ? toRenderBlock(child)->hasMarginBeforeQuirk() : child->style()->hasMarginBeforeQuirk();
+    
+    // The child has a different directionality. If the child is parallel, then it's just
+    // flipped relative to us. We can use the opposite edge.
+    if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+        return child->isRenderBlock() ? toRenderBlock(child)->hasMarginAfterQuirk() : child->style()->hasMarginAfterQuirk();
+    
+    // The child is perpendicular to us and box sides are never quirky in html.css, and we don't really care about
+    // whether or not authors specified quirky ems, since they're an implementation detail.
+    return false;
+}
+
+bool RenderBlock::hasMarginAfterQuirk(const RenderBox* child) const
+{
+    // If the child has the same directionality as we do, then we can just return its
+    // margin quirk.
+    if (!child->isWritingModeRoot())
+        return child->isRenderBlock() ? toRenderBlock(child)->hasMarginAfterQuirk() : child->style()->hasMarginAfterQuirk();
+    
+    // The child has a different directionality. If the child is parallel, then it's just
+    // flipped relative to us. We can use the opposite edge.
+    if (child->isHorizontalWritingMode() == isHorizontalWritingMode())
+        return child->isRenderBlock() ? toRenderBlock(child)->hasMarginBeforeQuirk() : child->style()->hasMarginBeforeQuirk();
+    
+    // The child is perpendicular to us and box sides are never quirky in html.css, and we don't really care about
+    // whether or not authors specified quirky ems, since they're an implementation detail.
+    return false;
+}
+
 RenderBlock::MarginValues RenderBlock::marginValuesForChild(RenderBox* child) const
 {
     LayoutUnit childBeforePositive = 0;

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (144343 => 144344)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2013-02-28 19:59:23 UTC (rev 144344)
@@ -142,6 +142,15 @@
     void setHasMarkupTruncation(bool b) { m_hasMarkupTruncation = b; }
     bool hasMarkupTruncation() const { return m_hasMarkupTruncation; }
 
+    void setHasMarginBeforeQuirk(bool b) { m_hasMarginBeforeQuirk = b; }
+    void setHasMarginAfterQuirk(bool b) { m_hasMarginAfterQuirk = b; }
+
+    bool hasMarginBeforeQuirk() const { return m_hasMarginBeforeQuirk; }
+    bool hasMarginAfterQuirk() const { return m_hasMarginAfterQuirk; }
+
+    bool hasMarginBeforeQuirk(const RenderBox* child) const;
+    bool hasMarginAfterQuirk(const RenderBox* child) const;
+
     RootInlineBox* createAndAppendRootInlineBox();
 
     bool generatesLineBoxesForInlineChild(RenderObject*);
@@ -962,8 +971,8 @@
 
         // These variables are used to detect quirky margins that we need to collapse away (in table cells
         // and in the body element).
-        bool m_marginBeforeQuirk : 1;
-        bool m_marginAfterQuirk : 1;
+        bool m_hasMarginBeforeQuirk : 1;
+        bool m_hasMarginAfterQuirk : 1;
         bool m_determinedMarginBeforeQuirk : 1;
 
         bool m_discardMargin : 1;
@@ -982,8 +991,8 @@
             m_positiveMargin = 0;
             m_negativeMargin = 0;
         }
-        void setMarginBeforeQuirk(bool b) { m_marginBeforeQuirk = b; }
-        void setMarginAfterQuirk(bool b) { m_marginAfterQuirk = b; }
+        void setHasMarginBeforeQuirk(bool b) { m_hasMarginBeforeQuirk = b; }
+        void setHasMarginAfterQuirk(bool b) { m_hasMarginAfterQuirk = b; }
         void setDeterminedMarginBeforeQuirk(bool b) { m_determinedMarginBeforeQuirk = b; }
         void setPositiveMargin(LayoutUnit p) { ASSERT(!m_discardMargin); m_positiveMargin = p; }
         void setNegativeMargin(LayoutUnit n) { ASSERT(!m_discardMargin); m_negativeMargin = n; }
@@ -1011,8 +1020,8 @@
         bool canCollapseMarginAfterWithChildren() const { return m_canCollapseMarginAfterWithChildren; }
         bool quirkContainer() const { return m_quirkContainer; }
         bool determinedMarginBeforeQuirk() const { return m_determinedMarginBeforeQuirk; }
-        bool marginBeforeQuirk() const { return m_marginBeforeQuirk; }
-        bool marginAfterQuirk() const { return m_marginAfterQuirk; }
+        bool hasMarginBeforeQuirk() const { return m_hasMarginBeforeQuirk; }
+        bool hasMarginAfterQuirk() const { return m_hasMarginAfterQuirk; }
         LayoutUnit positiveMargin() const { return m_positiveMargin; }
         LayoutUnit negativeMargin() const { return m_negativeMargin; }
         bool discardMargin() const { return m_discardMargin; }
@@ -1235,7 +1244,9 @@
     RenderObjectChildList m_children;
     RenderLineBoxList m_lineBoxes;   // All of the root line boxes created for this block flow.  For example, <div>Hello<br>world.</div> will have two total lines for the <div>.
 
-    mutable signed m_lineHeight : 29;
+    mutable signed m_lineHeight : 27;
+    bool m_hasMarginBeforeQuirk : 1; // Note these quirk values can't be put in RenderBlockRareData since they are set too frequently.
+    bool m_hasMarginAfterQuirk : 1;
     unsigned m_beingDestroyed : 1;
     unsigned m_hasMarkupTruncation : 1;
     unsigned m_hasBorderOrPaddingLogicalWidthChanged : 1;

Modified: trunk/Source/WebCore/rendering/RenderObject.h (144343 => 144344)


--- trunk/Source/WebCore/rendering/RenderObject.h	2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/rendering/RenderObject.h	2013-02-28 19:59:23 UTC (rev 144344)
@@ -903,11 +903,6 @@
      */
     virtual LayoutRect localCaretRect(InlineBox*, int caretOffset, LayoutUnit* extraWidthToEndOfLine = 0);
 
-    bool isMarginBeforeQuirk() const { return m_bitfields.marginBeforeQuirk(); }
-    bool isMarginAfterQuirk() const { return m_bitfields.marginAfterQuirk(); }
-    void setMarginBeforeQuirk(bool b = true) { m_bitfields.setMarginBeforeQuirk(b); }
-    void setMarginAfterQuirk(bool b = true) { m_bitfields.setMarginAfterQuirk(b); }
-
     // When performing a global document tear-down, the renderer of the document is cleared.  We use this
     // as a hook to detect the case of document destruction and don't waste time doing unnecessary work.
     bool documentBeingDestroyed() const;
@@ -1073,15 +1068,13 @@
             , m_everHadLayout(false)
             , m_inRenderFlowThread(false)
             , m_childrenInline(false)
-            , m_marginBeforeQuirk(false) 
-            , m_marginAfterQuirk(false)
             , m_hasColumns(false)
             , m_positionedState(IsStaticlyPositioned)
             , m_selectionState(SelectionNone)
         {
         }
         
-        // 32 bits have been used here. THERE ARE NO FREE BITS AVAILABLE.
+        // 29 bits have been used here. There are three bits available.
         ADD_BOOLEAN_BITFIELD(needsLayout, NeedsLayout);
         ADD_BOOLEAN_BITFIELD(needsPositionedMovementLayout, NeedsPositionedMovementLayout);
         ADD_BOOLEAN_BITFIELD(normalChildNeedsLayout, NormalChildNeedsLayout);
@@ -1115,8 +1108,6 @@
 
         // from RenderBlock
         ADD_BOOLEAN_BITFIELD(childrenInline, ChildrenInline);
-        ADD_BOOLEAN_BITFIELD(marginBeforeQuirk, MarginBeforeQuirk);
-        ADD_BOOLEAN_BITFIELD(marginAfterQuirk, MarginAfterQuirk);
         ADD_BOOLEAN_BITFIELD(hasColumns, HasColumns);
 
     private:

Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (144343 => 144344)


--- trunk/Source/WebCore/rendering/style/RenderStyle.h	2013-02-28 19:55:23 UTC (rev 144343)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h	2013-02-28 19:59:23 UTC (rev 144344)
@@ -377,8 +377,8 @@
     bool hasBorder() const { return surround->border.hasBorder(); }
     bool hasPadding() const { return surround->padding.nonZero(); }
     bool hasOffset() const { return surround->offset.nonZero(); }
-    bool isMarginBeforeQuirk() const { return marginBefore().quirk(); }
-    bool isMarginAfterQuirk() const { return marginAfter().quirk(); }
+    bool hasMarginBeforeQuirk() const { return marginBefore().quirk(); }
+    bool hasMarginAfterQuirk() const { return marginAfter().quirk(); }
 
     bool hasBackgroundImage() const { return m_background->background().hasImage(); }
     bool hasFixedBackgroundImage() const { return m_background->background().hasFixedImage(); }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to