If nsISVGValueObserver would inherit nsISupportsWeakReference, the size of vtable should be smaller for many SVG objects, I think. Also that way the comment http://lxr.mozilla.org/seamonkey/source/content/svg/content/src/nsISVGValueObserver.h#52 would be really forced to be true. Comments?
Created attachment 255203 [details] [diff] [review] possible patch This is one way to reduce the size of all the implementations of nsISVGValueObserver (on 32bit systems 4 bytes, 64bit systems 8 bytes). I know it is a bit ugly the make nsI*** to inherit nsSomeClass but that makes fixing this very easy.
Attachment #255203 - Flags: review?(tor)
We seem to be gradually removing nsISVGValueObserver and replacing it with nsIMutationObserver (bug 359516 and bug 370416 for instance). I wonder if we could or should take that to its logical conclusion and remove nsISVGValueObserver altogether.
While in general this looks good (though you if you were really feeling industrious you could remove the includes of nsWeakReference.h), I'd prefer to leave the style question of a nsI* inheriting from a nsSomeClass to one of the core gecko people.
Comment on attachment 255203 [details] [diff] [review] possible patch roc, what do you say about this? One (still ugly) solution would be to rename nsISVGValueObserver to nsSVGValueObserver but leave the IID (which is needed for do_QueryReferent) or nsISVGValueObserver could just inherit nsISupportsWeakReference and there would be a separate class (inheriting nsISVGValueObserver) to implement nsISupportsWeakReference. That would mean copy-pasting code from nsSupportsWeakReference.
Attachment #255203 - Flags: review?(roc)
Comment on attachment 255203 [details] [diff] [review] possible patch I'm OK with this. Although it'd be better still to get rid of nsISVGValueObserver if we could.
Attachment #255203 - Flags: review?(roc) → superreview+
I hope we could take the patch for now, and if someone comes up with a solution to remove nsISVGValueObserver, then even better.
Comment on attachment 255203 [details] [diff] [review] possible patch We are getting rid of nsISVGValueObserver gradually, there just hasn't been a concerted effort.
Attachment #255203 - Flags: review?(tor) → review+
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.