370332 - Kill nsIBindingManager

Status: RESOLVED FIXED

(Core :: XBL, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

There is no reason for nsIBindingManager to be an XPCOM interface. It's almost exclusively used inside gkcontent and we're never going to freeze it.

Patch coming up that kills nsIBindingManager and nsIStyleRuleSupplier since the bindingmanager is the only implementation of both those interfaces.

I copied the few functions that existed outside of gkcontent to nsIDocument. This isn't ideal but I couldn't think of a better solution. And ideally some of this will be cleaned up as I wade through the XBL code.

I didn't make any changes to any call signatures as this patch is big enough as it is. I plan to do that in a separate patch.

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

The patch to nsBindingManager.h is a bit messy. The only thing I did was copy the comments from nsIBindingManager.h and change the calling convention. I also removed the GetEnclosingScope function to avoid having to #include nsIContent.h

Comment 2

[Boris Zbarsky [:bzbarsky]]

Why not leave the member as nsRefPtr, or even nsCOMPtr given that we now only implement one interface?

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

r=dbaron on the mozilla/layout/style/* changes, although I'm a little puzzled by the nsInspectorCSSUtils.cpp #include change.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

The #include is needed since nsIDocument.h no longer #includes ns(I)BindingManager.h. I didn't want to include nsBindingManager.h since that drags in stuff from other modules so I would have had to add to REQUIRES in various places.

This is also the reason why nsIDocument::mBindingManager can't be an nsRefPtr as the dtor is inlined. Is that the member you were referring to bz?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yes, that's the one I meant.  Add a comment explaining that in the header, maybe?

Comment 6

[Johnny Stenback (:jst)]

Comment on attachment 255033 [details] [diff] [review]
Patch to fix

r+sr=jst, but the last two files (xul.css and nsCycleCollectionParticipant.h) are unrelated to this change, right?.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Opps, the xul.css change is unrelated yes. The nsCycleCollectionParticipant.h change is needed though.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

This was checked in a while ago.