Gecko:DeCOMtamination: Difference between revisions
Jump to navigation
Jump to search
mNo edit summary |
No edit summary |
||
Line 42: | Line 42: | ||
* Not really deCOMtamination, but anyway: many frames have an mPresContext field. This should be removed, and uses of that field can just call GetPresContext on the frame. | * Not really deCOMtamination, but anyway: many frames have an mPresContext field. This should be removed, and uses of that field can just call GetPresContext on the frame. | ||
* Lots of the non-Box methods of nsIFrame could be cleaned up too. We can remove the nsPresContext parameter from almost all of them. Many of them return their result in an 'out' parameter and should just return the result directly. Understanding the best way to do this for various methods will require some more creativity and understanding of the existing code. | * Lots of the non-Box methods of nsIFrame could be cleaned up too. We can remove the nsPresContext parameter from almost all of them. Many of them return their result in an 'out' parameter and should just return the result directly. Understanding the best way to do this for various methods will require some more creativity and understanding of the existing code. | ||
* https://bugzilla.mozilla.org/show_bug.cgi?id=300313 Line/word breaking service should be deCOMtaminated and simplified |
Revision as of 02:38, 11 July 2005
For more information about Gecko:DeCOMtamination contact roc@ocallahan.org.
The basic idea is to refactor interfaces to remove unnecessary "XPCOM style" ugliness and other interface design errors. Check the Gecko:Interface Style Guide for information about how to design these clean interfaces.
If you want to get started, follow these steps. If you get stuck, don't give up, mail roc for help!
- Make sure you have successfully built a Mozilla product from source (preferably Mozilla Suite or Firefox). Instructions for Windows here: http://texturizer.net/firefox/build.html Linux here: http://daihard.home.comcast.net/firefox/build.html You will want to change your configure options to include "ac_add_options --enable-debug" and "ac_add_options --disable-optimize".
- Choose one of the deCOMtamination problems below, or email roc for suggestions.
- Note in this wiki page that you are working on the task.
- Figure out what the cleaned-up function and method signatures will be, taking into account the hints in the Gecko:Interface Style Guide.
- Find an existing bug for the deCOMtamination task, or create a new bug for it, CCing roc@ocallahan.org, and attach a patch file showing what the new signatures will look like. Get feedback to make sure you're on the right track.
- Finish the task by updating all callers and implementations of the functions and methods to be consistent with the new signatures. Always use http://lxr.mozilla.org/mozilla to search for users of the method. There may be code that uses the method that's not built by your build configuration.
- Make sure you can build and run Mozilla with your changes. Test it thoroughly.
- Make a patch with CVS diff -utp8, and submit it as an attachment in the bug. Request review, e.g. from roc.
Here's the current Gecko:DeCOMtamination activity:
Here are some places known to need deCOMtamination or general interface cleanup:
- NS_New*Frame functions. E.g. currently we have
nsresult NS_NewViewportFrame(nsIPresShell* aPresShell, nsIFrame** aNewFrame) { NS_PRECONDITION(aNewFrame, "null OUT ptr"); if (nsnull == aNewFrame) { return NS_ERROR_NULL_POINTER; } ViewportFrame* it = new (aPresShell) ViewportFrame; if (nsnull == it) { return NS_ERROR_OUT_OF_MEMORY; } *aNewFrame = it; return NS_OK; }
This should change to
nsIFrame* NS_NewViewportFrame(nsIPresShell* aPresShell) { return new (aPresShell) ViewportFrame; }
- nsIFrame::Append/Insert/RemoveFrame: http://bugzilla.mozilla.org/show_bug.cgi?id=244581 The main goal here is to remove the nsPresContext and nsPresShell parameters, since the frame always knows its own prescontext and you can get the presshell from the prescontext.
- Box methods in nsIFrame: http://bugzilla.mozilla.org/show_bug.cgi?id=243370 See the section under "// BOX LAYOUT METHODS" in nsIFrame.h. GetPrefSize, GetMinSize, GetMaxSize, GetFlex, GetOrdinal, GetAscent, IsCollapsed, IsDirty, HasDirtyChildren, GetChildBox, GetNextBox, GetParentBox, GetBorderAndPadding, GetBorder, GetPadding, GetInset, GetMargin, GetLayoutManager, GetContentRect, GetClientRect, GetVAlign, GetHAlign, GetOverflow, GetIndexOf, and ChildrenMustHaveWidgets probably can all return their results directly instead of via an out parameter. GetOrientation and GetDirection can be removed and all callers redirected to IsHorizontal and IsNormalDirection. GetContentRect should be removed since it's trivial, and GetClientRect should be renamed to GetContentRect and return the rect directly. Some of these methods may only have one implementation in which case we can make them non-virtual or even inline.
- Not really deCOMtamination, but anyway: many frames have an mPresContext field. This should be removed, and uses of that field can just call GetPresContext on the frame.
- Lots of the non-Box methods of nsIFrame could be cleaned up too. We can remove the nsPresContext parameter from almost all of them. Many of them return their result in an 'out' parameter and should just return the result directly. Understanding the best way to do this for various methods will require some more creativity and understanding of the existing code.
- https://bugzilla.mozilla.org/show_bug.cgi?id=300313 Line/word breaking service should be deCOMtaminated and simplified