Bug 1339629 implements deep cloning of css rules, but some uses appear to leak memory. Mochitest layout/style/test/test_stylesheet_clone_font_face.html triggers cloning of a fontface rule, and appears to leak.

Comment on attachment 8874524 [details] Bug 1367523 Part 1: Servo-side define and call Gecko CounterStyle and FontFaceRule clone functions. https://reviewboard.mozilla.org/r/145514/#review150512 ::: servo/components/style/stylesheets/mod.rs:291 (Diff revision 3) > let rule = arc.read_with(guard); > CssRule::Media(Arc::new( > lock.wrap(rule.deep_clone_with_lock(lock, guard)))) > }, > CssRule::FontFace(ref arc) => { > - let rule = arc.read_with(guard); > + let rule = arc.read_with(&guard); Why do we need this "&"? ::: servo/components/style/stylesheets/mod.rs:296 (Diff revision 3) > - let rule = arc.read_with(guard); > - CssRule::FontFace(Arc::new(lock.wrap(rule.clone()))) > + let rule = arc.read_with(&guard); > + CssRule::FontFace(Arc::new(lock.wrap( > + rule.deep_clone_from_gecko()))) > }, > CssRule::CounterStyle(ref arc) => { > - let rule = arc.read_with(guard); > + let rule = arc.read_with(&guard); ...and here?

Comment on attachment 8874525 [details] Bug 1367523 Part 2: Gecko-side implement the CounterStyle and FontFaceRule clone functions. https://reviewboard.mozilla.org/r/145516/#review150514

Comment on attachment 8874526 [details] Bug 1367523 Part 3: Re-enable layout/style/test/test_stylesheet_clone_font_face.html. https://reviewboard.mozilla.org/r/145892/#review150516

Comment on attachment 8874527 [details] Bug 1367523 Part 4: Update the layout/reftests/stylesheet-cloning/counter-style-rule-clone.html test to use correct unicode literals. https://reviewboard.mozilla.org/r/145894/#review150520 Is there a problem with the non-ASCII values?

Comment on attachment 8874527 [details] Bug 1367523 Part 4: Update the layout/reftests/stylesheet-cloning/counter-style-rule-clone.html test to use correct unicode literals. https://reviewboard.mozilla.org/r/145894/#review150520 Yes, strangely, setting the symbols property to a string that contains unicode characters doesn't "stick". I'll search for an existing bug and file one if necessary.

Comment on attachment 8874527 [details] Bug 1367523 Part 4: Update the layout/reftests/stylesheet-cloning/counter-style-rule-clone.html test to use correct unicode literals. https://reviewboard.mozilla.org/r/145894/#review150520 Nevermind; I was using incorrect unicode literals -- correct for CSS, wrong for JS.

Comment on attachment 8875512 [details] Bug 1367523 Part 1b: Servo-side make cloning of FontFaceRule and CounterStyleRule compile for both servo and gecko configs. https://reviewboard.mozilla.org/r/146938/#review151056 ::: servo/components/style/stylesheets/mod.rs:293 (Diff revision 1) > - if cfg!(feature = "gecko") { > + rule.clone_conditionally_gecko_or_servo()))) > - rule.deep_clone_from_gecko() > - } else { > - rule.clone() > - }))) This change is fine. But FWIW you can do real conditional compilation on expression blocks, like this, if you wanted to keep the switching behaviour in here: CSSRule::FontFace(Arc::new(lock.wrap( #[cfg(feature = "gecko")] { rule.deep_clone_from_gecko() } #[cfg(not(feature = "gecko"))] { rule.clone() } )))

Comment on attachment 8875512 [details] Bug 1367523 Part 1b: Servo-side make cloning of FontFaceRule and CounterStyleRule compile for both servo and gecko configs. https://reviewboard.mozilla.org/r/146938/#review151374 ::: servo/components/style/stylesheets/mod.rs:293 (Diff revision 1) > - if cfg!(feature = "gecko") { > + rule.clone_conditionally_gecko_or_servo()))) > - rule.deep_clone_from_gecko() > - } else { > - rule.clone() > - }))) Rust compiler choked on that, not happy to find another # directive after the first block. I wasn't able to find a way to fight through it, so I'm keeping the proposed method.

Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f0ce0e021b87 Part 2: Gecko-side implement the CounterStyle and FontFaceRule clone functions. r=heycam https://hg.mozilla.org/integration/autoland/rev/2d26be0ac13f Part 3: Re-enable layout/style/test/test_stylesheet_clone_font_face.html. r=heycam https://hg.mozilla.org/integration/autoland/rev/30a798334757 Part 4: Update the layout/reftests/stylesheet-cloning/counter-style-rule-clone.html test to use correct unicode literals. r=heycam

Landed in mozilla-central: changeset: 363082:30a798334757 user: Brad Werth <bwerth@mozilla.com> date: Mon Jun 05 11:59:24 2017 -0700 summary: Bug 1367523 Part 4: Update the layout/reftests/stylesheet-cloning/counter-style-rule-clone.html test to use correct unicode literals. r=heycam changeset: 363081:2d26be0ac13f user: Brad Werth <bwerth@mozilla.com> date: Tue May 30 17:55:42 2017 -0700 summary: Bug 1367523 Part 3: Re-enable layout/style/test/test_stylesheet_clone_font_face.html. r=heycam changeset: 363080:f0ce0e021b87 user: Brad Werth <bwerth@mozilla.com> date: Mon May 22 17:21:09 2017 -0700 summary: Bug 1367523 Part 2: Gecko-side implement the CounterStyle and FontFaceRule clone functions. r=heycam