Closed Bug 1728754 Opened 3 months ago Closed 3 months ago

Basic layer rules plumbing.

Categories

(Core :: CSS Parsing and Computation, task)

task

Tracking

()

RESOLVED FIXED
94 Branch
Tracking Status
firefox94 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

Attachments

(5 files)

No description provided.

This shouldn't have any behavior change, but is necessary because for
cascade layers we are going to need to handle the child rules / sheets
ourselves, in order to handle nested layers properly.

For that, deal with children in add_rule recursively, to keep the
current layer name up-to-date in block layer rules.

This is not the final version of this code, as right now something like this:

@layer A {
...
}

@layer B {
...
}

@layer A.A {
...
}

Would give A.A more priority over B, which is not correct. There are tests for
this incoming in wpt sync and such, but that can be tweaked later.

Depends on D124334

This code is really hot, and we've had perf regressions in the past for
introducing function calls in the hot path.

After the previous patch, add_rule is recursive and thus it can't be
inlined, causing a function call for each CSS rule.

This reduces the overhead by making the function take a rule list
instead, causing a function call per rule list, which should be
unnoticeable in practice.

Depends on D124335

I want to land this separately because we might want to get smarter with
the size of the Rule struct (maybe restricting layer order to a u8 per
scope and packing it with the source order, since 255 layers seem
plenty), but I'd rather do the obvious thing for now.

Depends on D124336

Same, I want to land this separately to see if it affects
micro-benchmarks. If so, we might want to pack the layer order
somewhere (though in this case I'm not sure where, tbh).

With this, layer rules should have an effect on the page. There are
a few things missing before being able to enable them:

  • Fix nested layer order in some cases (when parent layers are declared
    out of order, see the previous commit mentioning this).
  • Some kind of OM representation, perhaps.
  • Tests of course, which are coming in bug 1728722 and bug 1727276.

But this should be enough to allow playing with them.

Depends on D124337

Keywords: leave-open
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/743777e6d2f2
Factor out adding a rule in CascadeData::add_rule. r=boris
https://hg.mozilla.org/integration/autoland/rev/56c800f59664
Compute layer order during CascadeData rebuild. r=boris
https://hg.mozilla.org/integration/autoland/rev/cb7802cd162d
Tweak recursion in add_rule to only cause a function call per recursion level. r=boris
Blocks: 1729151
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/84c423dca87e
Plumb layer order through ApplicableDeclarationBlock, and make it have an effect. r=boris
Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Regressions: 1729705
Target Milestone: --- → 94 Branch
You need to log in before you can comment on or make changes to this bug.