Bug 312025

Comment 1 Bozier jerome 2010-05-07 08:03:43 EDT

updating estimated time

here's my findings :
inside org.eclipse.hyades.ui.internal.extension.AssociationDescriptor

we make a call to  createImplementationClassInstance()

		Class cls = getImplementationClass();
		if(cls != null)
		{
			try
			{
				return cls.newInstance();
			}
			catch (Exception e)
			{
				CommonPlugin.logError(e);
			}
		}
			
		return null;

this code create an instance of a "class"
problem is that "getImplementationClass()" create another instance of the same class :

		if((extensionClass == null) && (configurationElement != null))
		{
			if(getValue(configurationElement, "class") != null)
			{
				try
				{
					extensionClass = configurationElement.createExecutableExtension("class").getClass();
				}
				catch (Exception e)
				{
					CommonPlugin.logError(e);
				}
			}
		}
			
		return extensionClass;

(instance is created inside "createExecutableExtension")

so, to make short :
. we create an instance
. we make a "getClass()" on it
. and on the result, we ask to create an instance
=> to open an editor, we created 2 different instance....

i will examine a bit who use theses method to rationalize a bit all of this....

Comment 2 Bozier jerome 2010-05-07 08:53:13 EDT

Created attachment 167461 [details]
patch

this patch fix the "double instance editor" problem

instead of calling "getImplementationClass", it make the same kind code if no extension class is defined, and return the instance used to calculate the "Class" instead of creating a new one

Comment 3 Bozier jerome 2010-05-07 08:56:49 EDT

updating worked hours

Paul, could you review it please ? 
many thanks in advance

to note : 
this patch is on tptp.platform.common.ui plugin, inside src-common-internal part.
I will make a test case as soon as possible but i won't be able to commit the patch itself

Comment 4 Paul Slauenwhite 2010-05-07 09:51:05 EDT

Reviewed and approved with comments:

-We still need org.eclipse.hyades.ui.internal.extension.AssociationDescriptor.getImplementationClass() since it is an API on the interface (poorly designed, in my opinion), so we should call it from org.eclipse.hyades.ui.internal.extension.AssociationDescriptor.createImplementationClassInstance(), but cache the instance at the class level when created in either method.

Comment 5 Bozier jerome 2010-05-07 10:32:27 EDT

Created attachment 167480 [details]
patch V2

this patch is previous one fixed following your review

Paul, could you please commit it under CVS ? i don't have the write access for this plugin

many thanks in advance

Comment 6 Paul Slauenwhite 2010-05-07 10:48:12 EDT

(In reply to comment #5)
> Created an attachment (id=167480) [details]
> patch V2
> 
> this patch is previous one fixed following your review
> 
> Paul, could you please commit it under CVS ? i don't have the write access for
> this plugin
> 
> many thanks in advance

The attached patch checked in to CVS (HEAD).

Note, we will test this scenario under defect 228262.

Comment 7 Bozier jerome 2010-05-17 09:11:40 EDT

fix verified, closing