Closed Bug 385750 Opened 13 years ago Closed 13 years ago
Cleanup implementations of Query
Interface in layout/ widget/ and content/
No description provided.
A couple of notes: nsDirectionalFrame::QI does not implement nsIFrameDebug/nsIFrame - is there a reason for this? (are they missing in frame dumps?) I've left it as is, but perhaps we should call nsFrame::QI at the end? BTW, do we really need to implement this QI at all? I could only find one consumer: http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsBidiPresUtils.cpp&rev=1.95&mark=598-600#597 couldn't we use NS_STATIC_CAST there instead? nsImageFrame and nsObjectFrame explicitly implements QI to nsIFrameDebug/nsIFrame. I've left it as is, but I think it would be better if we called its super instead (which will end up in nsFrame::QI). Let me know if you want any of the above fixed, I'll be happy to do so.
Did you mean to include the changes outside of layout in the patch for me to review? r+sr=dbaron on the changes inside layout in that patch, if you fix nsDirectionalFrame, nsImageFrame, and nsObjectFrame to call the base class QI. And feel free to attach a followup patch on removing the nsDirectionalFrame QI entirely...
(In reply to comment #2) > Did you mean to include the changes outside of layout in the patch for me to > review? Yes, please. > r+sr=dbaron on the changes inside layout in that patch, if you fix > nsDirectionalFrame, nsImageFrame, and nsObjectFrame to call the base class Ok. A question regarding QI to nsISupports: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsObjectFrame.cpp&rev=1.596&root=/cvsroot&mark=433-434#414 http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/layout/generic/nsIObjectFrame.h&rev=1.12&root=/cvsroot&mark=54#43 it looks like we currently return a different value than nsFrame::QI would return - I'm not sure if that is intentional, should I propagate nsISupports too or leave it as is? (same thing in nsImageFrame.cpp)
OK, r+sr=dbaron on the widget changes as well. But I'd prefer if you got module owner review for the content and xpcom changes. In nsObjectFrame, I think you should propagate the nsISupports QI as well.
Johnny, could you review the content/* changes please?
Comment on attachment 269871 [details] [diff] [review] Patch rev. 2 (diff -w) r+sr=jst for the content changes.
Checked in to trunk at 2007-07-03 19:15 PDT. -> FIXED
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Summary: Cleanup implementations of QueryInterface within Layout → Cleanup implementations of QueryInterface in layout/ widget/ and content/
Target Milestone: --- → mozilla1.9beta1
On the normal refcounted objects (nsMenuBoxObject and nsDeviceContextSpec*) can't we replace that handwritten code with the NS_IMPL_ISUPPPORTS macros?
(In reply to comment #9) > On the normal refcounted objects (nsMenuBoxObject and nsDeviceContextSpec*) > can't we replace that handwritten code with the NS_IMPL_ISUPPPORTS macros? Filed bug 387211
You need to log in before you can comment on or make changes to this bug.