diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java index ec360f9dcca..8254e6887e1 100644 --- a/bundles/org.eclipse.jface/src/org/eclipse/jface/resource/FontRegistry.java +++ b/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 null */ - 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 getAllocatedFonts() { + List 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 Display * @param cleanOnDisplayDisposal * whether all fonts allocated by this FontRegistry - * should be disposed when the display is disposed + * should be disposed when the display is disposed. If + * false, 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 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 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); } /** diff --git a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java b/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java index 7aa7f7dfcd0..93f8cd5808c 100644 --- a/tests/org.eclipse.jface.tests/src/org/eclipse/jface/tests/resources/FontRegistryTest.java +++ b/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 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");