Changeset 224360 in webkit


Ignore:
Timestamp:
Nov 2, 2017 3:38:35 PM (6 years ago)
Author:
Antti Koivisto
Message:

display:contents should work with dynamic table mutations
https://bugs.webkit.org/show_bug.cgi?id=179179

Reviewed by Ryosuke Niwa.

Source/WebCore:

  • rendering/RenderBlock.cpp:

(WebCore::RenderBlock::addChildIgnoringContinuation):

RenderText with inline text wrapper as beforeChild is now resolved in RenderTreePosition, covering all cases.
Verify this with assert.

  • rendering/RenderElement.cpp:

(WebCore::RenderElement::insertChildInternal):

Add assertion.

  • rendering/RenderTableSection.cpp:

(WebCore::RenderTableSection::addChild):

Fix cases where we did unchecked downcasts for anonymous beforeChild.

  • style/RenderTreePosition.cpp:

(WebCore::RenderTreePosition::insert):

When inserting before a text rendeder with an display:contents inline wrapper, use the wrapper as beforeChild.

  • style/RenderTreePosition.h:

(WebCore::RenderTreePosition::insert): Deleted.

  • style/RenderTreeUpdater.cpp:

(WebCore::RenderTreeUpdater::updateRenderTree):
(WebCore::RenderTreeUpdater::renderingParent):

Add separate helper to get parent frame for the closest rendered (non display:contents) ancestor.

(WebCore::RenderTreeUpdater::renderTreePosition):
(WebCore::RenderTreeUpdater::updateElementRenderer):
(WebCore::RenderTreeUpdater::textRendererIsNeeded):
(WebCore::RenderTreeUpdater::updateTextRenderer):
(WebCore::RenderTreeUpdater::storePreviousRenderer):

Use it for tracking state related to render tree siblings. With this we compute whitespace nodes
correctly for display:contents. The test cases end up depending on that.

  • style/RenderTreeUpdater.h:

LayoutTests:

These now pass:

imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-none.html
imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-002-none.html

