Bug 153347

Attaching patch with test.  Includes an addNotify() method with similar semantics to that of Children.  Use case: 
simplifies logic for subclasses which would otherwise have to do this work in createKeys() but would be forced to test
if it was really the first call to createKeys().  There are already some examples of doing that in mobility and elsewhere.

Created attachment 73843 [details]
Patch

Note: Nd to update spec version and correct it in apichanges doc before applying patch.

While you're at it, please let ChildFactory.createNodesForKey return null rather than new Node[0], changing

return n == null ? new Node[0] : new Node[] { n };

to

return n == null ? null : new Node[] { n };

Probably nothing else needs to change besides Javadoc, since {Asynch,Synch}Children just uses it for
Children.Keys.createNodes, which already accepts null in place of new Node[0].

Why would you want null returned versus an empty array? I'm asking because if someones code is relying on that empty
array it will break now with a null pointer exception, and also by returning empty arrays code can be made simpler when
all you'll do is loop over it because you can just

Node[] nodes = ...
for(Node n: nodes){
   ...
}

or

for(int i=0;i<nodes.length;i++){
   ...
}

and you don't need a null check or anything. You do create an empty object to return when there really is nothing to do
however. What would the main gains be versus the other besides those I've mentioned?

Null is good here simply to avoid constructing a new array object, since that is a very common case, and this code is
performance-critical. The method is part of a protected interface so no else could be calling it. As I said, the return
value would only be used by Children.Keys, which already accepts null from createNodes for the same reason.

Thanks for the clarifications.

My request for createNodesForKey to be able to return null was not really related to this patch, I just thought it would
be good to bundle it in. Anyway, I will retroactively label that [JG01].


[JG02] The <summary> in apichanges.xml is bogus.


[JG03] I believe "@param T" is correct, not "@param <T>". (Not sure, because the javadoc tool does not seem to actually
display docs for type parameters anyway.)


[JG04] It looks like AsynchChildren.remoteNotify will call factory.removeNotify() twice. ChildFactoryTest.DetachableImpl
should check this: addNotify should first assertFalse(added) and removeNotify should first assertFalse(removed).


[JG05] Do you have any patches prepared showing .Detachable in use? In mobility.* or wherever?

> Do you have any patches prepared showing .Detachable in use? In mobility.* or wherever?

http://www.netbeans.org/nonav/issues/showattachment.cgi/73844/mproject.diff

> I believe "@param T" is correct, not "@param <T>"

Hmm, our source editor generates @param <T> FWIW;  doesn't mean it's correct :-/

The patch I'm about to attach should address all of the comments.  If there are no further objections, I'd like to
commit this in the next few days.

Created attachment 74150 [details]
Revised patch

Integrated in e24a1479f40c