[webkit-changes] [253921] trunk/Source/WebCore

Fri, 27 Dec 2019 07:23:54 -0800

Title: [253921] trunk/Source/WebCore
Revision
253921
Author
[email protected]
Date
2019-12-27 07:22:39 -0800 (Fri, 27 Dec 2019)

Log Message

[LFC][Integration] Ensure layout boxes have expected display types
https://bugs.webkit.org/show_bug.cgi?id=205606

Reviewed by Zalan Bujtas.

In some cases render tree may have display property values that don't match the renderer type. This is fine since the behavior is driven by the renderer.

LFC layout is driven by display property so the effective value needs to make sense. This patch fixes assertions seen in

fast/css/fieldset-display-row.html
fast/css-grid-layout/grid-strict-ordering-crash-2.html
imported/w3c/web-platform-tests/css/css-display/display-flow-root-001.html
imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-display.html
tables/mozilla/bugs/bug275625.html

* layout/integration/LayoutIntegrationLineLayout.cpp:
(WebCore::LayoutIntegration::LineLayout::LineLayout):
* layout/layouttree/LayoutTreeBuilder.cpp:
(WebCore::Layout::TreeBuilder::buildLayoutTreeForIntegration):

Always set display to 'block' for the root RenderBlockFlow.
Renamed for clarity.

(WebCore::Layout::TreeBuilder::createLayoutBox):

Always set <br> display to inline.

(WebCore::Layout::TreeBuilder::buildTableStructure):
(WebCore::Layout::TreeBuilder::buildSubTree):

Pass the parent container instead of parent renderer so we can read effective style.

* layout/layouttree/LayoutTreeBuilder.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (253920 => 253921)


--- trunk/Source/WebCore/ChangeLog	2019-12-27 14:13:25 UTC (rev 253920)
+++ trunk/Source/WebCore/ChangeLog	2019-12-27 15:22:39 UTC (rev 253921)
@@ -1,3 +1,39 @@
+2019-12-27  Antti Koivisto  <[email protected]>
+
+        [LFC][Integration] Ensure layout boxes have expected display types
+        https://bugs.webkit.org/show_bug.cgi?id=205606
+
+        Reviewed by Zalan Bujtas.
+
+        In some cases render tree may have display property values that don't match the renderer type. This is fine since the behavior is driven by the renderer.
+
+        LFC layout is driven by display property so the effective value needs to make sense. This patch fixes assertions seen in
+
+        fast/css/fieldset-display-row.html
+        fast/css-grid-layout/grid-strict-ordering-crash-2.html
+        imported/w3c/web-platform-tests/css/css-display/display-flow-root-001.html
+        imported/w3c/web-platform-tests/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-display.html
+        tables/mozilla/bugs/bug275625.html
+
+        * layout/integration/LayoutIntegrationLineLayout.cpp:
+        (WebCore::LayoutIntegration::LineLayout::LineLayout):
+        * layout/layouttree/LayoutTreeBuilder.cpp:
+        (WebCore::Layout::TreeBuilder::buildLayoutTreeForIntegration):
+
+        Always set display to 'block' for the root RenderBlockFlow.
+        Renamed for clarity.
+
+        (WebCore::Layout::TreeBuilder::createLayoutBox):
+
+        Always set <br> display to inline.
+
+        (WebCore::Layout::TreeBuilder::buildTableStructure):
+        (WebCore::Layout::TreeBuilder::buildSubTree):
+
+        Pass the parent container instead of parent renderer so we can read effective style.
+
+        * layout/layouttree/LayoutTreeBuilder.h:
+
 2019-12-27  Zalan Bujtas  <[email protected]>
 
         [LFC][IFC] Fix LayoutTests/fast/backgrounds/size/backgroundSize15.html

Modified: trunk/Source/WebCore/layout/integration/LayoutIntegrationLineLayout.cpp (253920 => 253921)


