1404006 - stylo: Stop using Arc<Locked<PropertyDeclarationBlock>>.

Status: RESOLVED INCOMPLETE

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

Description

[Emilio Cobos Álvarez (:emilio)]

Everything that uses it already has a reference to a Locked<Something>, or can get one easily, so it's not clear to me why it's needed.

That would allow us to remove all the dirty style attribute stuff, and the immutable property declaration blocks, etc.

We could just use `Arc::make_mut` when relevant, and that way ensuring that the rule tree (which would contain just Arc<PropertyDeclarationBlock> for style attributes and such) doesn't change under our feet, given we're effectively cloning stuff anyway.

This will also allow servo to stop eagerly serializing declaration blocks for the style attribute, which is also nice.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

This is much easier after killing the old style system.

Comment 2

[Patrick Walton (:pcwalton)]

This is really hurting our MotionMark performance.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(In reply to Patrick Walton (:pcwalton) from comment #2)
> This is really hurting our MotionMark performance.

(To be clear, this is Servo's MotionMark performance).

And agreed, I really really want to fix this, but we need to significantly change the Setup in the Gecko side and in Servo, so I'd rather just change it in two instead of three style systems :)

Comment 4

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 5

[Hiroyuki Ikezoe (:hiro)]

If I understand what this bug will do, this would also fix bug 1393323.
But I couldn't add it to dependency, there are dependency loop somewhere. :)

Comment 6

[Emilio Cobos Álvarez (:emilio)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #5)
> If I understand what this bug will do, this would also fix bug 1393323.
> But I couldn't add it to dependency, there are dependency loop somewhere. :)

Yes, sounds likely. It effectively makes the rule tree immutable, which prevents bugs like that.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Upon reflection, this will not fix bug 1393323, since we still keep the Arc<Locked<StyleRule>> in the rule tree. We could not insert rules in the rule tree, only declaration blocks, in which case it would fix it, but then we'd have to rethink all our inspector stuff and what not as well.

Also, given that, this will not fix the dependent bugs which don't rely on style="", and also would make unfixable bug 1452214.

So I think I want to go with bug 1465474 instead for fixing those issues (note: not bug 1393323).