Changeset 288267 in webkit


Ignore:
Timestamp:
Jan 19, 2022 7:53:27 PM (6 months ago)
Author:
Simon Fraser
Message:

<dialog> with transformed ancestor asserts under RenderGeometryMap
​https://bugs.webkit.org/show_bug.cgi?id=235194

Reviewed by Antti Koivisto.

Source/WebCore:

With this change we reparent the RenderLayers of top layer elements under the RenderView's
layer. This makes the RenderLayer hierarchy a closer match to the containing block
hierarchy, and means that all the existing RenderLayer tree walks that use
parent()/firstChild()/nextSibling() traverse the the "top layer" layers as children of the
RenderView. This in turn means that the various bits of RenderLayer state that track the
state of descendants (e.g. m_hasVisibleDescendant, m_hasSelfPaintingLayerDescendant,
m_hasNotIsolatedBlendingDescendants) reflect descendency in the top-layer-aware hierarchy.

Note that m_hasVisibleDescendant is about the inherited visibility property which follows
DOM order, but since we consult it during painting-related tree walks, we want this state to
reflect the top-layer-aware tree.

The patch adds top-layer-aware helpers on RenderElement to find the parent and next sibling,
and uses those when parenting layers.

In addition, when the top layer status changes for a RenderLayer, we unparent and
re-parent its layer (which in turn should toggle the relevant dirty bits).

  • rendering/RenderElement.cpp:

(WebCore::findNextLayer):
(WebCore::layerNextSiblingRespectingTopLayer):
(WebCore::addLayers):
(WebCore::RenderElement::layerParentRespectingTopLayer const):
(WebCore::RenderElement::layerNextSiblingRespectingTopLayer const):
(WebCore::RenderElement::insertedIntoTree):
(WebCore::RenderElement::willBeRemovedFromTree):
(WebCore::RenderElement::findNextLayer const): Deleted.

  • rendering/RenderElement.h:
  • rendering/RenderLayer.cpp:

(WebCore::RenderLayer::insertOnlyThisLayer):
(WebCore::RenderLayer::stackingContext const): No need for the explicit establishesTopLayer() check.
(WebCore::RenderLayer::setHasVisibleContent):
(WebCore::RenderLayer::dirtyAncestorChainVisibleDescendantStatus):
(WebCore::RenderLayer::setAncestorChainHasVisibleDescendant):
(WebCore::RenderLayer::enclosingAncestorForPosition const): No need for the explicit establishesTopLayer() check.
(WebCore::RenderLayer::paintLayerWithEffects): Ditto
(WebCore::RenderLayer::establishesTopLayerWillChange):
(WebCore::RenderLayer::establishesTopLayerDidChange):
(WebCore::RenderLayer::clipCrossesPaintingBoundary const): No need for the explicit establishesTopLayer() check.
(WebCore::RenderLayer::calculateClipRects const): Ditto

LayoutTests:

  • TestExpectations: imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/top-layer-parent-transform.html

