Closed Bug 1025725 Opened 5 years ago Closed 5 years ago

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

Categories

(Core :: XBL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

Attachments

(3 files)

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.
We never create any other kind of nsIStyleRuleProcessor than a nsCSSRuleProcessor.
Assignee: nobody → cam
Status: NEW → ASSIGNED
Attachment #8440479 - Flags: review?(bzbarsky)
I'm giving this an nsStyleSet-like API, for familiarity.
Attachment #8440481 - Flags: review?(bzbarsky)
Comment on attachment 8440479 [details] [diff] [review]
Part 1: Give nsXBLPrototypeResources::mRuleProcessor a more concrete type.

r=me
Attachment #8440479 - Flags: review?(bzbarsky) → review+
Comment on attachment 8440480 [details] [diff] [review]
Part 2: Encapsulate nsXBLPrototypeResources::mRuleProcessor.

r=me
Attachment #8440480 - Flags: review?(bzbarsky) → review+
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.
Attachment #8440481 - Flags: review?(bzbarsky) → review+
(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.
Oh, I see.  I totally got what the function did backwards somehow.
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 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)
Attachment #8440479 - Flags: approval-mozilla-aurora?
Attachment #8440479 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.