99144 - nsIFactory needs to be frozen

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Doug Turner (:dougt)]

Comment 1

[Chak Nanga]

Attachment: First cut at doc'ing this interface. Please review for accuracy...Thanks

Comment 2

[Doug Turner (:dougt)]

I am not sure I like the comment describing this interface.  How is this:

A class factory allows creation of nsISupports derived objects without
specifying a concrete base class.  

Comment 3

[Chak Nanga]

Attachment: Updated patch based on Doug's comments

Comment 4

[Doug Turner (:dougt)]

r=dougt.  rick, can you sr=.

Comment 5

[rpotts (gone)]

Comment on attachment 49250 [details] [diff] [review]
Updated patch based on Doug's comments

we need to change the references to 'NS_TRUE' and 'NS_FALSE' to PR_TRUE/FALSE .  These are the correct constants 
to use with PRBool.

Also, (minor nit) in the general description... i'd change '... nsISupports derived objects...' to 'nsISupports derived components...'

Comment 6

[rpotts (gone)]

Also, you might want to add that if an 'outer' object is supplied, but the
component is not aggregatable, then 'NS_ERROR_NO_AGGREGATION' is returned.

I don't know if you want to mention that this interface is binary compatable
with COM's IClassFactory (assuming that it still is :-) )...

Comment 7

[Doug Turner (:dougt)]

good catch.  chak, can you pop another patch?

Comment 8

[Chak Nanga]

Attachment: Updated patch based on Rick's comments

Comment 9

[rpotts (gone)]

Comment on attachment 49377 [details] [diff] [review]
Updated patch based on Rick's comments

sr=[e-mail]

looks good.

Comment 10

[Chak Nanga]

Changes just checked in. 

nsIFactory is now frozen

Comment 11

[Mike Shaver (:shaver emeritus)]

I just saw this bug when the update showed up in a notice on the meta-bug.

Why do we have nsIFactory::lockFactory?  How does it interact with
nsIModule::canUnload?

What is a "component", in the comment for createInstance?  I know what an object
is, but people use "component" to mean
  - binary DLL implementing some interfaces, 
  - thing that is registered for a contract ID,
  - XPI-bundle of component[1]s and other resources
  - service (long-lived, singleton object)

I think it's confusing to say that the method "creates an instance of a
component", and then say that it returns a "component".

Comment 12

[Doug Turner (:dougt)]

> Why do we have nsIFactory::lockFactory?  

So that a factory can be held in memory if we ever support dynamic component
unloading.  

> How does it interact with nsIModule::canUnload?

I would guess that there would be a high level of interaction between these two
calls.  If I locked the factory, I would expect nsIModule::canUnload to
return false.  

> What is a "component", in the comment for createInstance?  I know what an
object is, but people use "component" to mean

I agree, we can replace the word 'object' for 'component' everywhere in that file.  

Comment 13

[Mike Shaver (:shaver emeritus)]

Talk me through needing to lock the factory in memory.  Who needs that?

Comment 14

[rpotts (gone)]

nsIFactory is another one of those 'COM compatable' interfaces :-)

originally, LockFactory(...) was present because IClassFactory has it too.

-- rick