1025725 - refactor some of nsXBLPrototypeResources to make it easier to deal with for style sheet changes

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Cameron McCormack (:heycam)]

Instead of allowing random objects to poke into public member variables on nsXBLPrototypeResources, let's encapsulate them to make code that manipulates them more sane.

Comment 1

[Cameron McCormack (:heycam)]

Attachment: Part 1: Give nsXBLPrototypeResources::mRuleProcessor a more concrete type.

We never create any other kind of nsIStyleRuleProcessor than a nsCSSRuleProcessor.

Comment 2

[Cameron McCormack (:heycam)]

Attachment: Part 2: Encapsulate nsXBLPrototypeResources::mRuleProcessor.

Comment 3

[Cameron McCormack (:heycam)]

Attachment: Part 3: Encapsulate nsXBLPrototypeResources::mStyleSheetList.

I'm giving this an nsStyleSet-like API, for familiarity.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8440479 [details] [diff] [review]
Part 1: Give nsXBLPrototypeResources::mRuleProcessor a more concrete type.

r=me

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8440480 [details] [diff] [review]
Part 2: Encapsulate nsXBLPrototypeResources::mRuleProcessor.

r=me

Comment 6

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8440481 [details] [diff] [review]
Part 3: Encapsulate nsXBLPrototypeResources::mStyleSheetList.

>+    binding->PrototypeBinding()->AppendStyleSheetsTo(*array);

Why AppendStyleSheetsTo, not just AppendStyleSheets?

>+nsXBLPrototypeBinding::RequireResources()

Normally we'd call this EnsureResources.

r=me with at least the EnsureResources change made.

Comment 7

[Cameron McCormack (:heycam)]

(In reply to Boris Zbarsky [:bz] from comment #6)
> Comment on attachment 8440481 [details] [diff] [review]
> Part 3: Encapsulate nsXBLPrototypeResources::mStyleSheetList.
> 
> >+    binding->PrototypeBinding()->AppendStyleSheetsTo(*array);
> 
> Why AppendStyleSheetsTo, not just AppendStyleSheets?

I thought AppendStyleSheets would sound like you were passing in the array of sheets that you wanted to append to nsXBLPrototypeResources::mStyleSheetList.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Oh, I see.  I totally got what the function did backwards somehow.

Comment 9

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d37890e3b9db
https://hg.mozilla.org/integration/mozilla-inbound/rev/6038055f6b19
https://hg.mozilla.org/integration/mozilla-inbound/rev/d4e7841a2cac

Comment 10

[Cameron McCormack (:heycam)]

Missing include:

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4edb2cf572f

Comment 11

[Cameron McCormack (:heycam)]

And a bit more:

https://hg.mozilla.org/integration/mozilla-inbound/rev/bff872c9d4b2

Comment 12

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d37890e3b9db
https://hg.mozilla.org/mozilla-central/rev/6038055f6b19
https://hg.mozilla.org/mozilla-central/rev/d4e7841a2cac
https://hg.mozilla.org/mozilla-central/rev/d4edb2cf572f
https://hg.mozilla.org/mozilla-central/rev/bff872c9d4b2

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Hey Cameron, this work blocks bug 1025738 which we'd like to backport to Aurora. Is this bug reasonably safe to land there as well? If so, can you please nominate it for approval? Thanks!

Comment 14

[Cameron McCormack (:heycam)]

Comment on attachment 8440479 [details] [diff] [review]
Part 1: Give nsXBLPrototypeResources::mRuleProcessor a more concrete type.

Approval Request Comment
[Feature/regressing bug #]: -
[User impact if declined]: Would prevent bug 1025738 from landing, which solves some memory leak problems.
[Describe test coverage new/current, TBPL]: Been on m-c for a couple of weeks.
[Risks and why]: Low, this is a small, no-behaviour-change refactoring.
[String/UUID change made/needed]: -

(request applies to all 3 parts in this bug)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4ee1eccd5cdd
https://hg.mozilla.org/releases/mozilla-aurora/rev/df0670536f69
https://hg.mozilla.org/releases/mozilla-aurora/rev/9e809dfdf2ae