Target Explorer: Fix launch configuration change notifications fired while launch configuration was not updated

5 files changed, 45 insertions, 16 deletions

diff --git a/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/lm/LaunchConfigHelper.java b/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/lm/LaunchConfigHelper.java
index 084e96f3a..20eeabd3f 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/lm/LaunchConfigHelper.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/lm/LaunchConfigHelper.java
@@ -22,6 +22,7 @@ import org.eclipse.debug.core.ILaunchConfigurationType;
 import org.eclipse.debug.core.ILaunchConfigurationWorkingCopy;
 import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.debug.core.ILaunchMode;
+import org.eclipse.tcf.te.launch.core.persistence.DefaultPersistenceDelegate;
 
 /**
  * Static launch configuration utility implementations.
@@ -39,22 +40,22 @@ public class LaunchConfigHelper {
 
 	public static void addLaunchConfigAttribute(ILaunchConfigurationWorkingCopy wc, String key, Object value) {
 		if (value instanceof String) {
-			wc.setAttribute(key, (String)value);
+			DefaultPersistenceDelegate.setAttribute(wc, key, (String)value);
 		}
 		else if (value instanceof List) {
-			wc.setAttribute(key, (List<?>)value);
+			DefaultPersistenceDelegate.setAttribute(wc, key, (List<?>)value);
 		}
 		else if (value instanceof Map) {
-			wc.setAttribute(key, (Map<?,?>)value);
+			DefaultPersistenceDelegate.setAttribute(wc, key, (Map<?,?>)value);
 		}
 		else if (value instanceof Set) {
-			wc.setAttribute(key, (Set<?>)value);
+			DefaultPersistenceDelegate.setAttribute(wc, key, (Set<?>)value);
 		}
 		else if (value instanceof Boolean) {
-			wc.setAttribute(key, ((Boolean)value).booleanValue());
+			DefaultPersistenceDelegate.setAttribute(wc, key, ((Boolean)value).booleanValue());
 		}
 		else if (value instanceof Number) {
-			wc.setAttribute(key, ((Number)value).intValue());
+			DefaultPersistenceDelegate.setAttribute(wc, key, ((Number)value).intValue());
 		}
 		else {
 			throw new IllegalArgumentException("Unknown attribute type " + value.getClass().getName() + "(" + value.toString() + ")"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/persistence/DefaultPersistenceDelegate.java b/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/persistence/DefaultPersistenceDelegate.java
index 557920def..8b5454801 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/persistence/DefaultPersistenceDelegate.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.launch.core/src/org/eclipse/tcf/te/launch/core/persistence/DefaultPersistenceDelegate.java
@@ -209,6 +209,31 @@ public class DefaultPersistenceDelegate {
 	}
 
 	/**
+	 * Stores the given set attribute value under the given attribute id if the value
+	 * has changed compared to the already stored value under the given attribute id or
+	 * if the attribute id has not been stored yet. If the attribute value is <code>null</code>,
+	 * the attribute id will be removed from the given launch configuration working copy.
+	 *
+	 * @param wc The launch configuration working copy instance to apply the attribute to. Must not be <code>null</code>.
+	 * @param attributeId The attribute id to store the attribute value under. Must not be <code>null</code>.
+	 * @param attributeValue The attribute value to store under the given attribute id.
+	 */
+	public final static void setAttribute(ILaunchConfigurationWorkingCopy wc, String attributeId, Set<?> attributeValue) {
+		if (wc == null || attributeId == null) return;
+		if (isAttributeChanged(wc, attributeId, attributeValue)) {
+			// Determine the old attribute value
+			Object oldValue = null;
+			if (hasAttribute(wc, attributeId)) try { oldValue = wc.getAttributes().get(attributeId); } catch (CoreException e) { /* ignored on purpose */ }
+
+			// Set the new value to the launch configuration
+			wc.setAttribute(attributeId, attributeValue);
+
+			// And fire an notification event
+			EventManager.getInstance().fireEvent(new LaunchConfigurationChangedEvent(wc, attributeId, oldValue, attributeValue));
+		}
+	}
+
+	/**
 	 * Returns the boolean attribute stored under the given attribute name or
 	 * the default value if the attribute does not exist or the read failed.
 	 *
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/AttachLaunchManagerDelegate.java b/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/AttachLaunchManagerDelegate.java
index 6c7fee00b..76505cbf2 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/AttachLaunchManagerDelegate.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/AttachLaunchManagerDelegate.java
@@ -20,6 +20,7 @@ import org.eclipse.tcf.internal.debug.launch.TCFLaunchDelegate;
 import org.eclipse.tcf.te.launch.core.lm.delegates.DefaultLaunchManagerDelegate;
 import org.eclipse.tcf.te.launch.core.lm.interfaces.ILaunchContextLaunchAttributes;
 import org.eclipse.tcf.te.launch.core.lm.interfaces.ILaunchSpecification;
+import org.eclipse.tcf.te.launch.core.persistence.DefaultPersistenceDelegate;
 import org.eclipse.tcf.te.launch.core.persistence.launchcontext.LaunchContextsPersistenceDelegate;
 import org.eclipse.tcf.te.launch.core.selection.interfaces.IRemoteSelectionContext;
 import org.eclipse.tcf.te.launch.core.selection.interfaces.ISelectionContext;
@@ -53,8 +54,9 @@ public class AttachLaunchManagerDelegate extends DefaultLaunchManagerDelegate {
 	public void updateLaunchConfigAttributes(ILaunchConfigurationWorkingCopy wc, ILaunchSpecification launchSpec) {
 		super.updateLaunchConfigAttributes(wc, launchSpec);
 
-		wc.setAttribute(ITcfLaunchStepAttributes.ATTR_ATTACH_SERVICES, (List<?>)null);
-		wc.setAttribute(TCFLaunchDelegate.ATTR_DISCONNECT_ON_CTX_EXIT, false);
+		DefaultPersistenceDelegate.setAttribute(wc, ITcfLaunchStepAttributes.ATTR_ATTACH_SERVICES, (List<?>)null);
+		DefaultPersistenceDelegate.setAttribute(wc, TCFLaunchDelegate.ATTR_DISCONNECT_ON_CTX_EXIT, false);
+
 		copySpecToConfig(launchSpec, wc);
 
 		wc.rename(getDefaultLaunchName(wc));
@@ -67,8 +69,8 @@ public class AttachLaunchManagerDelegate extends DefaultLaunchManagerDelegate {
 	public void initLaunchConfigAttributes(ILaunchConfigurationWorkingCopy wc, ILaunchSpecification launchSpec) {
 		super.initLaunchConfigAttributes(wc, launchSpec);
 
-		wc.setAttribute(ITcfLaunchStepAttributes.ATTR_ATTACH_SERVICES, (List<?>)null);
-		wc.setAttribute(TCFLaunchDelegate.ATTR_DISCONNECT_ON_CTX_EXIT, false);
+		DefaultPersistenceDelegate.setAttribute(wc, ITcfLaunchStepAttributes.ATTR_ATTACH_SERVICES, (List<?>)null);
+		DefaultPersistenceDelegate.setAttribute(wc, TCFLaunchDelegate.ATTR_DISCONNECT_ON_CTX_EXIT, false);
 
 		copySpecToConfig(launchSpec, wc);
 
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/RemoteAppLaunchManagerDelegate.java b/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/RemoteAppLaunchManagerDelegate.java
index 9620acc70..9cdfba980 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/RemoteAppLaunchManagerDelegate.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.tcf.launch.core/src/org/eclipse/tcf/te/tcf/launch/core/lm/delegates/RemoteAppLaunchManagerDelegate.java
@@ -67,14 +67,14 @@ public class RemoteAppLaunchManagerDelegate extends DefaultLaunchManagerDelegate
 	public void initLaunchConfigAttributes(ILaunchConfigurationWorkingCopy wc, ILaunchSpecification launchSpec) {
 		super.initLaunchConfigAttributes(wc, launchSpec);
 
-		wc.setAttribute(IProcessesStepAttributes.ATTR_STOP_AT_MAIN, true);
-		wc.setAttribute(IProcessesStepAttributes.ATTR_ATTACH_CHILDREN, true);
+		DefaultPersistenceDelegate.setAttribute(wc, IProcessesStepAttributes.ATTR_STOP_AT_MAIN, true);
+		DefaultPersistenceDelegate.setAttribute(wc, IProcessesStepAttributes.ATTR_ATTACH_CHILDREN, true);
 		try {
-			wc.setAttribute(ILaunchConfiguration.ATTR_SOURCE_LOCATOR_MEMENTO, CdtUtils.getDefaultSourceLookupDirector().getMemento());
+			DefaultPersistenceDelegate.setAttribute(wc, ILaunchConfiguration.ATTR_SOURCE_LOCATOR_MEMENTO, CdtUtils.getDefaultSourceLookupDirector().getMemento());
 			IModelNode[] contexts = LaunchContextsPersistenceDelegate.getLaunchContexts(launchSpec);
 			if (contexts != null && contexts.length == 1) {
 				ILaunchConfiguration attachLaunch = (ILaunchConfiguration)Platform.getAdapterManager().getAdapter(contexts[0], ILaunchConfiguration.class);
-				wc.setAttribute("org.eclipse.tcf.debug.PathMap", attachLaunch.getAttribute("org.eclipse.tcf.debug.PathMap", (String)null)); //$NON-NLS-1$ //$NON-NLS-2$
+				DefaultPersistenceDelegate.setAttribute(wc, "org.eclipse.tcf.debug.PathMap", attachLaunch.getAttribute("org.eclipse.tcf.debug.PathMap", (String)null)); //$NON-NLS-1$ //$NON-NLS-2$
 			}
 		}
 		catch (Exception e) {
diff --git a/target_explorer/plugins/org.eclipse.tcf.te.tests/src/org/eclipse/tcf/te/tests/tcf/launch/TcfLaunchTests.java b/target_explorer/plugins/org.eclipse.tcf.te.tests/src/org/eclipse/tcf/te/tests/tcf/launch/TcfLaunchTests.java
index a08e9c631..c09e1706f 100644
--- a/target_explorer/plugins/org.eclipse.tcf.te.tests/src/org/eclipse/tcf/te/tests/tcf/launch/TcfLaunchTests.java
+++ b/target_explorer/plugins/org.eclipse.tcf.te.tests/src/org/eclipse/tcf/te/tests/tcf/launch/TcfLaunchTests.java
@@ -22,6 +22,7 @@ import org.eclipse.debug.core.ILaunchManager;
 import org.eclipse.tcf.te.launch.core.lm.LaunchManager;
 import org.eclipse.tcf.te.launch.core.lm.LaunchSpecification;
 import org.eclipse.tcf.te.launch.core.lm.interfaces.ILaunchSpecification;
+import org.eclipse.tcf.te.launch.core.persistence.DefaultPersistenceDelegate;
 import org.eclipse.tcf.te.launch.core.persistence.filetransfer.FileTransfersPersistenceDelegate;
 import org.eclipse.tcf.te.launch.core.persistence.launchcontext.LaunchContextsPersistenceDelegate;
 import org.eclipse.tcf.te.runtime.model.interfaces.IModelNode;
@@ -102,8 +103,8 @@ public class TcfLaunchTests extends TcfTestCase {
 		try {
 			config = LaunchManager.getInstance().getLaunchConfiguration(spec, true);
 			ILaunchConfigurationWorkingCopy wc = config.getWorkingCopy();
-			wc.setAttribute("org.eclipse.debug.ui.ATTR_CONSOLE_OUTPUT_ON", false); //$NON-NLS-1$
-			wc.setAttribute("org.eclipse.debug.ui.ATTR_CAPTURE_IN_FILE", outFile.toOSString()); //$NON-NLS-1$
+			DefaultPersistenceDelegate.setAttribute(wc, "org.eclipse.debug.ui.ATTR_CONSOLE_OUTPUT_ON", false); //$NON-NLS-1$
+			DefaultPersistenceDelegate.setAttribute(wc, "org.eclipse.debug.ui.ATTR_CAPTURE_IN_FILE", outFile.toOSString()); //$NON-NLS-1$
 			config = wc.doSave();
 		}
 		catch (Exception e) {