--- trunk/Source/WebCore/layout/integration/LayoutIntegrationLineLayout.cpp	2019-12-27 14:13:25 UTC (rev 253920)
+++ trunk/Source/WebCore/layout/integration/LayoutIntegrationLineLayout.cpp	2019-12-27 15:22:39 UTC (rev 253921)
@@ -53,7 +53,7 @@
 LineLayout::LineLayout(const RenderBlockFlow& flow)
     : m_flow(flow)
 {
-    m_treeContent = Layout::TreeBuilder::buildLayoutTree(flow);
+    m_treeContent = Layout::TreeBuilder::buildLayoutTreeForIntegration(flow);
 }
 
 LineLayout::~LineLayout() = default;

Modified: trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp (253920 => 253921)


--- trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp	2019-12-27 14:13:25 UTC (rev 253920)
+++ trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp	2019-12-27 15:22:39 UTC (rev 253921)
@@ -31,6 +31,7 @@
 #include "CachedImage.h"
 #include "DisplayBox.h"
 #include "DisplayRun.h"
+#include "HTMLNames.h"
 #include "HTMLTableCellElement.h"
 #include "HTMLTableColElement.h"
 #include "InlineFormattingState.h"
@@ -76,7 +77,6 @@
     m_layoutBoxToRenderObject.add(&layoutBox, &renderer);
 }
 
-
 static void appendChild(Container& parent, Box& newChild)
 {
     if (!parent.hasChild()) {
@@ -148,12 +148,14 @@
     return layoutTreeContent;
 }
 
-std::unique_ptr<Layout::LayoutTreeContent> TreeBuilder::buildLayoutTree(const RenderBlockFlow& renderBlockFlow)
+std::unique_ptr<Layout::LayoutTreeContent> TreeBuilder::buildLayoutTreeForIntegration(const RenderBlockFlow& renderBlockFlow)
 {
     PhaseScope scope(Phase::Type::TreeBuilding);
 
-    auto style = RenderStyle::clone(renderBlockFlow.style());
-    auto layoutTreeContent = makeUnique<LayoutTreeContent>(renderBlockFlow, makeUnique<Container>(WTF::nullopt, WTFMove(style)));
+    auto clonedStyle = RenderStyle::clone(renderBlockFlow.style());
+    clonedStyle.setDisplay(DisplayType::Block);
+
+    auto layoutTreeContent = makeUnique<LayoutTreeContent>(renderBlockFlow, makeUnique<Container>(WTF::nullopt, WTFMove(clonedStyle)));
     TreeBuilder(*layoutTreeContent).buildTree();
     return layoutTreeContent;
 }
@@ -168,7 +170,7 @@
     buildSubTree(m_layoutTreeContent.rootRenderer(), m_layoutTreeContent.rootLayoutBox());
 }
 
