Comment on attachment 8801559 [details] Support list-style-image in stylo. https://reviewboard.mozilla.org/r/86262/#review84906 ::: servo/components/style/properties/gecko.mako.rs:1376 (Diff revision 1) > + pub fn set_list_style_image(&mut self, image: longhands::list_style_image::computed_value::T) { > + use values::computed::UrlOrNone; > + match image { > + UrlOrNone::None => { > + unsafe { > + Gecko_SetListStyleImageNone(&mut self.gecko); We probably should just have a function that lets us decref an image request and set it to null. The samge goes for setting the image, it's better if there's a single function that's used by all. Probably makes more sense as a pass in my FFI bindings work. Changes are fine as is, but once these are all done I'll try to refactor it a bit to reduce the number of FFI functions.

Comment on attachment 8801559 [details] Support list-style-image in stylo. https://reviewboard.mozilla.org/r/86262/#review84908

Comment on attachment 8801559 [details] Support list-style-image in stylo. https://reviewboard.mozilla.org/r/86262/#review84906 > We probably should just have a function that lets us decref an image request and set it to null. > > The samge goes for setting the image, it's better if there's a single function that's used by all. > > Probably makes more sense as a pass in my FFI bindings work. Changes are fine as is, but once these are all done I'll try to refactor it a bit to reduce the number of FFI functions. Yeah, it seems we could simplify things by just having an FFI function that produces a new nsStyleImageRequest object, and let the rust code do all the refcounting and setting. Happy to leave that for you to look at later.

Comment on attachment 8801555 [details] Bug 1310463 - Part 1: Make list-style-image use nsStyleImageRequest for storage. https://reviewboard.mozilla.org/r/86254/#review84952

Comment on attachment 8801556 [details] Bug 1310463 - Part 2: Simplify nsComputedDOMStyle::DoGetListStyleImage a little. https://reviewboard.mozilla.org/r/86256/#review84954 ::: layout/style/nsComputedDOMStyle.cpp:3538 (Diff revision 1) > - if (!list->GetListStyleImage()) { > + // XXXheycam As in SetValueToStyleImage, we might want to use the > + // URL stored in the nsStyleImageRequest's mImageValue if we > + // failed to resolve the imgRequestProxy. Probably also add a comment to SetValueToStyleImage stating that if that gets fixed, this function should be fixed in the same way.

Comment on attachment 8801557 [details] Bug 1310463 - Part 3: Add FFI functions for setting list-style-image. https://reviewboard.mozilla.org/r/86258/#review84964

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/869787233418 Part 1: Make list-style-image use nsStyleImageRequest for storage. r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/23bad27dfeb3 Part 2: Simplify nsComputedDOMStyle::DoGetListStyleImage a little. r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/5ebcedea1fa5 Part 3: Add FFI functions for setting list-style-image. r=xidorn

Servo PR: https://github.com/servo/servo/pull/13786

https://hg.mozilla.org/mozilla-central/rev/869787233418 https://hg.mozilla.org/mozilla-central/rev/23bad27dfeb3 https://hg.mozilla.org/mozilla-central/rev/5ebcedea1fa5

Backed out for being on top of 1288302, which I backed out for the bug 1311921 Talos regressions: https://hg.mozilla.org/integration/mozilla-inbound/rev/7fd0f297a13d

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5fd7c7cad759 Part 1: Make list-style-image use nsStyleImageRequest for storage. r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/0520d9787931 Part 2: Simplify nsComputedDOMStyle::DoGetListStyleImage a little. r=xidorn https://hg.mozilla.org/integration/mozilla-inbound/rev/be398e3e2b20 Part 3: Add FFI functions for setting list-style-image. r=xidorn

https://hg.mozilla.org/mozilla-central/rev/5fd7c7cad759 https://hg.mozilla.org/mozilla-central/rev/0520d9787931 https://hg.mozilla.org/mozilla-central/rev/be398e3e2b20