Clean caches upon deletion of a Target Definition #1246

TargetPlatformHelper and TargetPlatformService keep separate caches for
target definition, resulting in a leak of TargetDefinition objects.

The leak happens when TargetDefinition.resolve() caches data in
TargetPlatformHelper.

This also allows to skip the explicit removal of a target selected for
removal from the TargetPlatformHelper's cache in
TargetPlatformPreferencePage.handleRemove() because the removal will
done when the change is applied and TargetPlatformService.deleteTarget()
is then called.

Fixes #1246

ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/target/TargetPlatformService.java

@@ -74,6 +74,7 @@
 import org.eclipse.pde.internal.core.PDECore;
 import org.eclipse.pde.internal.core.PDEPreferencesManager;
 import org.eclipse.pde.internal.core.TargetDefinitionManager;
+import org.eclipse.pde.internal.core.TargetPlatformHelper;
 import org.osgi.service.prefs.BackingStoreException;
 
 /**
@@ -156,6 +157,7 @@ public void deleteTarget(ITargetHandle handle) throws CoreException {
 			fExtTargetHandles.remove(((ExternalFileTargetHandle) handle).getLocation());
 		}
 		((AbstractTargetHandle) handle).delete();
+		TargetPlatformHelper.getTargetDefinitionMap().remove(handle);
 	}
 
 	@Override

ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/target/LocalTargetDefinitionTests.java

@@ -765,6 +765,22 @@ public void testArguments() throws Exception {
 
 	}
 
+	/**
+	 * Test for https://github.com/eclipse-pde/eclipse.pde/issues/1246
+	 */
+	@Test
+	public void testDeleteCleansCaches() throws Exception {
+		ITargetDefinition definition = getNewTarget();
+		try {
+			assertFalse(TargetPlatformHelper.getTargetDefinitionMap().containsKey(definition.getHandle()));
+			definition.resolve(null);
+			assertTrue(TargetPlatformHelper.getTargetDefinitionMap().containsKey(definition.getHandle()));
+		} finally {
+			getTargetService().deleteTarget(definition.getHandle());
+			assertFalse(TargetPlatformHelper.getTargetDefinitionMap().containsKey(definition.getHandle()));
+		}
+	}
+
 	/**
 	 * Tests that a single (lower) version of a bundle can be included in the
 	 * target platform.

ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/preferences/TargetPlatformPreferencePage.java

@@ -744,9 +744,6 @@ private void handleRemove() {
 			}
 			fRemoved.addAll(selected);
 			fTargets.removeAll(selected);
-			for (ITargetDefinition element : selected) {
-				TargetPlatformHelper.getTargetDefinitionMap().remove(element.getHandle());
-			}
 			// Quick hack because the first refresh loses the checkedState, which is being used to bold the active target
 			fTableViewer.refresh(false);
 			fTableViewer.refresh(true);