Fix #364 - Use image registries

.github/copilot-instructions.md

@@ -301,14 +301,33 @@ CompletableFuture.supplyAsync(() -> {
 
 #### Resource Management
 
-**When** creating SWT resources (fonts, images) → dispose them when done; Colors do not need disposal in modern SWT
+**When** accessing SWT resources (fonts, images) → prefer accessing them via a plug-in's registry; in that case never dispose them
+
+**When** accessing SWT resources via a registry is not possible → you must dispose the resources you create
+yourself: `Image`, `GC`, `Cursor`, `Region`, `Path`, `Pattern`. `Color` and `Font` are auto-disposed in recent
+SWT. Never dispose system colors/fonts, registry-owned images/fonts, or `Display`/`Shell` (framework-managed)
+
+**When** designing a registry → store `ImageDescriptor`s (not eagerly created `Image`s); the registry creates
+and caches the `Image` lazily on first access and disposes it at display shutdown
 
 **When** using shared resources → do not dispose resources from other bundles
 
 **When** child has a parent → resources with a defined parent (e.g., Label with parent Composite) do not need explicit disposal
 
 **When** using streams/files → use try-with-resources for Eclipse resources (IFile, IDocument)
 
+##### Image Registries
+
+A plug-in's images can be offered using an `ImageRegistry`. We have `CopilotImages` in 
+`com.microsoft.copilot.eclipse.ui` and `CopilotJobsImages` in `com.microsoft.copilot.eclipse.ui.jobs`.
+The registry loads each image once, caches it, and disposes it automatically when the workbench shuts down.
+
+**When** accessing *static* images, always prefer using image registries. Do NOT pass image path `"/icons/...png"`
+string literals around the code base.
+
+**When** disposing → do NOT dispose images obtained from a registry or from shared images; the registry owns
+them. Only dispose images you constructed yourself with `new Image(...)`.
+
 #### Error Handling
 
 **When** reporting errors → use Eclipse `IStatus` and `Status` objects

com.microsoft.copilot.eclipse.ui.jobs.test/.classpath

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<classpath>
+	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-17"/>
+	<classpathentry kind="con" path="org.eclipse.pde.core.requiredPlugins"/>
+	<classpathentry kind="src" path="src">
+		<attributes>
+			<attribute name="test" value="true"/>
+		</attributes>
+	</classpathentry>
+	<classpathentry kind="output" path="target/classes"/>
+</classpath>

com.microsoft.copilot.eclipse.ui.jobs.test/.project

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>com.microsoft.copilot.eclipse.ui.jobs.test</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+		<buildCommand>
+			<name>org.eclipse.jdt.core.javabuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.m2e.core.maven2Builder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.ManifestBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.SchemaBuilder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
+	</buildSpec>
+	<natures>
+		<nature>org.eclipse.m2e.core.maven2Nature</nature>
+		<nature>org.eclipse.pde.PluginNature</nature>
+		<nature>org.eclipse.jdt.core.javanature</nature>
+	</natures>
+</projectDescription>

com.microsoft.copilot.eclipse.ui.jobs.test/.settings/org.eclipse.jdt.core.prefs

@@ -0,0 +1,9 @@
+eclipse.preferences.version=1
+org.eclipse.jdt.core.compiler.codegen.targetPlatform=17
+org.eclipse.jdt.core.compiler.compliance=17
+org.eclipse.jdt.core.compiler.problem.assertIdentifier=error
+org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=disabled
+org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
+org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures=warning
+org.eclipse.jdt.core.compiler.release=enabled
+org.eclipse.jdt.core.compiler.source=17

com.microsoft.copilot.eclipse.ui.jobs.test/.settings/org.eclipse.m2e.core.prefs

@@ -0,0 +1,4 @@
+activeProfiles=
+eclipse.preferences.version=1
+resolveWorkspaceProjects=true
+version=1

com.microsoft.copilot.eclipse.ui.jobs.test/META-INF/MANIFEST.MF

@@ -0,0 +1,15 @@
+Manifest-Version: 1.0
+Bundle-ManifestVersion: 2
+Bundle-Name: com.microsoft.copilot.eclipse.ui.jobs.test
+Bundle-SymbolicName: com.microsoft.copilot.eclipse.ui.jobs.test;singleton:=true
+Bundle-Version: 0.20.0.qualifier
+Bundle-Vendor: GitHub Copilot
+Bundle-RequiredExecutionEnvironment: JavaSE-17
+Fragment-Host: com.microsoft.copilot.eclipse.ui.jobs
+Automatic-Module-Name: com.microsoft.copilot.eclipse.ui.jobs.test
+Import-Package: org.objenesis;version="[3.4.0,4.0.0)"
+Require-Bundle: junit-jupiter-api;bundle-version="5.10.1",
+ junit-jupiter-params;bundle-version="5.10.1",
+ org.mockito.mockito-core;bundle-version="5.14.2",
+ org.mockito.junit-jupiter;bundle-version="5.10.2",
+ com.microsoft.copilot.eclipse.ui.jobs;bundle-version="0.20.0"

com.microsoft.copilot.eclipse.ui.jobs.test/build.properties

@@ -0,0 +1,6 @@
+source.. = src
+output.. = target/classes
+bin.includes = META-INF/,\
+               .,\
+               fragment.xml
+

com.microsoft.copilot.eclipse.ui.jobs.test/fragment.xml

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<?eclipse version="3.4"?>
+<plugin>
+
+</plugin>

com.microsoft.copilot.eclipse.ui.jobs.test/pom.xml

@@ -0,0 +1,54 @@
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+	<modelVersion>4.0.0</modelVersion>
+	<parent>
+		<groupId>com.microsoft.copilot.eclipse</groupId>
+		<artifactId>github-copilot-for-eclipse</artifactId>
+		<version>${copilot-plugin-version}</version>
+	</parent>
+	<artifactId>com.microsoft.copilot.eclipse.ui.jobs.test</artifactId>
+	<packaging>eclipse-test-plugin</packaging>
+	<name>${base.name} :: UI Jobs Tests</name>
+
+	<properties>
+		<checkstyle.skip>true</checkstyle.skip>
+	</properties>
+
+	<profiles>
+		<profile>
+			<id>skip-tests-during-ui-probe</id>
+			<activation>
+				<property>
+					<name>probe.script</name>
+				</property>
+			</activation>
+			<build>
+				<plugins>
+					<plugin>
+						<groupId>org.eclipse.tycho</groupId>
+						<artifactId>tycho-surefire-plugin</artifactId>
+						<version>${tycho-version}</version>
+						<configuration>
+							<skipTests>true</skipTests>
+						</configuration>
+					</plugin>
+				</plugins>
+			</build>
+		</profile>
+	</profiles>
+
+	<build>
+		<plugins>
+			<plugin>
+				<groupId>org.eclipse.tycho</groupId>
+				<artifactId>tycho-surefire-plugin</artifactId>
+				<version>${tycho-version}</version>
+				<configuration>
+					<useUIHarness>true</useUIHarness>
+					<useUIThread>false</useUIThread>
+				</configuration>
+			</plugin>
+		</plugins>
+	</build>
+</project>

com.microsoft.copilot.eclipse.ui.jobs.test/src/com/microsoft/copilot/eclipse/ui/jobs/CopilotJobsImagesTest.java

@@ -0,0 +1,41 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+package com.microsoft.copilot.eclipse.ui.jobs;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.util.stream.Stream;
+
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+/**
+ * Verifies that every public constant in {@link CopilotJobsImages} refers to a bundle resource
+ * that actually exists on the classpath.
+ *
+ * <p>New constants are discovered automatically via reflection; no separate list needs to be
+ * maintained.
+ */
+class CopilotJobsImagesTest {
+
+  static Stream<Arguments> iconPaths() {
+    return Stream.of(CopilotJobsImages.class.getDeclaredFields())
+        .filter(f -> f.getName().startsWith("IMG_"))
+        .map(f -> {
+          try {
+            return Arguments.of(f.getName(), f.get(null));
+          } catch (IllegalAccessException e) {
+            throw new IllegalStateException("Cannot read constant: " + f.getName(), e);
+          }
+        });
+  }
+
+  @ParameterizedTest(name = "{0}")
+  @MethodSource("iconPaths")
+  void testIconFileExists(String fieldName, String path) {
+    assertNotNull(CopilotJobsImages.class.getResource("/" + path),
+        "Bundle resource not found for constant " + fieldName + ": " + path);
+  }
+}

com.microsoft.copilot.eclipse.ui.jobs/src/com/microsoft/copilot/eclipse/ui/jobs/CopilotJobs.java

@@ -3,10 +3,12 @@
 
 package com.microsoft.copilot.eclipse.ui.jobs;
 
+import org.eclipse.core.runtime.Assert;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.jface.resource.ImageRegistry;
 import org.eclipse.ui.plugin.AbstractUIPlugin;
 import org.osgi.framework.BundleContext;
 
@@ -17,11 +19,27 @@
  */
 public class CopilotJobs extends AbstractUIPlugin {
 
+  private static volatile CopilotJobs COPILOT_JOBS_PLUGIN;
+
+  /** Returns the shared plugin instance. */
+  public static CopilotJobs getPlugin() {
+    Assert.isNotNull(COPILOT_JOBS_PLUGIN);
+    return COPILOT_JOBS_PLUGIN;
+  }
+
+  @Override
+  protected void initializeImageRegistry(ImageRegistry registry) {
+    CopilotJobsImages.initialize(registry);
+  }
+
   @Override
   public void start(BundleContext context) throws Exception {
     super.start(context);
+    COPILOT_JOBS_PLUGIN = this;
 
+    // Explicitly call method from core to ensure core is activated
     CopilotCore.getPlugin();
+
     Job initWaitJob = new Job("Waiting for Copilot initialization") {
       @Override
       protected IStatus run(IProgressMonitor monitor) {
@@ -42,5 +60,6 @@ protected IStatus run(IProgressMonitor monitor) {
   @Override
   public void stop(BundleContext context) throws Exception {
     super.stop(context);
+    COPILOT_JOBS_PLUGIN = null;
   }
 }

com.microsoft.copilot.eclipse.ui.jobs/src/com/microsoft/copilot/eclipse/ui/jobs/CopilotJobsImages.java

@@ -0,0 +1,99 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+package com.microsoft.copilot.eclipse.ui.jobs;
+
+import java.net.URL;
+
+import org.eclipse.jface.resource.ImageDescriptor;
+import org.eclipse.jface.resource.ImageRegistry;
+import org.eclipse.swt.graphics.Image;
+import org.eclipse.ui.ISharedImages;
+import org.eclipse.ui.PlatformUI;
+
+/**
+ * Centralized access to all static icons in the Copilot Jobs UI bundle.
+ *
+ * <p>Bundle images are owned by the plugin's {@link ImageRegistry} and disposed automatically when
+ * the plugin stops. Callers must <em>not</em> dispose images returned by {@link #getImage(String)}.
+ */
+public final class CopilotJobsImages {
+
+  // Path prefixes
+  private static final String ICONS_ROOT = "icons/";
+  private static final String ICONS_STATUS = ICONS_ROOT + "status/";
+
+  // Icons
+  public static final String IMG_REPO = ICONS_ROOT + "repo.png";
+  public static final String IMG_INFORMATION = ICONS_ROOT + "information.png";
+  public static final String IMG_STATUS_LOADING = ICONS_STATUS + "loading.png";
+  public static final String IMG_STATUS_COMPLETE = ICONS_STATUS + "complete.png";
+
+  private CopilotJobsImages() {
+    // prevent instantiation
+  }
+
+  /**
+   * Returns the plugin's image registry.
+   */
+  static ImageRegistry getImageRegistry() {
+    return CopilotJobs.getPlugin().getImageRegistry();
+  }
+
+  /**
+   * Registers all static icon descriptors. Called once from
+   * {@link CopilotJobs#initializeImageRegistry(ImageRegistry)}.
+   */
+  static void initialize(ImageRegistry registry) {
+    register(registry, IMG_REPO);
+    register(registry, IMG_INFORMATION);
+    register(registry, IMG_STATUS_LOADING);
+    register(registry, IMG_STATUS_COMPLETE);
+  }
+
+  private static void register(ImageRegistry registry, String path) {
+    URL url = CopilotJobsImages.class.getResource("/" + path);
+    ImageDescriptor descriptor = url != null
+        ? ImageDescriptor.createFromURL(url) : ImageDescriptor.getMissingImageDescriptor();
+    registry.put(path, descriptor);
+  }
+
+  /**
+   * Returns the image for the given key. The returned image is owned by the plugin registry;
+   * callers must <em>not</em> dispose it.
+   *
+   * @param key one of the {@code IMG_*} constants defined in this class
+   */
+  public static Image getImage(String key) {
+    return getImageRegistry().get(key);
+  }
+
+  /**
+   * Returns the {@link ImageDescriptor} for the given key. Useful where a descriptor is required,
+   * e.g. for {@code Action.setImageDescriptor()}.
+   *
+   * @param key one of the {@code IMG_*} constants defined in this class
+   */
+  public static ImageDescriptor getImageDescriptor(String key) {
+    return getImageRegistry().getDescriptor(key);
+  }
+
+  /**
+   * Convenience access to Eclipse's workbench shared images.
+   *
+   * @param imageId a constant from {@link ISharedImages}, e.g. {@link ISharedImages#IMG_OBJS_WARN_TSK}
+   */
+  public static Image getSharedImage(String imageId) {
+    return PlatformUI.getWorkbench().getSharedImages().getImage(imageId);
+  }
+
+  /**
+   * Convenience access to Eclipse's workbench shared image descriptors.
+   *
+   * @param imageId a constant from {@link ISharedImages}
+   */
+  public static ImageDescriptor getSharedImageDescriptor(String imageId) {
+    return PlatformUI.getWorkbench().getSharedImages().getImageDescriptor(imageId);
+  }
+
+}