Clean up FontRegistry's legacy collection handling

FontRegistry still handles its collections the way it did before
generics: explicit Iterators, casts through Object, and one variable
reused for two unrelated values. It also lets FontRecord reach into the
registry to retire its own fonts, mixing up who owns that decision.

Simplify all of that, and write down what cleanOnDisplayDisposal ==
false already promises.

No behavior change, other than put() now invalidating the replaced
record entirely before notifying listeners rather than partly after, so
the registry is consistent by the time they run. That listeners already
see the new font when notified was untested, so a test now covers it.

Assisted-by: Claude Opus 5 <noreply@anthropic.com>

Author: HeikoKlare

bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java

@@ -19,7 +19,6 @@
 import java.util.Collections;
 import java.util.Enumeration;
 import java.util.HashMap;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.MissingResourceException;
@@ -164,23 +163,22 @@ public Font getItalicFont() {
 		}
 
 		/**
-		 * Add any fonts that were allocated for this record to the
-		 * stale fonts. Anything that matches the default font will
-		 * be skipped.
-		 * @param defaultFont The system default.
+		 * Return all of the fonts allocated by the receiver, that is the base
+		 * font and whichever styled variants have been realized so far.
+		 * @return the allocated fonts, never <code>null</code>
 		 */
-		void addAllocatedFontsToStale(Font defaultFont) {
-			//Return all of the fonts allocated by the receiver.
-			//if any of them are the defaultFont then don't bother.
-			if (defaultFont != baseFont && baseFont != null) {
-				staleFonts.add(baseFont);
+		List<Font> getAllocatedFonts() {
+			List<Font> allocatedFonts = new ArrayList<>(3);
+			if (baseFont != null) {
+				allocatedFonts.add(baseFont);
 			}
-			if (defaultFont != boldFont && boldFont != null) {
-				staleFonts.add(boldFont);
+			if (boldFont != null) {
+				allocatedFonts.add(boldFont);
 			}
-			if (defaultFont != italicFont && italicFont != null) {
-				staleFonts.add(italicFont);
+			if (italicFont != null) {
+				allocatedFonts.add(italicFont);
 			}
+			return allocatedFonts;
 		}
 	}
 
@@ -368,7 +366,10 @@ public FontRegistry(Display display) {
 	 *            the <code>Display</code>
 	 * @param cleanOnDisplayDisposal
 	 *            whether all fonts allocated by this <code>FontRegistry</code>
-	 *            should be disposed when the display is disposed
+	 *            should be disposed when the display is disposed. If
+	 *            <code>false</code>, this registry never disposes a font by
+	 *            itself; the fonts it allocated are retained until
+	 *            {@link #clearCaches()} disposes them
 	 * @since 3.1
 	 */
 	public FontRegistry(Display display, boolean cleanOnDisplayDisposal) {
@@ -678,19 +679,19 @@ public Font getItalic(String symbolicName) {
 	 */
 	private FontRecord getFontRecord(String symbolicName) {
 		Assert.isNotNull(symbolicName);
-		Object result = stringToFontRecord.get(symbolicName);
-		if (result != null) {
-			return (FontRecord) result;
+		FontRecord existingRecord = stringToFontRecord.get(symbolicName);
+		if (existingRecord != null) {
+			return existingRecord;
 		}
 
-		result = stringToFontData.get(symbolicName);
+		FontData[] existingFontData = stringToFontData.get(symbolicName);
 
 		FontRecord fontRecord;
 
-		if (result == null) {
+		if (existingFontData == null) {
 			fontRecord = defaultFontRecord();
 		} else {
-			fontRecord = createFont(symbolicName, (FontData[]) result);
+			fontRecord = createFont(symbolicName, existingFontData);
 		}
 
 		if (fontRecord == null) {
@@ -719,31 +720,14 @@ public boolean hasValueFor(String fontKey) {
 
 	@Override
 	protected void clearCaches() {
-
-		Iterator<FontRecord> iterator = stringToFontRecord.values().iterator();
-		while (iterator.hasNext()) {
-			Object next = iterator.next();
-			((FontRecord) next).dispose();
-		}
-
-		disposeFonts(staleFonts.iterator());
+		stringToFontRecord.values().forEach(FontRecord::dispose);
 		stringToFontRecord.clear();
+		staleFonts.forEach(Font::dispose);
 		staleFonts.clear();
 
 		displayDisposeHooked.remove(Display.getCurrent());
 	}
 
-	/**
-	 * Dispose of all of the fonts in this iterator.
-	 * @param iterator over Collection of Font
-	 */
-	private void disposeFonts(Iterator<Font> iterator) {
-		while (iterator.hasNext()) {
-			Object next = iterator.next();
-			((Font) next).dispose();
-		}
-	}
-
 	/**
 	 * Hook a dispose listener on the SWT display.
 	 */
@@ -821,16 +805,26 @@ private void put(String symbolicName, FontData[] fontData, boolean update) {
 			return;
 		}
 
-		FontRecord oldFont = stringToFontRecord
-				.remove(symbolicName);
 		stringToFontData.put(symbolicName, fontData);
+		invalidate(symbolicName);
 		if (update) {
 			fireMappingChanged(symbolicName, existing, fontData);
 		}
+	}
 
-		if (oldFont != null) {
-			oldFont.addAllocatedFontsToStale(defaultFontRecord().getBaseFont());
+	/**
+	 * Drop the realized font record for the given symbolic name, if any, and
+	 * defer disposal of the fonts it had allocated until it is safe to dispose
+	 * them, since they may still be in use. The default font is kept, as it
+	 * stays in use under its own symbolic name.
+	 */
+	private void invalidate(String symbolicName) {
+		FontRecord replacedRecord = stringToFontRecord.remove(symbolicName);
+		if (replacedRecord == null) {
+			return;
 		}
+		Font defaultFont = defaultFontRecord().getBaseFont();
+		replacedRecord.getAllocatedFonts().stream().filter(font -> font != defaultFont).forEach(staleFonts::add);
 	}
 
 	/**

tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java

@@ -214,6 +214,22 @@ public void put_firesPropertyChangeOnlyWhenDataActuallyChanges() {
 		assertEquals(2, events.size());
 	}
 
+	@Test
+	public void put_notifiesListenersOnlyAfterTheNewFontIsInEffect() {
+		FontRegistry fontRegistry = new FontRegistry();
+		fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 12, SWT.NORMAL) });
+		Font originalFont = fontRegistry.get("myfont");
+
+		AtomicReference<Font> fontSeenByListener = new AtomicReference<>();
+		fontRegistry.addListener(event -> fontSeenByListener.set(fontRegistry.get("myfont")));
+
+		fontRegistry.put("myfont", new FontData[] { new FontData("Arial", 18, SWT.NORMAL) });
+
+		assertNotEquals(originalFont, fontSeenByListener.get(),
+				"a listener must not still see the replaced font when it is notified");
+		assertEquals(18, fontSeenByListener.get().getFontData()[0].getHeight());
+	}
+
 	@Test
 	public void put_withNewData_disposesOldFontOnlyOnDisplayDispose() {
 		assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");