Move long running operations to the activation of the PluginsTab

- Inline value and delete some outdated JavaDoc - Skip calls to AbstractpluginBlock::performApply before the block is even created to avoid NPEs Contributes to eclipse-pde#1232 Co-authored-by: Hannes Wellmann <[email protected]>
fedejeanne · Apr 19, 2024 · d480bdb · d480bdb
1 parent a90e738
commit d480bdb
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 27 deletions.
diff --git a/org.eclipse.pde.doc.user/META-INF/MANIFEST.MF b/org.eclipse.pde.doc.user/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.pde.doc.user; singleton:=true
-Bundle-Version: 3.15.100.qualifier
+Bundle-Version: 3.15.200.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.help;bundle-version="[3.2.0,4.0.0)"
diff --git a/org.eclipse.pde.doc.user/pom.xml b/org.eclipse.pde.doc.user/pom.xml
@@ -17,7 +17,7 @@
     <version>4.32.0-SNAPSHOT</version>
   </parent>
   <artifactId>org.eclipse.pde.doc.user</artifactId>
-  <version>3.15.100-SNAPSHOT</version>
+  <version>3.15.200-SNAPSHOT</version>
   <packaging>eclipse-plugin</packaging>
 
   <properties>

diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/AbstractPluginBlock.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/AbstractPluginBlock.java
@@ -151,6 +151,8 @@ public abstract class AbstractPluginBlock {
 
 	private PluginStatusDialog fDialog;
 
+	private boolean fControlCreated;
+
 	class PluginModelNameBuffer {
 		private final Set<String> nameSet;
 
@@ -458,6 +460,8 @@ public void createControl(Composite parent, int span, int indent) {
 
 		SWTUtil.setButtonDimensionHint(fValidateButton);
 		fValidateButton.addSelectionListener(fListener);
+
+		fControlCreated = true;
 	}
 
 	private Button createButton(Composite parent, int span, int indent, String text) {
@@ -943,6 +947,12 @@ private int countChecked(Object[] elements) {
 	}
 
 	public void performApply(ILaunchConfigurationWorkingCopy config) {
+		if (!fControlCreated) {
+			// This method is also being called before the proper UI elements
+			// are created, which might cause a NPE
+			return;
+		}
+
 		if (fAutoIncludeRequirementsButtonChanged) {
 			boolean includeRequirements = fAutoIncludeRequirementsButton.getSelection();
 			config.setAttribute(IPDELauncherConstants.AUTOMATIC_INCLUDE_REQUIREMENTS, includeRequirements);

diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/PluginsTab.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/ui/launcher/PluginsTab.java
@@ -28,6 +28,7 @@
 import org.eclipse.pde.internal.ui.launcher.PluginBlock;
 import org.eclipse.pde.launching.IPDELauncherConstants;
 import org.eclipse.swt.SWT;
+import org.eclipse.swt.custom.BusyIndicator;
 import org.eclipse.swt.events.ModifyEvent;
 import org.eclipse.swt.events.ModifyListener;
 import org.eclipse.swt.events.SelectionAdapter;
@@ -59,6 +60,7 @@ public class PluginsTab extends AbstractLauncherTab {
 	private Combo fDefaultAutoStart;
 	private Spinner fDefaultStartLevel;
 	private final Listener fListener;
+	private boolean fActivated;
 
 	private static final int DEFAULT_SELECTION = 0;
 	private static final int PLUGIN_SELECTION = 1;
@@ -156,24 +158,40 @@ public void createControl(Composite parent) {
 
 	@Override
 	public void initializeFrom(ILaunchConfiguration configuration) {
-		try {
-			int index = DEFAULT_SELECTION;
-			if (configuration.getAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false)) {
-				index = FEATURE_SELECTION;
-			} else if (!configuration.getAttribute(IPDELauncherConstants.USE_DEFAULT, true)) {
-				index = PLUGIN_SELECTION;
-			}
-			fSelectionCombo.select(index);
-			fBlock.setActiveBlock(index);
-			boolean custom = fSelectionCombo.getSelectionIndex() == PLUGIN_SELECTION;
-			fBlock.initializeFrom(configuration, custom);
-			boolean auto = configuration.getAttribute(IPDELauncherConstants.DEFAULT_AUTO_START, false);
-			fDefaultAutoStart.setText(Boolean.toString(auto));
-			int level = configuration.getAttribute(IPDELauncherConstants.DEFAULT_START_LEVEL, 4);
-			fDefaultStartLevel.setSelection(level);
-		} catch (CoreException e) {
-			PDEPlugin.log(e);
+		// Long-running initialization happens on first activation of this tab
+	}
+
+	@Override
+	public void activated(ILaunchConfigurationWorkingCopy configuration) {
+		if (fActivated) {
+			// Since this method can be expensive, only activate this tab once.
+			return;
 		}
+
+		BusyIndicator.showWhile(getControl().getDisplay(), () -> {
+			try {
+				int index = DEFAULT_SELECTION;
+				if (configuration.getAttribute(IPDELauncherConstants.USE_CUSTOM_FEATURES, false)) {
+					index = FEATURE_SELECTION;
+				} else if (!configuration.getAttribute(IPDELauncherConstants.USE_DEFAULT, true)) {
+					index = PLUGIN_SELECTION;
+				}
+				fSelectionCombo.select(index);
+				fBlock.setActiveBlock(index);
+				boolean custom = fSelectionCombo.getSelectionIndex() == PLUGIN_SELECTION;
+				fBlock.initializeFrom(configuration, custom);
+				boolean auto = configuration.getAttribute(IPDELauncherConstants.DEFAULT_AUTO_START, false);
+				fDefaultAutoStart.setText(Boolean.toString(auto));
+				int level = configuration.getAttribute(IPDELauncherConstants.DEFAULT_START_LEVEL, 4);
+				fDefaultStartLevel.setSelection(level);
+
+				// If everything ran smoothly, this tab is activated
+				fActivated = true;
+			} catch (CoreException e) {
+				PDEPlugin.log(e);
+			}
+		});
+
 	}
 
 	@Override
@@ -217,16 +235,9 @@ public Image getImage() {
 		return fImage;
 	}
 
-	/**
-	 * Validates the tab.  If the feature option is chosen, and the workspace is not correctly set up,
-	 * the error message is set.
-	 *
-	 * @see org.eclipse.pde.ui.launcher.AbstractLauncherTab#validateTab()
-	 */
 	@Override
 	public void validateTab() {
-		String errorMessage = null;
-		setErrorMessage(errorMessage);
+		setErrorMessage(null);
 	}
 
 	@Override