-std::unique_ptr<Box> TreeBuilder::createLayoutBox(const RenderElement& parentRenderer, const RenderObject& childRenderer)
+std::unique_ptr<Box> TreeBuilder::createLayoutBox(const Container& parentContainer, const RenderObject& childRenderer)
 {
     auto elementAttributes = [] (const RenderElement& renderer) -> Optional<Box::ElementAttributes> {
         if (renderer.isDocumentElementRenderer())
@@ -191,27 +193,31 @@
     std::unique_ptr<Box> childLayoutBox;
     if (is<RenderText>(childRenderer)) {
         auto& textRenderer = downcast<RenderText>(childRenderer);
-        auto text = applyTextTransform(textRenderer.text(), parentRenderer.style());
+        auto text = applyTextTransform(textRenderer.text(), parentContainer.style());
         // FIXME: Clearly there must be a helper function for this.
-        auto textContent = TextContext { text, canUseSimplifiedTextMeasuring(text, parentRenderer.style().fontCascade(), parentRenderer.style().collapseWhiteSpace()) };
-        if (parentRenderer.style().display() == DisplayType::Inline)
-            childLayoutBox = makeUnique<Box>(WTFMove(textContent), RenderStyle::clone(parentRenderer.style()));
+        auto textContent = TextContext { text, canUseSimplifiedTextMeasuring(text, parentContainer.style().fontCascade(), parentContainer.style().collapseWhiteSpace()) };
+        if (parentContainer.style().display() == DisplayType::Inline)
+            childLayoutBox = makeUnique<Box>(WTFMove(textContent), RenderStyle::clone(parentContainer.style()));
         else
-            childLayoutBox = makeUnique<Box>(WTFMove(textContent), RenderStyle::createAnonymousStyleWithDisplay(parentRenderer.style(), DisplayType::Inline));
+            childLayoutBox = makeUnique<Box>(WTFMove(textContent), RenderStyle::createAnonymousStyleWithDisplay(parentContainer.style(), DisplayType::Inline));
     } else {
         auto& renderer = downcast<RenderElement>(childRenderer);
         auto displayType = renderer.style().display();
-        if (is<RenderLineBreak>(renderer))
-            childLayoutBox = makeUnique<Box>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
-        else if (is<RenderTable>(renderer)) {
+
+        auto clonedStyle = RenderStyle::clone(renderer.style());
+
+        if (is<RenderLineBreak>(renderer)) {
+            clonedStyle.setDisplay(DisplayType::Inline);
+            childLayoutBox = makeUnique<Box>(elementAttributes(renderer), WTFMove(clonedStyle));
+        } else if (is<RenderTable>(renderer)) {
             // Construct the principal table wrapper box (and not the table box itself).
-            childLayoutBox = makeUnique<Container>(Box::ElementAttributes { Box::ElementType::TableWrapperBox }, RenderStyle::clone(renderer.style()));
+            childLayoutBox = makeUnique<Container>(Box::ElementAttributes { Box::ElementType::TableWrapperBox }, WTFMove(clonedStyle));
             childLayoutBox->setIsAnonymous();
         } else if (is<RenderReplaced>(renderer)) {
             if (displayType == DisplayType::Block)
-                childLayoutBox = makeUnique<Box>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                childLayoutBox = makeUnique<Box>(elementAttributes(renderer), WTFMove(clonedStyle));
             else
-                childLayoutBox = makeUnique<Box>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                childLayoutBox = makeUnique<Box>(elementAttributes(renderer), WTFMove(clonedStyle));
             // FIXME: We don't yet support all replaced elements and this is temporary anyway.
             if (childLayoutBox->replaced())
                 childLayoutBox->replaced()->setIntrinsicSize(downcast<RenderReplaced>(renderer).intrinsicSize());
@@ -225,23 +231,22 @@
         } else {
             if (displayType == DisplayType::Block) {
                 if (auto offset = accumulatedOffsetForInFlowPositionedContinuation(downcast<RenderBox>(renderer))) {
-                    auto style = RenderStyle::clonePtr(renderer.style());
-                    style->setTop({ offset->height(), Fixed });
-                    style->setLeft({ offset->width(), Fixed });
-                    childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(*style));
+                    clonedStyle.setTop({ offset->height(), Fixed });
+                    clonedStyle.setLeft({ offset->width(), Fixed });
+                    childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(clonedStyle));
                 } else
-                    childLayoutBox = makeUnique<Container>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                    childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(clonedStyle));
             } else if (displayType == DisplayType::Inline)
-                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(clonedStyle));
             else if (displayType == DisplayType::InlineBlock)
-                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(clonedStyle));
             else if (displayType == DisplayType::TableCaption || displayType == DisplayType::TableCell) {
-                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(clonedStyle));
             } else if (displayType == DisplayType::TableRowGroup || displayType == DisplayType::TableHeaderGroup || displayType == DisplayType::TableFooterGroup
                 || displayType == DisplayType::TableRow || displayType == DisplayType::TableColumnGroup) {
-                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(clonedStyle));
             } else if (displayType == DisplayType::TableColumn) {
-                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), RenderStyle::clone(renderer.style()));
+                childLayoutBox = makeUnique<Container>(elementAttributes(renderer), WTFMove(clonedStyle));
                 auto& tableColElement = static_cast<HTMLTableColElement&>(*renderer.element());
                 auto columnWidth = tableColElement.width();
                 if (!columnWidth.isEmpty())
