Confirmed users
97
edits
Gecko:DeCOMtamination: Difference between revisions
Upd
m (Sync remaining work on bug 243370) |
(Upd) |
||
| (15 intermediate revisions by 6 users not shown) | |||
| Line 1: | Line 1: | ||
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. | 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 want to get started, follow these steps. | ||
# Make sure you have successfully built a Mozilla product from source (preferably Mozilla Suite or Firefox). Official instructions can be found [[mdc:Build_Documentation|on MDC]]. You will want to change your configure options to include "ac_add_options --enable-debug" and "ac_add_options --disable-optimize". | # Make sure you have successfully built a Mozilla product from source (preferably Mozilla Suite or Firefox). Official instructions can be found [[mdc:Build_Documentation|on MDC]]. 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 | # Choose one of the deCOMtamination problems below. | ||
# Note in this wiki page that you are working on the task. | # 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]]. | # 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 [https://bugzilla.mozilla.org/buglist.cgi?query_format=&short_desc_type=allwordssubstr&short_desc=decomtam&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&value0-0-0= existing bug] for the deCOMtamination task, or create a new bug for it | # Find an [https://bugzilla.mozilla.org/buglist.cgi?query_format=&short_desc_type=allwordssubstr&short_desc=decomtam&long_desc_type=substring&long_desc=&bug_file_loc_type=allwordssubstr&bug_file_loc=&status_whiteboard_type=allwordssubstr&status_whiteboard=&keywords_type=allwords&keywords=&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&emailassigned_to1=1&emailtype1=exact&email1=&emailassigned_to2=1&emailreporter2=1&emailqa_contact2=1&emailtype2=exact&email2=&bugidtype=include&bug_id=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&cmdtype=doit&order=Reuse+same+sort+as+last+time&field0-0-0=noop&type0-0-0=noop&value0-0-0= existing bug] for the deCOMtamination task, or create a new bug for it, 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:// | # Finish the task by updating all callers and implementations of the functions and methods to be consistent with the new signatures. Always use http://mxr.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 sure you can build and run Mozilla with your changes. Test it thoroughly. | ||
# Make a patch with | # Make a patch with <code>hg diff -p -U 8</code>, and submit it as an attachment in the bug. Request review, e.g. from roc@ocallahan.org. | ||
Here are some places known to need deCOMtamination or general interface cleanup: | Here are some places known to need deCOMtamination or general interface cleanup: | ||
| Line 20: | Line 18: | ||
* in many cases we have a singleton interface like nsIComboboxControlFrame that is only implemented by one class, is not defined in IDL (and is therefore not scriptable), and is not visible outside Gecko. In these cases we can save code and data size by eliminating the interface, moving its methods to the concrete class and devirtualizing them, and casting to the class directly, like we just did with nsGfxButtonControlFrame. We can even assign the class an IID so you can get to it with QueryInterface. For example you can QI to nsBlockFrame this way.({{bug|326944}}) | * in many cases we have a singleton interface like nsIComboboxControlFrame that is only implemented by one class, is not defined in IDL (and is therefore not scriptable), and is not visible outside Gecko. In these cases we can save code and data size by eliminating the interface, moving its methods to the concrete class and devirtualizing them, and casting to the class directly, like we just did with nsGfxButtonControlFrame. We can even assign the class an IID so you can get to it with QueryInterface. For example you can QI to nsBlockFrame this way.({{bug|326944}}) | ||
* Not really deCOMtamination, but we have new methods nsIContent::AttrValueIs and nsIContent::FindAttrValueIn that can be used in many many places where nsIContent::GetAttr is currently used. The new methods reduce code size and are faster. We need to convert GetAttr calls to use the new methods.(<strike>{{bug|333896}}</strike>) | * Not really deCOMtamination, but we have new methods nsIContent::AttrValueIs and nsIContent::FindAttrValueIn that can be used in many many places where nsIContent::GetAttr is currently used. The new methods reduce code size and are faster. We need to convert GetAttr calls to use the new methods.(<strike>{{bug|333896}}</strike>) | ||
* Access to nsISupports* objects could be improved by using direct access to the nsSupports*Impl implementation classes from C++ ({{bug|399237}}). | |||
Here's some content/ specific interfaces that could be decomtaminated, and by that I mean the nsresult return type removed (replaced with void or an out param, if available): | |||
''Note: nsAString& outparams can't be rewritten, in that case the return type should become void'' | |||
* ns[I]NodeInfo(remove return types manually), nsNodeInfoManager | |||
* ns[I]NameSpaceManager, impl and interface could be combined I think, and decomtaminated. | |||
* nsIFormControl::GetForm(), SetForm(), Reset() | |||
**<del>GetForm causes multiple inheritance dance due to being defined in nsIDOMHTMLInputElement.idl</del> | |||
**<del>Reset should loose return value</del> (not possible) | |||
* content/html/content/public/nsIRadio*.h | |||
* nsIFrameSetElement {{bug|578564}} | |||
* nsILink, nuke nsresults for GetLinkState(), SetLinkState(), LinkAdded(), and LinkRemoved() | |||
* <del>nsIOptionElement</del> {{bug|578570}} | |||
* nsITextControlElement | |||
* nsIPrivateTextEvent::GetText(), nuke the nsresult, and for GetInputRange() and GetEventReply(), nuke the nsresult, return null on error, no callers check anyways. | |||
* nsIPrivateTextRangeList::GetLength(), nuke nsresult, and for Item() too, but need to check callers. Should simply just return null if invalid. | |||
* nsIPrivateDOMEvent, all nsresults can go. | |||
* nsBindingManager, nsIXBLService | |||