1351662 - stylo: Figure out an approach to completely avoid leaking from owning Gecko @font-face rule in Servo

Status: NEW

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

Description

[Xidorn Quan [:xidorn] UTC+10]

In bug 1345696, we make Servo's CssRule own Gecko's nsCSSFontFaceRule object. This makes that bug easier to implement (reasoning in bug 1345696 comment 4), but opens a potential problem that cycle reference may not be detected correctly because CC doesn't track through Servo objects.

We use a hack in that bug (part 6) that we note two edges from ServoCSSRuleList to its nsCSSFontFaceRule child, one for itself, another for the indirect ownership via Servo's CssRules. The idea is that if someone wants to put some
reference on the @font-face rule, the rule itself and its parent list should
have been constructed in the CSSOM tree. If they are not there, we are safe.

However, that is not perfect, because we still have internal interface nsIDOMFontFace which can return the rule to script without connecting the objects in the CSSOM tree.

It doesn't seem to me devtools is setting anything on that object, so we are probably fine now (thus low priority for now). But we may want to figure out an approach to completely avoid this kind of leak from happening.

There are three possible routes:

1. We eventually make FontFace object backed by Servo's struct, and we construct a wrapper CSSFontFaceRule like what we do for other rules. We can either move FontFace from relying on nsCSSValue, or we store nsCSSValue in Servo's rule object to allow efficient use.

2. We eventually build an mechanism to allow cycle collector to traverse through Servo objects. This is possible given now Rust has Macro 1.1, and we can have a custom derive to help implementing that on objects.

3. We add a mechanism to aggressively build up the CSSOM tree, and trigger it in any script-faced interface, and set a flag to the document to make CSSOM tree be aggressively built when new style sheet or group rule is attached.

None of these three options is trivial to implement... but I guess the third one is probably the easiest to add if we find it urgent, but the first one is probably what we want to do in long term.

Comment 1

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57