no longer asserts.

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r288266 r288267  
     12022-01-19  Simon Fraser  <simon.fraser@apple.com>
     2
     3        <dialog> with transformed ancestor asserts under RenderGeometryMap
     4        https://bugs.webkit.org/show_bug.cgi?id=235194
     5
     6        Reviewed by Antti Koivisto.
     7
     8        * TestExpectations: imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/top-layer-parent-transform.html
     9        no longer asserts.
     10
    1112022-01-19  Devin Rousso  <drousso@apple.com>
    212

  • trunk/LayoutTests/TestExpectations

    r288132 r288267  
    23692369webkit.org/b/229315 imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/modal-dialog-in-replaced-renderer.html [ ImageOnlyFailure ]
    23702370webkit.org/b/229315 imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/modal-dialog-in-table-column.html [ ImageOnlyFailure ]
    2371 # Needs layer re-parenting
    2372 webkit.org/b/234980 [ Debug ] imported/w3c/web-platform-tests/html/semantics/interactive-elements/the-dialog-element/top-layer-parent-transform.html [ Crash ]
    23732371
    23742372# Assertion failure in MessagePort::contextDestroyed, usually attributed to later tests

  • trunk/Source/WebCore/ChangeLog

    r288264 r288267  
     12022-01-19  Simon Fraser  <simon.fraser@apple.com>
     2
     3        <dialog> with transformed ancestor asserts under RenderGeometryMap
     4        https://bugs.webkit.org/show_bug.cgi?id=235194
     5
     6        Reviewed by Antti Koivisto.
     7
     8        With this change we reparent the RenderLayers of top layer elements under the RenderView's
     9        layer. This makes the RenderLayer hierarchy a closer match to the containing block
     10        hierarchy, and means that all the existing RenderLayer tree walks that use
     11        parent()/firstChild()/nextSibling() traverse the the "top layer" layers as children of the
     12        RenderView. This in turn means that the various bits of RenderLayer state that track the
     13        state of descendants (e.g. m_hasVisibleDescendant, m_hasSelfPaintingLayerDescendant,
     14        m_hasNotIsolatedBlendingDescendants) reflect descendency in the top-layer-aware hierarchy.
     15
     16        Note that m_hasVisibleDescendant is about the inherited `visibility` property which follows
     17        DOM order, but since we consult it during painting-related tree walks, we want this state to
     18        reflect the top-layer-aware tree.
     19       
     20        The patch adds top-layer-aware helpers on RenderElement to find the parent and next sibling,
     21        and uses those when parenting layers.
     22       
     23        In addition, when the top layer status changes for a RenderLayer, we unparent and
     24        re-parent its layer (which in turn should toggle the relevant dirty bits).
     25
     26        * rendering/RenderElement.cpp:
     27        (WebCore::findNextLayer):
     28        (WebCore::layerNextSiblingRespectingTopLayer):
     29        (WebCore::addLayers):
     30        (WebCore::RenderElement::layerParentRespectingTopLayer const):
     31        (WebCore::RenderElement::layerNextSiblingRespectingTopLayer const):
     32        (WebCore::RenderElement::insertedIntoTree):
     33        (WebCore::RenderElement::willBeRemovedFromTree):
     34        (WebCore::RenderElement::findNextLayer const): Deleted.
     35        * rendering/RenderElement.h:
     36        * rendering/RenderLayer.cpp:
     37        (WebCore::RenderLayer::insertOnlyThisLayer):
     38        (WebCore::RenderLayer::stackingContext const): No need for the explicit establishesTopLayer() check.
     39        (WebCore::RenderLayer::setHasVisibleContent):
     40        (WebCore::RenderLayer::dirtyAncestorChainVisibleDescendantStatus):
     41        (WebCore::RenderLayer::setAncestorChainHasVisibleDescendant):
     42        (WebCore::RenderLayer::enclosingAncestorForPosition const): No need for the explicit establishesTopLayer() check.
     43        (WebCore::RenderLayer::paintLayerWithEffects): Ditto
     44        (WebCore::RenderLayer::establishesTopLayerWillChange):
     45        (WebCore::RenderLayer::establishesTopLayerDidChange):
     46        (WebCore::RenderLayer::clipCrossesPaintingBoundary const): No need for the explicit establishesTopLayer() check.
     47        (WebCore::RenderLayer::calculateClipRects const): Ditto
     48
    1492022-01-19  Chris Dumez  <cdumez@apple.com>
    250

  • trunk/Source/WebCore/rendering/RenderElement.cpp

    r288127 r288267  
    635635}
    636636
     637static RenderLayer* findNextLayer(const RenderElement& currRenderer, RenderLayer& parentLayer, const RenderObject* siblingToTraverseFrom, bool checkParent = true)
     638{
     639    // Step 1: If our layer is a child of the desired parent, then return our layer.
     640    auto* ourLayer = currRenderer.hasLayer() ? downcast<RenderLayerModelObject>(currRenderer).layer() : nullptr;
     641    if (ourLayer && ourLayer->parent() == &parentLayer)
     642        return ourLayer;
     643
     644    // Step 2: If we don't have a layer, or our layer is the desired parent, then descend
     645    // into our siblings trying to find the next layer whose parent is the desired parent.
     646    if (!ourLayer || ourLayer == &parentLayer) {
     647        for (auto* child = siblingToTraverseFrom ? siblingToTraverseFrom->nextSibling() : currRenderer.firstChild(); child; child = child->nextSibling()) {
     648            if (!is<RenderElement>(*child))
     649                continue;
     650            if (auto* nextLayer = findNextLayer(downcast<RenderElement>(*child), parentLayer, nullptr, false))
     651                return nextLayer;
     652        }
     653    }
     654
     655    // Step 3: If our layer is the desired parent layer, then we're finished. We didn't
     656    // find anything.
     657    if (ourLayer == &parentLayer)
     658        return nullptr;
     659
     660    // Step 4: If |checkParent| is set, climb up to our parent and check its siblings that
     661    // follow us to see if we can locate a layer.
     662    if (checkParent && currRenderer.parent())
     663        return findNextLayer(*currRenderer.parent(), parentLayer, &currRenderer, true);
     664
     665    return nullptr;
     666}
     667
     668static RenderLayer* layerNextSiblingRespectingTopLayer(const RenderElement& renderer, RenderLayer& parentLayer)
     669{
     670    ASSERT_IMPLIES(isInTopLayerOrBackdrop(renderer.style(), renderer.element()), renderer.hasLayer());
     671
     672    if (is<RenderLayerModelObject>(renderer) && isInTopLayerOrBackdrop(renderer.style(), renderer.element())) {
     673        auto& layerModelObject = downcast<RenderLayerModelObject>(renderer);
     674        ASSERT(layerModelObject.hasLayer());
     675        auto topLayerLayers = RenderLayer::topLayerRenderLayers(renderer.view());
     676        auto layerIndex = topLayerLayers.find(layerModelObject.layer());
     677        if (layerIndex != notFound && layerIndex < topLayerLayers.size() - 1)
     678            return topLayerLayers[layerIndex + 1];
     679
     680        return nullptr;
     681    }
     682
     683    return findNextLayer(*renderer.parent(), parentLayer, &renderer);
     684}
     685
    637686static void addLayers(const RenderElement& addedRenderer, RenderElement& currentRenderer, RenderLayer& parentLayer, std::optional<RenderLayer*>& beforeChild)
    638687{
    639688    if (currentRenderer.hasLayer()) {
    640689        if (!beforeChild.has_value())
    641             beforeChild = addedRenderer.parent()->findNextLayer(parentLayer, &addedRenderer);
     690            beforeChild = layerNextSiblingRespectingTopLayer(addedRenderer, parentLayer);
    642691
    643692        parentLayer.addChild(*downcast<RenderLayerModelObject>(currentRenderer).layer(), beforeChild.value());
    … …  
    687736}
    688737
    689 RenderLayer* RenderElement::findNextLayer(RenderLayer& parentLayer, const RenderObject* siblingToTraverseFrom, bool checkParent) const
    690 {
    691     // Step 1: If our layer is a child of the desired parent, then return our layer.
    692     auto* ourLayer = hasLayer() ? downcast<RenderLayerModelObject>(*this).layer() : nullptr;
    693     if (ourLayer && ourLayer->parent() == &parentLayer)
    694         return ourLayer;
    695 
    696     // Step 2: If we don't have a layer, or our layer is the desired parent, then descend
    697     // into our siblings trying to find the next layer whose parent is the desired parent.
    698     if (!ourLayer || ourLayer == &parentLayer) {
    699         for (auto* child = siblingToTraverseFrom ? siblingToTraverseFrom->nextSibling() : firstChild(); child; child = child->nextSibling()) {
    700             if (!is<RenderElement>(*child))
    701                 continue;
    702             if (auto* nextLayer = downcast<RenderElement>(*child).findNextLayer(parentLayer, nullptr, false))
    703                 return nextLayer;
    704         }
    705     }
    706 
    707     // Step 3: If our layer is the desired parent layer, then we're finished. We didn't
    708     // find anything.
    709     if (ourLayer == &parentLayer)
    710         return nullptr;
    711 
    712     // Step 4: If |checkParent| is set, climb up to our parent and check its siblings that
    713     // follow us to see if we can locate a layer.
    714     if (checkParent && parent())
    715         return parent()->findNextLayer(parentLayer, this, true);
    716 
    717     return nullptr;
     738RenderLayer* RenderElement::layerParent() const
     739{
     740    ASSERT_IMPLIES(isInTopLayerOrBackdrop(style(), element()), hasLayer());
     741
     742    if (hasLayer() && isInTopLayerOrBackdrop(style(), element()))
     743        return view().layer();
     744
     745    return parent()->enclosingLayer();
     746}
     747
     748// This answers the question "if this renderer had a layer, what would its next sibling layer be".
     749RenderLayer* RenderElement::layerNextSibling(RenderLayer& parentLayer) const
     750{
     751    return WebCore::layerNextSiblingRespectingTopLayer(*this, parentLayer);
    718752}
    719753
    … …  
    962996    // Keep our layer hierarchy updated. Optimize for the common case where we don't have any children
    963997    // and don't have a layer attached to ourselves.
    964     RenderLayer* layer = nullptr;
     998    RenderLayer* parentLayer = nullptr;
    965999    if (firstChild() || hasLayer()) {
    966         layer = parent()->enclosingLayer();
    967         addLayers(layer);
     1000        auto* parentLayer = layerParent();
     1001        addLayers(parentLayer);
    9681002    }
    9691003
    … …  
    9711005    // that needs to be drawn and layer visibility optimization can't be used
    9721006    if (parent()->style().visibility() != Visibility::Visible && style().visibility() == Visibility::Visible && !hasLayer()) {
    973         if (!layer)
    974             layer = parent()->enclosingLayer();
    975         if (layer)
    976             layer->dirtyVisibleContentStatus();
     1007        if (!parentLayer)
     1008            parentLayer = layerParent();
     1009        if (parentLayer)
     1010            parentLayer->dirtyVisibleContentStatus();
    9771011    }
    9781012
    … …  
    9831017{
    9841018    // If we remove a visible child from an invisible parent, we don't know the layer visibility any more.
    985     RenderLayer* layer = nullptr;
    9861019    if (parent()->style().visibility() != Visibility::Visible && style().visibility() == Visibility::Visible && !hasLayer()) {
    987         if ((layer = parent()->enclosingLayer()))
    988             layer->dirtyVisibleContentStatus();
     1020        // FIXME: should get parent layer. Necessary?
     1021        if (auto* enclosingLayer = parent()->enclosingLayer())
     1022            enclosingLayer->dirtyVisibleContentStatus();
    9891023    }
    9901024    // Keep our layer hierarchy updated.
    9911025    if (firstChild() || hasLayer()) {
    992         if (!layer)
    993             layer = parent()->enclosingLayer();
    994         removeLayers(layer);
     1026        auto* parentLayer = layerParent();
     1027        removeLayers(parentLayer);
    9951028    }
    9961029

  • trunk/Source/WebCore/rendering/RenderElement.h

    r288127 r288267  
    109109    // properly added and removed. Since containership can be implemented by any subclass, and since a hierarchy
    110110    // can contain a mixture of boxes and other object types, these functions need to be in the base class.
     111    RenderLayer* layerParent() const;
     112    RenderLayer* layerNextSibling(RenderLayer& parentLayer) const;
    111113    void addLayers(RenderLayer* parentLayer);
    112114    void removeLayers(RenderLayer* parentLayer);
    113115    void moveLayers(RenderLayer* oldParent, RenderLayer& newParent);
    114     RenderLayer* findNextLayer(RenderLayer& parentLayer, const RenderObject* siblingToTraverseFrom, bool checkParent = true) const;
    115116
    116117    virtual void dirtyLinesFromChangedChild(RenderObject&) { }

  • trunk/Source/WebCore/rendering/RenderLayer.cpp

    r288127 r288267  
    484484        // We need to connect ourselves when our renderer() has a parent.
    485485        // Find our enclosingLayer and add ourselves.
    486         auto* parentLayer = renderer().parent()->enclosingLayer();
     486        auto* parentLayer = renderer().layerParent();
    487487        if (!parentLayer)
    488488            return;
    489489
    490         auto* beforeChild = parentLayer->reflectionLayer() != this ? renderer().parent()->findNextLayer(*parentLayer, &renderer()) : nullptr;
     490        auto* beforeChild = parentLayer->reflectionLayer() != this ? renderer().layerNextSibling(*parentLayer) : nullptr;
    491491        parentLayer->addChild(*this, beforeChild);
    492492    }
    … …  
    654654RenderLayer* RenderLayer::stackingContext() const
    655655{
    656     if (establishesTopLayer())
    657         return renderer().view().layer();
    658 
    659656    auto* layer = parent();
    660657    while (layer && !layer->isStackingContext())
    … …  
    662659
    663660    ASSERT(!layer || layer->isStackingContext());
     661    ASSERT_IMPLIES(establishesTopLayer(), !layer || layer == renderer().view().layer());
    664662    return layer;
    665663}
    … …  
    14831481        // As we became visible, we need to dirty our stacking containers ancestors to be properly
    14841482        // collected. FIXME: When compositing, we could skip this dirtying phase.
    1485         for (RenderLayer* sc = stackingContext(); sc; sc = sc->stackingContext()) {
     1483        for (auto* sc = stackingContext(); sc; sc = sc->stackingContext()) {
    14861484            sc->dirtyZOrderLists();
    14871485            if (sc->hasVisibleContent())
    … …  
    15031501void RenderLayer::dirtyAncestorChainVisibleDescendantStatus()
    15041502{
    1505     for (RenderLayer* layer = this; layer; layer = layer->parent()) {
     1503    for (auto* layer = this; layer; layer = layer->parent()) {
    15061504        if (layer->m_visibleDescendantStatusDirty)
    15071505            break;
    … …  
    15131511void RenderLayer::setAncestorChainHasVisibleDescendant()
    15141512{
    1515     for (RenderLayer* layer = this; layer; layer = layer->parent()) {
     1513    for (auto* layer = this; layer; layer = layer->parent()) {
    15161514        if (shouldApplyPaintContainment(renderer())) {
    15171515            m_hasVisibleDescendant = true;
    … …  
    18371835RenderLayer* RenderLayer::enclosingAncestorForPosition(PositionType position) const
    18381836{
    1839     if (establishesTopLayer())
    1840         return renderer().view().layer();
    1841 
    1842     RenderLayer* curr = parent();
     1837    auto* curr = parent();
    18431838    while (curr && !isContainerForPositioned(*curr, position, establishesTopLayer()))
    18441839        curr = curr->parent();
    18451840
     1841    ASSERT_IMPLIES(establishesTopLayer(), !curr || curr == renderer().view().layer());
    18461842    return curr;
    18471843}
    … …  
    30723068        // layer from the parent now, assuming there is a parent
    30733069        if (paintFlags & PaintLayerFlag::HaveTransparency) {
    3074             // Top layer elements are not affected by ancestor opacities
    3075             if (!establishesTopLayer() && parent())
     3070            if (parent())
    30763071                parent()->beginTransparencyLayers(context, paintingInfo, paintingInfo.paintDirtyRect);
    30773072            else
    … …  
    40224017void RenderLayer::establishesTopLayerWillChange()
    40234018{
    4024     dirtyStackingContextZOrderLists();
     4019    if (auto* parentLayer = parent())
     4020        parentLayer->removeChild(*this);
    40254021}
    40264022
    40274023void RenderLayer::establishesTopLayerDidChange()
    40284024{
    4029     dirtyStackingContextZOrderLists();
    4030     if (isStackingContext())
    4031         dirtyZOrderLists();
     4025    if (auto* parentLayer = renderer().layerParent())
     4026        parentLayer->addChild(*this);
    40324027}
    40334028
    … …  
    44994494bool RenderLayer::clipCrossesPaintingBoundary() const
    45004495{
    4501     if (establishesTopLayer())
    4502         return true;
    4503 
    45044496    return parent()->enclosingPaginationLayer(IncludeCompositedPaginatedLayers) != enclosingPaginationLayer(IncludeCompositedPaginatedLayers)
    45054497        || parent()->enclosingCompositingLayerForRepaint() != enclosingCompositingLayerForRepaint();
    … …  
    45894581Ref<ClipRects> RenderLayer::parentClipRects(const ClipRectsContext& clipRectsContext) const
    45904582{
    4591     ASSERT(parent() || establishesTopLayer());
    4592 
    4593     auto containerLayer = establishesTopLayer() ? renderer().view().layer() : parent();
     4583    ASSERT(parent());
     4584
     4585    auto containerLayer = parent();
    45944586    auto temporaryParentClipRects = [&](const ClipRectsContext& clipContext) {
    45954587        auto parentClipRects = ClipRects::create();
Note: See TracChangeset for help on using the changeset viewer.