@@ -278,7 +283,7 @@
     auto* tableChild = tableRenderer.firstChild();
     while (is<RenderTableCaption>(tableChild)) {
         auto& captionRenderer = *tableChild;
-        auto captionBox = createLayoutBox(tableRenderer, captionRenderer);
+        auto captionBox = createLayoutBox(tableWrapperBox, captionRenderer);
         appendChild(tableWrapperBox, *captionBox);
         auto& captionContainer = downcast<Container>(*captionBox);
         buildSubTree(downcast<RenderElement>(captionRenderer), captionContainer);
@@ -290,7 +295,7 @@
     appendChild(tableWrapperBox, *tableBox);
     auto* sectionRenderer = tableChild;
     while (sectionRenderer) {
-        auto sectionBox = createLayoutBox(tableRenderer, *sectionRenderer);
+        auto sectionBox = createLayoutBox(*tableBox, *sectionRenderer);
         appendChild(*tableBox, *sectionBox);
         auto& sectionContainer = downcast<Container>(*sectionBox);
         buildSubTree(downcast<RenderElement>(*sectionRenderer), sectionContainer);
@@ -300,11 +305,11 @@
     m_layoutTreeContent.addBox(WTFMove(tableBox));
 }
 
-void TreeBuilder::buildSubTree(const RenderElement& rootRenderer, Container& rootContainer)
+void TreeBuilder::buildSubTree(const RenderElement& parentRenderer, Container& parentContainer)
 {
-    for (auto& childRenderer : childrenOfType<RenderObject>(rootRenderer)) {
-        auto childLayoutBox = createLayoutBox(rootRenderer, childRenderer);
-        appendChild(rootContainer, *childLayoutBox);
+    for (auto& childRenderer : childrenOfType<RenderObject>(parentRenderer)) {
+        auto childLayoutBox = createLayoutBox(parentContainer, childRenderer);
+        appendChild(parentContainer, *childLayoutBox);
         if (childLayoutBox->isTableWrapperBox())
             buildTableStructure(downcast<RenderTable>(childRenderer), downcast<Container>(*childLayoutBox));
         else if (is<Container>(*childLayoutBox))

Modified: trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.h (253920 => 253921)


--- trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.h	2019-12-27 14:13:25 UTC (rev 253920)
+++ trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.h	2019-12-27 15:22:39 UTC (rev 253921)
@@ -76,15 +76,15 @@
 class TreeBuilder {
 public:
     static std::unique_ptr<Layout::LayoutTreeContent> buildLayoutTree(const RenderView&);
-    static std::unique_ptr<Layout::LayoutTreeContent> buildLayoutTree(const RenderBlockFlow&);
+    static std::unique_ptr<Layout::LayoutTreeContent> buildLayoutTreeForIntegration(const RenderBlockFlow&);
 
 private:
     TreeBuilder(LayoutTreeContent&);
 
     void buildTree();
-    void buildSubTree(const RenderElement& rootRenderer, Container& rootContainer);
+    void buildSubTree(const RenderElement& parentRenderer, Container& parentContainer);
     void buildTableStructure(const RenderTable& tableRenderer, Container& tableWrapperBox);
-    std::unique_ptr<Box> createLayoutBox(const RenderElement& parentRenderer, const RenderObject& childRenderer);
+    std::unique_ptr<Box> createLayoutBox(const Container& parentContainer, const RenderObject& childRenderer);
 
     LayoutTreeContent& m_layoutTreeContent;
 };

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to