[Webkit-unassigned] [Bug 229666] [CSS Cascade Layers] Computer order correctly for late added sublayers

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=229666

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #436764|review?                     |review+
              Flags|                            |

--- Comment #3 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 436764
  --> https://bugs.webkit.org/attachment.cgi?id=436764
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=436764&action=review

> Source/WebCore/ChangeLog:3
> +        [CSS Cascade Layers] Computer order correctly for late added sublayers

Compute order

> Source/WebCore/style/RuleSet.cpp:86
> +void RuleSet::addRule(const StyleRule& rule, unsigned selectorIndex, unsigned selectorListIndex, unsigned cascadeLayerIdentifier, MediaQueryCollector* mediaQueryCollector)

unsigned -> CascadeLayerIdentifier ?

> Source/WebCore/style/RuleSet.cpp:93
> +        std::fill(m_cascadeLayerIdentifierForRulePosition.begin() + oldSize, m_cascadeLayerIdentifierForRulePosition.end(), 0);

Shame they don't init to 0 automatically

> Source/WebCore/style/RuleSet.cpp:405
> +        CascadeLayerIdentifier identifier = 0;

Oh there is a typedef! Perhaps it should be a type which can initialize itself to 0.

> Source/WebCore/style/RuleSet.cpp:470
> +    auto compareCascadeLayers = [&](CascadeLayerIdentifier a, CascadeLayerIdentifier b) {
> +        while (a && b) {
> +            // Identifiers are in parse order which almost corresponds to the layer priority order.
> +            // The only exception is when a sublayer gets added to a layer after adding other non-sublayers.
> +            // To resolve this we need look for a shared ancestor layer.
> +            auto aParent = ruleSet->cascadeLayerForIdentifier(a).parentIdentifier;
> +            auto bParent = ruleSet->cascadeLayerForIdentifier(b).parentIdentifier;
> +            if (aParent == bParent || aParent == b || bParent == a)
> +                break;
> +            if (aParent > bParent)
> +                a = aParent;
> +            else
> +                b = bParent;
> +        }
> +        return a < b;
> +    };
> +
> +    auto updateCascadeLayerOrder = [&] {
> +        Vector<CascadeLayerIdentifier> orderVector;
> +        for (CascadeLayerIdentifier identifier = 1; identifier <= ruleSet->m_cascadeLayers.size(); ++identifier)
> +            orderVector.append(identifier);
> +        std::sort(orderVector.begin(), orderVector.end(), compareCascadeLayers);
> +        for (unsigned i = 0; i < orderVector.size(); ++i)
> +            ruleSet->cascadeLayerForIdentifier(orderVector[i]).order = i + 1;
> +    };
> +
> +    if (mode == Mode::Normal && !ruleSet->m_cascadeLayers.isEmpty())
> +        updateCascadeLayerOrder();

Maybe put this in its own function

> Source/WebCore/style/RuleSet.h:193
> +    CascadeLayer& cascadeLayerForIdentifier(CascadeLayerIdentifier identifier) { return m_cascadeLayers[identifier - 1]; }
> +    const CascadeLayer& cascadeLayerForIdentifier(CascadeLayerIdentifier identifier) const { return m_cascadeLayers[identifier - 1]; }

Maybe assert that identifier != 0

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20210830/6a6ecbe7/attachment.htm>