318128 - Please decide whether the CSS loader is a service or not

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[[e-mail]]

Then I can run up a patch to switch the errant consumers.

Comment 1

[Boris Zbarsky [:bzbarsky]]

It's not a service.  We have existing bugs on at least some of the code that uses GetService to get it, iirc.

Comment 2

[[e-mail]]

Attachment: Proposed patch

This changes the four cases found by the query. I converted the code to use NS_NewCSSLoader in the two layout sources. I was worried that something was expecting the CSS loader to outlive the function but it hasn't stopped me from using this patched build to attach the patch so I guess it's not an issue ;-)

Comment 3

[Boris Zbarsky [:bzbarsky]]

Won't this lead to some perf impact from all the thrashing?  Some of these (document viewer and chrome registry) would likely want CSSLoaders fairly often...

Comment 4

[[e-mail]]

Attachment: Cache CSS loader?

I can cache the CSS loader in the chrome registry if you like (I can't write a patch for toolkit because I don't have a xulrunner tree). Since the stylesheet cache already caches the stylesheets I didn't see the point of caching the CSS loader as well, and the document viewer only requires its CSS loader when the usechromesheets attribute is set, which only applies to the sidebar in Modern.
I do see that I could optimize the placement of the loader creation slightly.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 204534 [details] [diff] [review]
Cache CSS loader?

> Since the stylesheet cache already caches the stylesheets I didn't see the
> point of caching the CSS loader as well

It'd prevent us from creating (quite as many) oodles of CSS loaders at startup...

I agree that for the viewer this might be a non-issue.

Comment 6

[[e-mail]]

Attachment: Updated patch

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 205031 [details] [diff] [review]
Updated patch

Thank you for doing this!  sr=bzbarsky

Comment 8

[[e-mail]]

Fix checked in.