Closed Bug 235882 Opened 20 years ago Closed 20 years ago

Make nsComponentManagerImpl::AddLoaderType return only one type of value

Categories

(Core :: XPCOM, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

Details

Attachments

(2 files)

from bug 235381 comment 5. dougt agrees that
nsComponentManagerImpl::AddLoaderType needs to be changed, the proposal is to
make it return an nsresult and out the loadertype.

an alternative could be to just have it return a result and make people use
GetLoaderType after calling AddLoaderType.
Attached patch proposalSplinter Review
Attachment #142477 - Flags: review?(dougt)
Comment on attachment 142477 [details] [diff] [review]
proposal

don't make this change:

-	 int loadertype = GetLoaderType(values[2]);
+	 PRUint32 loadertype = GetLoaderType(values[2]);


why bother with this:

+	     if (rv == NS_ERROR_OUT_OF_MEMORY)
+		 return NS_ERROR_OUT_OF_MEMORY;

Just let it bail out later -- maybe this will happen in the arena_allocate
(maybe). 

drop the ws change:
-	 //*/
+	 // */

with that, r=
Attachment #142477 - Flags: review?(dougt) → review+
This patch caused a significant leak increase on tinderbox.
From lhasa tinderbox log:

--NEW-LEAKS-----------------------------------leaks------leaks%-----------------------
nsMemoryImpl                                     68          -
GlobalWindowImpl                                684          -
nsSecretDecoderRing                              12          -
nsXPCComponents                                 240          -
nsLocalFile                                     116          -
XPCWrappedNative                                396          -
nsVoidArray                                      24          -
nsPrefBranch                                     60          -
nsHashtable                                      44          -
NavigatorImpl                                    48          -
nsDOMClassInfo                                  112          -
XPCNativeScriptableInfo                          72          -
nsPrefService                                    40          -
XPCNativeScriptableShared                       608          -
nsEntropyCollector                             1044          -
nsDebugImpl                                       8          -
AtomImpl                                         12          -
nsSharedPrefHandler                              84          -
nsPrincipal                                     116          -
nsSimpleURI                                       4          -
XPCWrappedNativeProto                           252          -
nsSystemPrincipal                                28          -
From brad tinderbox log:

--NEW-LEAKS-----------------------------------leaks------leaks%-----------------------
GlobalWindowImpl                                684          -
nsSystemPrincipal                                28          -
nsDOMClassInfo                                  128          -
nsPrincipal                                     232          -
NavigatorImpl                                    48          -
nsPrefService                                    40          -
BarPropImpl                                      12          -
nsPrefBranch                                     60          -
nsBaseURLParser                                   8          -
nsHashtable                                      88          -
nsSharedPrefHandler                              84          -
nsVoidArray                                      32          -
nsEntropyCollector                             1044          -
nsStandardURL                                   192          -
XPCNativeScriptableInfo                          72          -
nsLocalFile                                     116          -
nsSimpleURI                                       4          -
XPCWrappedNativeProto                           252          -
XPCWrappedNative                                396     800.00%
XPCNativeScriptableShared                       684     800.00%
nsXPCComponents                                 240     400.00%

(Note that the trace-malloc based leak stats show about a 100K increase.)
This patch is also likely responsible for bug 236903
Comment on attachment 143444 [details] [diff] [review]
fix leak regression 

verbal r=caillon sr=jst
Attachment #143444 - Flags: superreview+
Attachment #143444 - Flags: review+
this patch broke the JS components. JS components can no longer be registered
with the component manager.

Backing out this checkin fixes the regression.

See Bug #236952 for some of the side effects. this needs fixed before 1.7b


I think we should back out both patches and try again later.
thanks doug. I just backed this out. 
So dbaron's patch did not fix the regressions?  In addition to fixing leaks it
also made the code actually work, if you look at it.  It certainly fixes bug
236903 (once you recreate compreg.dat after applying that patch).
Blocks: 236903, 236950, 236952
I am not sure, but I do not want to have to have people be blocked for any
length of time while we are unsure about the regression.  This bug isn't that
important.
dbaron's patch did indeed fix the regression we were seeing. So I put the
changes back in.
final versions:
mozilla/xpcom/components/nsComponentManager.cpp 	1.248
mozilla/xpcom/components/nsComponentManager.h 	1.96
Status: NEW → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
*** Bug 236993 has been marked as a duplicate of this bug. ***
*** Bug 237036 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: