Open Bug 1375525 Opened 7 years ago Updated 2 months ago

Stylo: Fix servo's animation cascade so we can take rule nodes in the common case

Categories

(Core :: CSS Parsing and Computation, enhancement, P4)

enhancement

Tracking

()

Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- ?

People

(Reporter: jryans, Unassigned)

References

(Blocks 1 open bug)

Details

For the normal match and cascade path, we'd like to continue `take()`ing rule nodes, to avoid refcount traffic, etc.

However, it's currently difficult to do so after bug 1370719 which places the rule nodes inside `ComputedValues` during the cascade.  The main problem is the custom iterator that animation can pass to `apply_declarations`, so we should fix this and resume taking the rules in the common case again.

Quoting :emilio from bug 1370719 comment 34:
> The lifetime issue seems unavoidable as things stand. But the root cause is
> that still we have that apply_declarations call from Servo's animation code,
> which instead of creating a rule node, it goes and applies them directly.
> Seems like we should really fix that and use a rule node there instead. Then
> all the iterator madness is moot and can go away, and the cascade() function
> can just take rule nodes.
I think we should fix servo's animation code, right? We need to change both of them?
Summary: Stylo: Fix animation cascade so we can take rule nodes in the common case → Stylo: Fix servo's animation cascade so we can take rule nodes in the common case
(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> I think we should fix servo's animation code, right? We need to change both
> of them?

The issue is in `animation::compute_style_for_animation_step`, which looks like it's only called in Servo (if I am reading things correctly).  Essentially, anyone calling `apply_declarations` might need some adjustment.  At the moment, it looks like that's the main `cascade` path and also `compute_style_for_animation_step`.
Blocks: stylo-perf
No longer blocks: stylo
Priority: -- → P4
Severity: normal → S3
See Also: → 1883255
You need to log in before you can comment on or make changes to this bug.