Attachment 341620 Details for Bug 186117 – Patch

[patch] Patch

bug-186117-20180530163902.patch (text/plain), 15.20 KB, created by zalan on 2018-05-30 16:39:03 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-05-30 16:39:03 PDT

Size: 15.20 KB

patch

obsolete

>Subversion Revision: 232291
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index e6b63ec7c0870b927d492af14f7c2677878e9eed..d11bca8bd696e8a819d98fbbb6d02ba693512682 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-05-30  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Layout code needs to know the type of the Element associated with a Layout::Box
>+        https://bugs.webkit.org/show_bug.cgi?id=186117
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Since these attributes don't change during layout, we could just pass them in to Layout::Box instead
>+        of keep querying the Element.
>+
>+        * layout/layouttree/LayoutBlockContainer.cpp:
>+        (WebCore::Layout::BlockContainer::BlockContainer):
>+        * layout/layouttree/LayoutBlockContainer.h:
>+        * layout/layouttree/LayoutBox.cpp:
>+        (WebCore::Layout::Box::Box):
>+        (WebCore::Layout::Box::isPaddingApplicable const):
>+        (WebCore::Layout::Box::isDocumentBox const): Deleted.
>+        (WebCore::Layout::Box::isBodyBox const): Deleted.
>+        * layout/layouttree/LayoutBox.h:
>+        (WebCore::Layout::Box::isAnonymous const):
>+        (WebCore::Layout::Box::isDocumentBox const):
>+        (WebCore::Layout::Box::isBodyBox const):
>+        (WebCore::Layout::Box::ElementAttributes::ElementAttributes):
>+        (WebCore::Layout::Box::setPreviousSibling):
>+        (WebCore::Layout::Box::setIsAnonymous): Deleted.
>+        * layout/layouttree/LayoutContainer.cpp:
>+        (WebCore::Layout::Container::Container):
>+        * layout/layouttree/LayoutContainer.h:
>+        * layout/layouttree/LayoutInlineBox.cpp:
>+        (WebCore::Layout::InlineBox::InlineBox):
>+        * layout/layouttree/LayoutInlineBox.h:
>+        * layout/layouttree/LayoutInlineContainer.cpp:
>+        (WebCore::Layout::InlineContainer::InlineContainer):
>+        * layout/layouttree/LayoutInlineContainer.h:
>+        * layout/layouttree/LayoutTreeBuilder.cpp:
>+        (WebCore::Layout::TreeBuilder::createLayoutTree):
>+        (WebCore::Layout::TreeBuilder::createSubTree):
>+
> 2018-05-30  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Miscellaneous fixes to get closer to geometry correctness
>diff --git a/Source/WebCore/layout/layouttree/LayoutBlockContainer.cpp b/Source/WebCore/layout/layouttree/LayoutBlockContainer.cpp
>index 7f43b3782e493dbf6f3cbefa5ccfd2aeee82c197..90c53492f76103b1801fc596031ccb2e2fc6165c 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBlockContainer.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutBlockContainer.cpp
>@@ -36,8 +36,8 @@ namespace Layout {
> 
> WTF_MAKE_ISO_ALLOCATED_IMPL(BlockContainer);
> 
>-BlockContainer::BlockContainer(RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>-    : Container(WTFMove(style), baseTypeFlags | BlockContainerFlag)
>+BlockContainer::BlockContainer(std::optional<ElementAttributes> attributes, RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>+    : Container(attributes, WTFMove(style), baseTypeFlags | BlockContainerFlag)
> {
> }
> 
>diff --git a/Source/WebCore/layout/layouttree/LayoutBlockContainer.h b/Source/WebCore/layout/layouttree/LayoutBlockContainer.h
>index dade7060eadd09b3d9e3628ada6f886fb60fd095..c844844ae6197af6ad87cdc10cc06ae3e199b89f 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBlockContainer.h
>+++ b/Source/WebCore/layout/layouttree/LayoutBlockContainer.h
>@@ -44,7 +44,7 @@ public:
>     bool establishesInlineFormattingContext() const final;
> 
> protected:
>-    BlockContainer(RenderStyle&&, BaseTypeFlags = BlockContainerFlag);
>+    BlockContainer(std::optional<ElementAttributes>, RenderStyle&&, BaseTypeFlags = BlockContainerFlag);
> 
> };
> 
>diff --git a/Source/WebCore/layout/layouttree/LayoutBox.cpp b/Source/WebCore/layout/layouttree/LayoutBox.cpp
>index 540318a0e324fa3c38f1899f1ed1569f8c88e314..defad2c509e3cc6a320c18ac1e91d2bc4261319c 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBox.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutBox.cpp
>@@ -36,10 +36,10 @@ namespace Layout {
> 
> WTF_MAKE_ISO_ALLOCATED_IMPL(Box);
> 
>-Box::Box(RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>+Box::Box(std::optional<ElementAttributes> attributes, RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>     : m_style(WTFMove(style))
>+    , m_elementAttributes(attributes)
>     , m_baseTypeFlags(baseTypeFlags)
>-    , m_isAnonymous(false)
> {
> }
> 
>@@ -181,18 +181,6 @@ bool Box::isInitialContainingBlock() const
>     return !parent();
> }
> 
>-bool Box::isDocumentBox() const
>-{
>-    // return document().documentElement() == &element();
>-    return false;
>-}
>-
>-bool Box::isBodyBox() const
>-{
>-    // return element().hasTagName(HTMLNames::bodyTag);
>-    return false;
>-}
>-
> const Box* Box::nextInFlowSibling() const
> {
>     if (auto* nextSibling = this->nextSibling()) {
>@@ -242,7 +230,15 @@ bool Box::isPaddingApplicable() const
> {
>     // 8.4 Padding properties:
>     // Applies to: all elements except table-row-group, table-header-group, table-footer-group, table-row, table-column-group and table-column
>-    return true;
>+    if (!m_elementAttributes)
>+        return false;
>+    auto elementType = m_elementAttributes.value().elementType;
>+    return elementType != ElementType::TableRowGroup
>+        && elementType != ElementType::TableHeaderGroup
>+        && elementType != ElementType::TableFooterGroup
>+        && elementType != ElementType::TableRow
>+        && elementType != ElementType::TableColumnGroup
>+        && elementType != ElementType::TableColumn;
> }
> 
> }
>diff --git a/Source/WebCore/layout/layouttree/LayoutBox.h b/Source/WebCore/layout/layouttree/LayoutBox.h
>index f98214c15334bb71aa3dc5979aeee932ab286a76..5a05f1911c7adab51597e056cd07b489a67dad99 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBox.h
>+++ b/Source/WebCore/layout/layouttree/LayoutBox.h
>@@ -66,7 +66,7 @@ public:
>     const Container& formattingContextRoot() const;
>     bool isDescendantOf(Container&) const;
> 
>-    bool isAnonymous() const { return m_isAnonymous; }
>+    bool isAnonymous() const { return !m_elementAttributes; }
> 
>     bool isBlockLevelBox() const;
>     bool isInlineLevelBox() const;
>@@ -74,8 +74,8 @@ public:
>     bool isBlockContainerBox() const;
>     bool isInitialContainingBlock() const;
> 
>-    bool isDocumentBox() const;
>-    bool isBodyBox() const;
>+    bool isDocumentBox() const { return m_elementAttributes && m_elementAttributes.value().elementType == ElementType::Document; }
>+    bool isBodyBox() const { return m_elementAttributes && m_elementAttributes.value().elementType == ElementType::Body; }
> 
>     const Container* parent() const { return m_parent; }
>     const Box* nextSibling() const { return m_nextSibling; }
>@@ -99,24 +99,45 @@ public:
>     std::optional<const Replaced> replaced() const { return m_replaced; }
> 
> protected:
>+    enum class ElementType {
>+        Document,
>+        Body,
>+        TableColumn,
>+        TableRow,
>+        TableColumnGroup,
>+        TableRowGroup,
>+        TableHeaderGroup,
>+        TableFooterGroup 
>+    };
>+
>+    struct ElementAttributes {
>+        ElementAttributes(ElementType elementType)
>+            : elementType(elementType)
>+            {
>+            }
>+
>+        ElementType elementType;
>+    };
>+
>     enum BaseTypeFlag {
>         ContainerFlag         = 1 << 0,
>         BlockContainerFlag    = 1 << 1,
>         InlineBoxFlag         = 1 << 2,
>         InlineContainerFlag   = 1 << 3
>     };
>-    Box(RenderStyle&&, BaseTypeFlags);
>+    Box(std::optional<ElementAttributes>, RenderStyle&&, BaseTypeFlags);
> 
>     bool isOverflowVisible() const;
> 
> private:
>+
>     void setParent(Container& parent) { m_parent = &parent; }
>     void setNextSibling(Box& nextSibling) { m_nextSibling = &nextSibling; }
>     void setPreviousSibling(Box& previousSibling) { m_previousSibling = &previousSibling; }
>-    void setIsAnonymous() { m_isAnonymous = true; }
> 
>     WeakPtrFactory<Box> m_weakFactory;
>     RenderStyle m_style;
>+    std::optional<ElementAttributes> m_elementAttributes;
> 
>     Container* m_parent { nullptr };
>     Box* m_previousSibling { nullptr };
>@@ -125,8 +146,6 @@ private:
>     std::optional<const Replaced> m_replaced;
> 
>     unsigned m_baseTypeFlags : 4;
>-    unsigned m_isAnonymous : 1;
>-
> };
> 
> }
>diff --git a/Source/WebCore/layout/layouttree/LayoutContainer.cpp b/Source/WebCore/layout/layouttree/LayoutContainer.cpp
>index 5363da3996aadf55d6ab606cddccfae49f69615c..9b62ee7daa4debe2dbbd902ce37e0359a24d475f 100644
>--- a/Source/WebCore/layout/layouttree/LayoutContainer.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutContainer.cpp
>@@ -36,8 +36,8 @@ namespace Layout {
> 
> WTF_MAKE_ISO_ALLOCATED_IMPL(Container);
> 
>-Container::Container(RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>-    : Box(WTFMove(style), baseTypeFlags | ContainerFlag)
>+Container::Container(std::optional<ElementAttributes> attributes, RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>+    : Box(attributes, WTFMove(style), baseTypeFlags | ContainerFlag)
> {
> }
> 
>diff --git a/Source/WebCore/layout/layouttree/LayoutContainer.h b/Source/WebCore/layout/layouttree/LayoutContainer.h
>index 6ba89ae73276ce1d96ebf70c8b6f8abb13480b3c..5cb7313a620fb6ca1ef1e54da6e6103d7cdc240a 100644
>--- a/Source/WebCore/layout/layouttree/LayoutContainer.h
>+++ b/Source/WebCore/layout/layouttree/LayoutContainer.h
>@@ -56,7 +56,7 @@ public:
>     const Vector<WeakPtr<Box>>& outOfFlowDescendants() { return m_outOfFlowDescendants; }
> 
> protected:
>-    Container(RenderStyle&&, BaseTypeFlags);
>+    Container(std::optional<ElementAttributes>, RenderStyle&&, BaseTypeFlags);
> 
> private:
>     void setFirstChild(Box&);
>diff --git a/Source/WebCore/layout/layouttree/LayoutInlineBox.cpp b/Source/WebCore/layout/layouttree/LayoutInlineBox.cpp
>index 8e5390badffd942348e81bba384cf3dd51b3e788..fb134f72e76b817cd14e9bfc115ffcc1c21bd588 100644
>--- a/Source/WebCore/layout/layouttree/LayoutInlineBox.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutInlineBox.cpp
>@@ -36,8 +36,8 @@ namespace Layout {
> 
> WTF_MAKE_ISO_ALLOCATED_IMPL(InlineBox);
> 
>-InlineBox::InlineBox(RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>-    : Box(WTFMove(style), baseTypeFlags | InlineBoxFlag)
>+InlineBox::InlineBox(std::optional<ElementAttributes> attributes, RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>+    : Box(attributes, WTFMove(style), baseTypeFlags | InlineBoxFlag)
> {
> }
> 
>diff --git a/Source/WebCore/layout/layouttree/LayoutInlineBox.h b/Source/WebCore/layout/layouttree/LayoutInlineBox.h
>index d06d096824e898ed30ad8e281dc6711bc1cc59e9..29f8a7d3c9a525530dcb7379097ee9e071856bf8 100644
>--- a/Source/WebCore/layout/layouttree/LayoutInlineBox.h
>+++ b/Source/WebCore/layout/layouttree/LayoutInlineBox.h
>@@ -39,7 +39,7 @@ namespace Layout {
> class InlineBox : public Box {
>     WTF_MAKE_ISO_ALLOCATED(InlineBox);
> public:
>-    InlineBox(RenderStyle&&, BaseTypeFlags);
>+    InlineBox(std::optional<ElementAttributes>, RenderStyle&&, BaseTypeFlags);
> };
> 
> }
>diff --git a/Source/WebCore/layout/layouttree/LayoutInlineContainer.cpp b/Source/WebCore/layout/layouttree/LayoutInlineContainer.cpp
>index 47151dbd722e30ed62bf0d65351f9d31ed9e4308..980d9a9f377b9025f974d9be06f22fb0c8bef98c 100644
>--- a/Source/WebCore/layout/layouttree/LayoutInlineContainer.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutInlineContainer.cpp
>@@ -36,8 +36,8 @@ namespace Layout {
> 
> WTF_MAKE_ISO_ALLOCATED_IMPL(InlineContainer);
> 
>-InlineContainer::InlineContainer(RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>-    : Container(WTFMove(style), baseTypeFlags | InlineContainerFlag)
>+InlineContainer::InlineContainer(std::optional<ElementAttributes> attributes, RenderStyle&& style, BaseTypeFlags baseTypeFlags)
>+    : Container(attributes, WTFMove(style), baseTypeFlags | InlineContainerFlag)
> {
> }
> 
>diff --git a/Source/WebCore/layout/layouttree/LayoutInlineContainer.h b/Source/WebCore/layout/layouttree/LayoutInlineContainer.h
>index aaa21d5aba906b5a958017ebba516406da6fa371..7373b36873c3c68a23aad0aaa804113e5d80481f 100644
>--- a/Source/WebCore/layout/layouttree/LayoutInlineContainer.h
>+++ b/Source/WebCore/layout/layouttree/LayoutInlineContainer.h
>@@ -42,7 +42,7 @@ public:
>     friend class TreeBuilder;
> 
> protected:
>-    InlineContainer(RenderStyle&&, BaseTypeFlags = InlineContainerFlag);
>+    InlineContainer(std::optional<ElementAttributes>, RenderStyle&&, BaseTypeFlags = InlineContainerFlag);
> };
> 
> }
>diff --git a/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp b/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp
>index ee67450a929c0c3a8b74d66fa234f13195e10ba5..6591821f4157c55a218eb6ce982ad2dddec920ed 100644
>--- a/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp
>@@ -29,6 +29,7 @@
> #if ENABLE(LAYOUT_FORMATTING_CONTEXT)
> 
> #include "LayoutBlockContainer.h"
>+#include "LayoutBox.h"
> #include "LayoutChildIterator.h"
> #include "LayoutContainer.h"
> #include "LayoutInlineBox.h"
>@@ -46,21 +47,44 @@ namespace Layout {
> 
> std::unique_ptr<Container> TreeBuilder::createLayoutTree(const RenderView& renderView)
> {
>-    std::unique_ptr<Container> initialContainingBlock(new BlockContainer(RenderStyle::clone(renderView.style())));
>+    std::unique_ptr<Container> initialContainingBlock(new BlockContainer(std::nullopt, RenderStyle::clone(renderView.style())));
>     TreeBuilder::createSubTree(renderView, *initialContainingBlock);
>     return initialContainingBlock;
> }
> 
> void TreeBuilder::createSubTree(const RenderElement& rootRenderer, Container& rootContainer)
> {
>+    auto elementAttributes = [] (const RenderElement& renderer) -> std::optional<Box::ElementAttributes> {
>+        if (renderer.isDocumentElementRenderer())
>+            return Box::ElementAttributes(Box::ElementType::Document);
>+        if (auto* element = renderer.element()) {
>+            if (element->hasTagName(HTMLNames::bodyTag))
>+                return Box::ElementAttributes(Box::ElementType::Body);
>+            if (element->hasTagName(HTMLNames::colTag))
>+                return Box::ElementAttributes(Box::ElementType::TableColumn);
>+            if (element->hasTagName(HTMLNames::trTag))
>+                return Box::ElementAttributes(Box::ElementType::TableRow);
>+            if (element->hasTagName(HTMLNames::colgroupTag))
>+                return Box::ElementAttributes(Box::ElementType::TableColumnGroup);
>+            if (element->hasTagName(HTMLNames::tbodyTag))
>+                return Box::ElementAttributes(Box::ElementType::TableRowGroup);
>+            if (element->hasTagName(HTMLNames::theadTag))
>+                return Box::ElementAttributes(Box::ElementType::TableHeaderGroup);
>+            if (element->hasTagName(HTMLNames::tfootTag))
>+                return Box::ElementAttributes(Box::ElementType::TableFooterGroup);
>+        } 
>+        return std::nullopt;
>+    };
>+
>     // Skip RenderText (and some others) for now.
>     for (auto& child : childrenOfType<RenderElement>(rootRenderer)) {
>         Box* box = nullptr;
>+
>         if (is<RenderBlock>(child)) {
>-            box = new BlockContainer(RenderStyle::clone(child.style()));
>+            box = new BlockContainer(elementAttributes(child), RenderStyle::clone(child.style()));
>             createSubTree(child, downcast<Container>(*box));
>         } else if (is<RenderInline>(child)) {
>-            box = new InlineContainer(RenderStyle::clone(child.style()));
>+            box = new InlineContainer(elementAttributes(child), RenderStyle::clone(child.style()));
>             createSubTree(child, downcast<Container>(*box));
>         } else
>             ASSERT_NOT_IMPLEMENTED_YET();

Actions: View | Formatted Diff | Diff

Attachments on bug 186117: 341620 | 341666