Location:
trunk
Files:
10 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r224357 r224360  
     12017-11-02  Antti Koivisto  <antti@apple.com>
     2
     3        display:contents should work with dynamic table mutations
     4        https://bugs.webkit.org/show_bug.cgi?id=179179
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * TestExpectations:
     9
     10        These now pass:
     11
     12        imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-none.html
     13        imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-002-none.html
     14
    1152017-11-02  Joseph Pecoraro  <pecoraro@apple.com>
    216
  • trunk/LayoutTests/TestExpectations

    r224352 r224360  
    13411341### START OF display: contents failures
    13421342
    1343 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-002-none.html [ ImageOnlyFailure ]
    13441343webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-flex-003.html [ ImageOnlyFailure ]
    13451344webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-inline.html [ ImageOnlyFailure ]
     
    13481347webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-flow-root-001.html [ ImageOnlyFailure ]
    13491348webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-003-none.html [ ImageOnlyFailure ]
    1350 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-none.html [ ImageOnlyFailure ]
    13511349webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-002-inline.html [ ImageOnlyFailure ]
    13521350webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-003-inline.html [ ImageOnlyFailure ]
  • trunk/Source/WebCore/ChangeLog

    r224359 r224360  
     12017-11-02  Antti Koivisto  <antti@apple.com>
     2
     3        display:contents should work with dynamic table mutations
     4        https://bugs.webkit.org/show_bug.cgi?id=179179
     5
     6        Reviewed by Ryosuke Niwa.
     7
     8        * rendering/RenderBlock.cpp:
     9        (WebCore::RenderBlock::addChildIgnoringContinuation):
     10
     11            RenderText with inline text wrapper as beforeChild is now resolved in RenderTreePosition, covering all cases.
     12            Verify this with assert.
     13
     14        * rendering/RenderElement.cpp:
     15        (WebCore::RenderElement::insertChildInternal):
     16
     17            Add assertion.
     18
     19        * rendering/RenderTableSection.cpp:
     20        (WebCore::RenderTableSection::addChild):
     21
     22            Fix cases where we did unchecked downcasts for anonymous beforeChild.
     23
     24        * style/RenderTreePosition.cpp:
     25        (WebCore::RenderTreePosition::insert):
     26
     27            When inserting before a text rendeder with an display:contents inline wrapper, use the wrapper as beforeChild.
     28
     29        * style/RenderTreePosition.h:
     30        (WebCore::RenderTreePosition::insert): Deleted.
     31        * style/RenderTreeUpdater.cpp:
     32        (WebCore::RenderTreeUpdater::updateRenderTree):
     33        (WebCore::RenderTreeUpdater::renderingParent):
     34
     35            Add separate helper to get parent frame for the closest rendered (non display:contents) ancestor.
     36
     37        (WebCore::RenderTreeUpdater::renderTreePosition):
     38        (WebCore::RenderTreeUpdater::updateElementRenderer):
     39        (WebCore::RenderTreeUpdater::textRendererIsNeeded):
     40        (WebCore::RenderTreeUpdater::updateTextRenderer):
     41        (WebCore::RenderTreeUpdater::storePreviousRenderer):
     42
     43            Use it for tracking state related to render tree siblings. With this we compute whitespace nodes
     44            correctly for display:contents. The test cases end up depending on that.
     45
     46        * style/RenderTreeUpdater.h:
     47
    1482017-11-02  Tim Horton  <timothy_horton@apple.com>
    249
  • trunk/Source/WebCore/rendering/RenderBlock.cpp

    r224327 r224360  
    551551
    552552        if (beforeChildContainer->isAnonymous()) {
    553             if (beforeChildContainer->isInline()) {
    554                 ASSERT(RenderText::findByDisplayContentsInlineWrapperCandidate(*beforeChildContainer) == beforeChild);
    555                 addChild(WTFMove(newChild), beforeChildContainer);
    556                 return;
    557             }
     553            RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(!beforeChildContainer->isInline());
     554
    558555            // If the requested beforeChild is not one of our children, then this is because
    559556            // there is an anonymous container within this object that contains the beforeChild.
     
    575572
    576573            ASSERT(beforeChildAnonymousContainer->isTable());
     574
    577575            if (newChild->isTablePart()) {
    578576                // Insert into the anonymous table.
     
    583581            beforeChild = splitAnonymousBoxesAroundChild(beforeChild);
    584582
    585             ASSERT(beforeChild->parent() == this);
    586             if (beforeChild->parent() != this) {
    587                 // We should never reach here. If we do, we need to use the
    588                 // safe fallback to use the topmost beforeChild container.
    589                 beforeChild = beforeChildContainer;
    590             }
     583            RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(beforeChild->parent() == this);
    591584        }
    592585    }
  • trunk/Source/WebCore/rendering/RenderElement.cpp

    r224359 r224360  
    539539
    540540    ASSERT(!beforeChild || beforeChild->parent() == this);
     541    ASSERT(!is<RenderText>(beforeChild) || !downcast<RenderText>(*beforeChild).inlineWrapperForDisplayContents());
    541542
    542543    // Take the ownership.
  • trunk/Source/WebCore/rendering/RenderTableSection.cpp

    r223728 r224360  
    123123        if (!last)
    124124            last = lastRow();
    125         if (last && last->isAnonymous() && !last->isBeforeOrAfterContent()) {
     125        if (is<RenderTableRow>(last) && last->isAnonymous() && !last->isBeforeOrAfterContent()) {
    126126            RenderTableRow& row = downcast<RenderTableRow>(*last);
    127127            if (beforeChild == &row)
     
    144144        while (lastBox && lastBox->parent()->isAnonymous() && !is<RenderTableRow>(*lastBox))
    145145            lastBox = lastBox->parent();
    146         if (lastBox && lastBox->isAnonymous() && !lastBox->isBeforeOrAfterContent()) {
     146        if (is<RenderTableRow>(lastBox) && lastBox->isAnonymous() && !lastBox->isBeforeOrAfterContent()) {
    147147            downcast<RenderTableRow>(*lastBox).addChild(WTFMove(child), beforeChild);
    148148            return;
  • trunk/Source/WebCore/style/RenderTreePosition.cpp

    r223848 r224360  
    2929#include "ComposedTreeIterator.h"
    3030#include "PseudoElement.h"
     31#include "RenderInline.h"
    3132#include "RenderObject.h"
    3233#include "ShadowRoot.h"
    3334
    3435namespace WebCore {
     36
     37void RenderTreePosition::insert(RenderPtr<RenderObject> renderer)
     38{
     39    ASSERT(m_hasValidNextSibling);
     40    auto* insertBefore = m_nextSibling;
     41    if (is<RenderText>(insertBefore)) {
     42        if (auto* wrapperInline = downcast<RenderText>(*insertBefore).inlineWrapperForDisplayContents())
     43            insertBefore = wrapperInline;
     44    }
     45    m_parent.addChild(WTFMove(renderer), insertBefore);
     46}
    3547
    3648void RenderTreePosition::computeNextSibling(const Node& node)
  • trunk/Source/WebCore/style/RenderTreePosition.h

    r223848 r224360  
    7979}
    8080
    81 inline void RenderTreePosition::insert(RenderPtr<RenderObject> renderer)
    82 {
    83     ASSERT(m_hasValidNextSibling);
    84     m_parent.addChild(WTFMove(renderer), m_nextSibling);
    85 }
    86 
    8781} // namespace WebCore
  • trunk/Source/WebCore/style/RenderTreeUpdater.cpp

    r224034 r224360  
    176176            auto* textUpdate = m_styleUpdate->textUpdate(text);
    177177            bool didCreateParent = parent().updates && parent().updates->update.change == Style::Detach;
    178             bool mayNeedUpdateWhitespaceOnlyRenderer = parent().didCreateOrDestroyChildRenderer && text.containsOnlyWhitespace();
     178            bool mayNeedUpdateWhitespaceOnlyRenderer = renderingParent().didCreateOrDestroyChildRenderer && text.containsOnlyWhitespace();
    179179            if (didCreateParent || textUpdate || mayNeedUpdateWhitespaceOnlyRenderer)
    180180                updateTextRenderer(text, textUpdate);
     
    216216}
    217217
     218auto RenderTreeUpdater::renderingParent() -> Parent&
     219{
     220    for (unsigned i = m_parentStack.size(); i--;) {
     221        if (m_parentStack[i].renderTreePosition)
     222            return m_parentStack[i];
     223    }
     224    ASSERT_NOT_REACHED();
     225    return m_parentStack.last();
     226}
     227
    218228RenderTreePosition& RenderTreeUpdater::renderTreePosition()
    219229{
    220     for (unsigned i = m_parentStack.size(); i; --i) {
    221         if (auto& position = m_parentStack[i - 1].renderTreePosition)
    222             return *position;
    223     }
    224     ASSERT_NOT_REACHED();
    225     return *m_parentStack.last().renderTreePosition;
     230    return *renderingParent().renderTreePosition;
    226231}
    227232
     
    315320        tearDownRenderers(element, teardownType);
    316321
    317         parent().didCreateOrDestroyChildRenderer = true;
     322        renderingParent().didCreateOrDestroyChildRenderer = true;
    318323    }
    319324
     
    330335        createRenderer(element, RenderStyle::clone(*update.style));
    331336
    332         parent().didCreateOrDestroyChildRenderer = true;
     337        renderingParent().didCreateOrDestroyChildRenderer = true;
    333338        return;
    334339    }
     
    411416        return true;
    412417
    413     auto* previousRenderer = parent().previousChildRenderer;
     418    auto* previousRenderer = renderingParent().previousChildRenderer;
    414419    if (previousRenderer && previousRenderer->isBR()) // <span><br/> <br/></span>
    415420        return false;
     
    484489        }
    485490        tearDownRenderer(text);
    486         parent().didCreateOrDestroyChildRenderer = true;
     491        renderingParent().didCreateOrDestroyChildRenderer = true;
    487492        return;
    488493    }
     
    490495        return;
    491496    createTextRenderer(text, renderTreePosition(), textUpdate);
    492     parent().didCreateOrDestroyChildRenderer = true;
     497    renderingParent().didCreateOrDestroyChildRenderer = true;
    493498}
    494499
     
    498503    if (!renderer)
    499504        return;
    500     ASSERT(parent().previousChildRenderer != renderer);
    501     parent().previousChildRenderer = renderer;
     505    ASSERT(renderingParent().previousChildRenderer != renderer);
     506    renderingParent().previousChildRenderer = renderer;
    502507}
    503508
  • trunk/Source/WebCore/style/RenderTreeUpdater.h

    r224034 r224360  
    7979    };
    8080    Parent& parent() { return m_parentStack.last(); }
     81    Parent& renderingParent();
    8182    RenderTreePosition& renderTreePosition();
    8283
Note: See TracChangeset for help on using the changeset viewer.