Dispose bold/italic fonts in FontRegistry upon display disposal

The FontRegistry implementation uses hooks to dispose registered fonts
upon disposal of the display they have been created for. This
implementation does, however, only cover the base fonts but not the
bold/italic versions of them. In case only a bold/italic font of a
record is created on a separate display, no dispose hook for that
display will be created, such that those fonts will not be disposed when
the display becomes disposed. In consequence, fonts for non-disposed
devices may be returned by the registry.

This change improves the FontRegistry implementation to create a display
dispose hook also upon creation of a bold/italic font.

Author: HeikoKlare

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

@@ -121,7 +121,8 @@ public Font getBoldFont() {
 			}
 
 			FontData[] boldData = getModifiedFontData(SWT.BOLD);
-			boldFont = new Font(Display.getCurrent(), boldData);
+			Display display = getDisplayAndHookForDisposal();
+			boldFont = new Font(display, boldData);
 			return boldFont;
 		}
 
@@ -157,7 +158,8 @@ public Font getItalicFont() {
 			}
 
 			FontData[] italicData = getModifiedFontData(SWT.ITALIC);
-			italicFont = new Font(Display.getCurrent(), italicData);
+			Display display = getDisplayAndHookForDisposal();
+			italicFont = new Font(display, italicData);
 			return italicFont;
 		}
 
@@ -489,13 +491,10 @@ else if (fonts.length == 0) {
 	 * @return FontRecord for the new Font or <code>null</code>.
 	 */
 	private FontRecord createFont(String symbolicName, FontData[] fonts) {
-		Display display = Display.getCurrent();
+		Display display = getDisplayAndHookForDisposal();
 		if (display == null) {
 			return null;
 		}
-		if (cleanOnDisplayDisposal && !displayDisposeHooked.contains(display)) {
-			hookDisplayDispose(display);
-		}
 
 		FontData[] validData = filterData(fonts, display);
 		if (validData.length == 0) {
@@ -509,6 +508,17 @@ private FontRecord createFont(String symbolicName, FontData[] fonts) {
 		return new FontRecord(newFont, validData);
 	}
 
+	private Display getDisplayAndHookForDisposal() {
+		Display display = Display.getCurrent();
+		if (display == null) {
+			return null;
+		}
+		if (cleanOnDisplayDisposal && !displayDisposeHooked.contains(display)) {
+			hookDisplayDispose(display);
+		}
+		return display;
+	}
+
 	/**
 	 * Calculates the default font and returns the result.
 	 * This method creates a font that must be disposed.

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

@@ -23,6 +23,7 @@
 import java.time.Duration;
 import java.time.Instant;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Supplier;
 
 import org.eclipse.core.runtime.Platform.OS;
 import org.eclipse.jface.resource.FontRegistry;
@@ -58,10 +59,34 @@ public void multipleDisplayDispose() {
 		assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
 
 		FontRegistry fontRegistry = new FontRegistry();
+		testMultipleDisplayDispose(fontRegistry::defaultFont);
+	}
+
+	@Test
+	public void multipleDisplayDispose_boldFont() {
+		assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
+
+		FontRegistry fontRegistry = new FontRegistry();
+		fontRegistry.get(JFaceResources.DEFAULT_FONT);
+		testMultipleDisplayDispose(() -> fontRegistry.getBold(JFaceResources.DEFAULT_FONT));
+	}
+
+	@Test
+	public void multipleDisplay_italicFont() {
+		assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
+
+		FontRegistry fontRegistry = new FontRegistry();
+		fontRegistry.get(JFaceResources.DEFAULT_FONT);
+		testMultipleDisplayDispose(() -> fontRegistry.getItalic(JFaceResources.DEFAULT_FONT));
+	}
+
+	private static void testMultipleDisplayDispose(Supplier<Font> fontSupplier) {
+		assumeTrue(OS.isWindows(), "multiple Display instance only allowed on Windows");
+
 		Display secondDisplay = initializeDisplayInSeparateThread();
-		Font fontOnSecondDisplay = secondDisplay.syncCall(fontRegistry::defaultFont);
+		Font fontOnSecondDisplay = secondDisplay.syncCall(fontSupplier::get);
 
-		Font fontOnThisDisplayBeforeSecondDisplayDispose = fontRegistry.defaultFont();
+		Font fontOnThisDisplayBeforeSecondDisplayDispose = fontSupplier.get();
 		Device displayOfFontOnSecondDisplay = fontOnSecondDisplay.getDevice();
 		// font registry returns same font for every display
 		assertEquals(secondDisplay, displayOfFontOnSecondDisplay);
@@ -70,7 +95,7 @@ public void multipleDisplayDispose() {
 		// after disposing font's display, registry should reinitialize the font
 		secondDisplay.syncExec(secondDisplay::dispose);
 		assertTrue(fontOnSecondDisplay.isDisposed());
-		Font fontOnThisDisplayAfterSecondDisplayDispose = fontRegistry.defaultFont();
+		Font fontOnThisDisplayAfterSecondDisplayDispose = fontSupplier.get();
 		assertNotEquals(fontOnThisDisplayAfterSecondDisplayDispose, fontOnSecondDisplay);
 	}