From f17830879fc04c12e5a152806d5a5eaeb95feea1 Mon Sep 17 00:00:00 2001 From: Dmitry Jemerov Date: Wed, 13 Nov 2019 19:14:15 +0100 Subject: [PATCH] To avoid memory leaks on plugin unload, don't store theme references in Appearance combo box (IDEA-224200) GitOrigin-RevId: c878df71fe63f44796f7c9c2f0125a8faba20922 --- .../src/com/intellij/ide/ui/LafManager.java | 49 +++++++++++- .../ide/ui/AppearanceConfigurable.java | 14 ++-- .../ide/ui/laf/HeadlessLafManagerImpl.java | 12 ++- .../intellij/ide/ui/laf/LafManagerImpl.java | 76 +++++++++++++------ 4 files changed, 117 insertions(+), 34 deletions(-) diff --git a/platform/platform-api/src/com/intellij/ide/ui/LafManager.java b/platform/platform-api/src/com/intellij/ide/ui/LafManager.java index 65e249b27441..fca01853863a 100644 --- a/platform/platform-api/src/com/intellij/ide/ui/LafManager.java +++ b/platform/platform-api/src/com/intellij/ide/ui/LafManager.java @@ -9,6 +9,7 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import javax.swing.*; +import java.util.Objects; public abstract class LafManager { public static LafManager getInstance() { @@ -19,11 +20,17 @@ public abstract class LafManager { public abstract UIManager.LookAndFeelInfo[] getInstalledLookAndFeels(); @ApiStatus.Internal - public abstract CollectionComboBoxModel getLafComboBoxModel(); + public abstract CollectionComboBoxModel getLafComboBoxModel(); + + @ApiStatus.Internal + public abstract UIManager.LookAndFeelInfo findLaf(LafReference reference); @Nullable public abstract UIManager.LookAndFeelInfo getCurrentLookAndFeel(); + @ApiStatus.Internal + public abstract LafReference getCurrentLookAndFeelReference(); + public abstract void setCurrentLookAndFeel(@NotNull UIManager.LookAndFeelInfo lookAndFeelInfo); public abstract void updateUI(); @@ -47,4 +54,44 @@ public abstract class LafManager { */ @Deprecated public abstract void removeLafManagerListener(@NotNull LafManagerListener listener); + + public static class LafReference { + private final String name; + private final String className; + private final String themeId; + + public LafReference(@NotNull String name, @NotNull String className, @Nullable String themeId) { + this.name = name; + this.className = className; + this.themeId = themeId; + } + + @Override + public String toString() { + return name; + } + + public String getClassName() { + return className; + } + + public String getThemeId() { + return themeId; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + LafReference reference = (LafReference)o; + return name.equals(reference.name) && + className.equals(reference.className) && + Objects.equals(themeId, reference.themeId); + } + + @Override + public int hashCode() { + return Objects.hash(name, className, themeId); + } + } } \ No newline at end of file diff --git a/platform/platform-impl/src/com/intellij/ide/ui/AppearanceConfigurable.java b/platform/platform-impl/src/com/intellij/ide/ui/AppearanceConfigurable.java index a673079316e4..4e80d0e83e5d 100644 --- a/platform/platform-impl/src/com/intellij/ide/ui/AppearanceConfigurable.java +++ b/platform/platform-impl/src/com/intellij/ide/ui/AppearanceConfigurable.java @@ -4,7 +4,6 @@ package com.intellij.ide.ui; import com.intellij.ide.GeneralSettings; import com.intellij.ide.IdeBundle; import com.intellij.ide.actions.QuickChangeLookAndFeel; -import com.intellij.ide.ui.laf.LafManagerImpl; import com.intellij.openapi.actionSystem.ActionPlaces; import com.intellij.openapi.actionSystem.ex.ActionUtil; import com.intellij.openapi.editor.EditorFactory; @@ -29,7 +28,6 @@ import org.jetbrains.annotations.NotNull; import javax.swing.*; import java.awt.*; -import java.awt.event.ItemEvent; import java.util.Dictionary; import java.util.Hashtable; @@ -66,7 +64,6 @@ public class AppearanceConfigurable implements SearchableConfigurable { myComponent.myPresentationModeFontSize.setEditable(true); myComponent.myLafComboBox.setModel(LafManager.getInstance().getLafComboBoxModel()); - myComponent.myLafComboBox.setRenderer(SimpleListCellRenderer.create("", UIManager.LookAndFeelInfo::getName)); myComponent.myAntialiasingInIDE.setModel(new DefaultComboBoxModel(AntialiasingType.values())); myComponent.myAntialiasingInEditor.setModel(new DefaultComboBoxModel(AntialiasingType.values())); @@ -118,12 +115,14 @@ public class AppearanceConfigurable implements SearchableConfigurable { myComponent.myBackgroundImageButton.addActionListener(ActionUtil.createActionListener( "Images.SetBackgroundImage", myComponent.myPanel, ActionPlaces.UNKNOWN)); + /* updateDarkWindowHeaderVisibility((UIManager.LookAndFeelInfo)myComponent.myLafComboBox.getSelectedItem()); myComponent.myLafComboBox.addItemListener(itemEvent -> { if (itemEvent.getStateChange() == ItemEvent.SELECTED) { updateDarkWindowHeaderVisibility((UIManager.LookAndFeelInfo)itemEvent.getItem()); } }); + */ return myComponent.myPanel; } @@ -241,7 +240,8 @@ public class AppearanceConfigurable implements SearchableConfigurable { settings.setCompactTreeIndents(myComponent.myCompactTreeIndents.isSelected()); if (!Comparing.equal(myComponent.myLafComboBox.getSelectedItem(), lafManager.getCurrentLookAndFeel())) { - UIManager.LookAndFeelInfo lafInfo = (UIManager.LookAndFeelInfo)myComponent.myLafComboBox.getSelectedItem(); + LafManager.LafReference lafReference = (LafManager.LafReference)myComponent.myLafComboBox.getSelectedItem(); + UIManager.LookAndFeelInfo lafInfo = LafManager.getInstance().findLaf(lafReference); update = true; shouldUpdateUI = false; QuickChangeLookAndFeel.switchLafAndUpdateUI(lafManager, lafInfo, true); @@ -350,7 +350,7 @@ public class AppearanceConfigurable implements SearchableConfigurable { myComponent.myMoveMouseOnDefaultButtonCheckBox.setSelected(settings.getMoveMouseOnDefaultButton()); myComponent.myHideNavigationPopupsCheckBox.setSelected(settings.getHideNavigationOnFocusLoss()); myComponent.myAltDNDCheckBox.setSelected(settings.getDndWithPressedAltOnly()); - myComponent.myLafComboBox.setSelectedItem(LafManager.getInstance().getCurrentLookAndFeel()); + myComponent.myLafComboBox.setSelectedItem(LafManager.getInstance().getCurrentLookAndFeelReference()); myComponent.myOverrideLAFFonts.setSelected(settings.getOverrideLafFonts()); myComponent.myDisableMnemonics.setSelected(settings.getDisableMnemonics()); myComponent.myWidescreenLayoutCheckBox.setSelected(settings.getWideScreenSupport()); @@ -434,7 +434,7 @@ public class AppearanceConfigurable implements SearchableConfigurable { isModified |= myComponent.myMoveMouseOnDefaultButtonCheckBox.isSelected() != settings.getMoveMouseOnDefaultButton(); isModified |= myComponent.myHideNavigationPopupsCheckBox.isSelected() != settings.getHideNavigationOnFocusLoss(); isModified |= myComponent.myAltDNDCheckBox.isSelected() != settings.getDndWithPressedAltOnly(); - isModified |= !Comparing.equal(myComponent.myLafComboBox.getSelectedItem(), LafManager.getInstance().getCurrentLookAndFeel()); + isModified |= !Comparing.equal(myComponent.myLafComboBox.getSelectedItem(), LafManager.getInstance().getCurrentLookAndFeelReference()); if (WindowManagerEx.getInstanceEx().isAlphaModeSupported()) { isModified |= myComponent.myEnableAlphaModeCheckBox.isSelected() != settings.getEnableAlphaMode(); int delay = -1; @@ -473,7 +473,7 @@ public class AppearanceConfigurable implements SearchableConfigurable { private JCheckBox myWindowShortcutsCheckBox; private JCheckBox myShowToolStripesCheckBox; private JCheckBox myShowMemoryIndicatorCheckBox; - private JComboBox myLafComboBox; + private JComboBox myLafComboBox; private JCheckBox myCycleScrollingCheckBox; private JCheckBox myMoveMouseOnDefaultButtonCheckBox; diff --git a/platform/platform-impl/src/com/intellij/ide/ui/laf/HeadlessLafManagerImpl.java b/platform/platform-impl/src/com/intellij/ide/ui/laf/HeadlessLafManagerImpl.java index ba557cff9ed9..a80925301ffd 100644 --- a/platform/platform-impl/src/com/intellij/ide/ui/laf/HeadlessLafManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/ide/ui/laf/HeadlessLafManagerImpl.java @@ -26,10 +26,20 @@ public class HeadlessLafManagerImpl extends LafManager { } @Override - public CollectionComboBoxModel getLafComboBoxModel() { + public LafReference getCurrentLookAndFeelReference() { + return null; + } + + @Override + public CollectionComboBoxModel getLafComboBoxModel() { return new CollectionComboBoxModel<>(); } + @Override + public UIManager.LookAndFeelInfo findLaf(LafReference reference) { + return null; + } + @Override public void setCurrentLookAndFeel(@NotNull UIManager.LookAndFeelInfo lookAndFeelInfo) { } diff --git a/platform/platform-impl/src/com/intellij/ide/ui/laf/LafManagerImpl.java b/platform/platform-impl/src/com/intellij/ide/ui/laf/LafManagerImpl.java index cad5f2c36f63..55241dcf3ef2 100644 --- a/platform/platform-impl/src/com/intellij/ide/ui/laf/LafManagerImpl.java +++ b/platform/platform-impl/src/com/intellij/ide/ui/laf/LafManagerImpl.java @@ -45,6 +45,7 @@ import com.intellij.util.IJSwingUtilities; import com.intellij.util.IconUtil; import com.intellij.util.ObjectUtils; import com.intellij.util.concurrency.SynchronizedClearableLazy; +import com.intellij.util.containers.ContainerUtil; import com.intellij.util.ui.*; import org.intellij.lang.annotations.JdkConstants; import org.jdom.Element; @@ -105,7 +106,7 @@ public final class LafManagerImpl extends LafManager implements PersistentStateC private static final Map ourLafClassesAliases = new HashMap<>(); - private CollectionComboBoxModel myLafComboBoxModel; + private CollectionComboBoxModel myLafComboBoxModel; static { ourLafClassesAliases.put("idea.dark.laf.classname", DarculaLookAndFeelInfo.CLASS_NAME); @@ -231,13 +232,13 @@ public final class LafManagerImpl extends LafManager implements PersistentStateC sortThemesIfNecessary(newLaFs); myLaFs.setValue(newLaFs); if (myLafComboBoxModel != null) { - myLafComboBoxModel.replaceAll(newLaFs); + myLafComboBoxModel.replaceAll(getLafReferences()); } // When updating a theme plugin that doesn't provide the current theme, don't select any of its themes as current if (!myThemesInUpdatedPlugin.contains(theme.getId())) { setCurrentLookAndFeel(newTheme); if (myLafComboBoxModel != null) { - myLafComboBoxModel.setSelectedItem(newTheme); + myLafComboBoxModel.setSelectedItem(createLafReference(newTheme)); } JBColor.setDark(newTheme.getTheme().isDark()); updateUI(); @@ -266,12 +267,12 @@ public final class LafManagerImpl extends LafManager implements PersistentStateC } myLaFs.setValue(list); if (myLafComboBoxModel != null) { - myLafComboBoxModel.replaceAll(list); + myLafComboBoxModel.replaceAll(getLafReferences()); } if (switchLafTo != null) { setCurrentLookAndFeel(switchLafTo, true); if (myLafComboBoxModel != null) { - myLafComboBoxModel.setSelectedItem(switchLafTo); + myLafComboBoxModel.setSelectedItem(createLafReference(switchLafTo)); } JBColor.setDark(switchLafTo == myDefaultDarkTheme); updateUI(); @@ -312,29 +313,12 @@ public final class LafManagerImpl extends LafManager implements PersistentStateC @Override public void loadState(@NotNull Element element) { - String className = null; UIManager.LookAndFeelInfo laf = null; Element lafElement = element.getChild(ELEMENT_LAF); if (lafElement != null) { - className = lafElement.getAttributeValue(ATTRIBUTE_CLASS_NAME); - if (className != null && ourLafClassesAliases.containsKey(className)) { - className = ourLafClassesAliases.get(className); - } - - String themeId = lafElement.getAttributeValue(ATTRIBUTE_THEME_NAME); - if (themeId != null) { - for (UIManager.LookAndFeelInfo l : myLaFs.getValue()) { - if (l instanceof UIThemeBasedLookAndFeelInfo && ((UIThemeBasedLookAndFeelInfo)l).getTheme().getId().equals(themeId)) { - laf = l; - break; - } - } - } + laf = findLaf(lafElement.getAttributeValue(ATTRIBUTE_CLASS_NAME), lafElement.getAttributeValue(ATTRIBUTE_THEME_NAME)); } - if (laf == null && className != null) { - laf = findLaf(className); - } // If LAF is undefined (wrong class name or something else) we have set default LAF anyway. if (laf == null) { laf = getDefaultLaf(); @@ -343,6 +327,25 @@ public final class LafManagerImpl extends LafManager implements PersistentStateC myCurrentLaf = laf; } + @Nullable + private UIManager.LookAndFeelInfo findLaf(String lafClassName, String themeId) { + if (lafClassName != null && ourLafClassesAliases.containsKey(lafClassName)) { + lafClassName = ourLafClassesAliases.get(lafClassName); + } + + if (themeId != null) { + for (UIManager.LookAndFeelInfo l : myLaFs.getValue()) { + if (l instanceof UIThemeBasedLookAndFeelInfo && ((UIThemeBasedLookAndFeelInfo)l).getTheme().getId().equals(themeId)) { + return l; + } + } + } + if (lafClassName != null) { + return findLaf(lafClassName); + } + return null; + } + @Override public void noStateLoaded() { myCurrentLaf = getDefaultLaf(); @@ -377,18 +380,41 @@ public final class LafManagerImpl extends LafManager implements PersistentStateC } @Override - public CollectionComboBoxModel getLafComboBoxModel() { + public CollectionComboBoxModel getLafComboBoxModel() { if (myLafComboBoxModel == null) { - myLafComboBoxModel = new CollectionComboBoxModel<>(myLaFs.getValue()); + myLafComboBoxModel = new CollectionComboBoxModel<>(getLafReferences()); } return myLafComboBoxModel; } + private List getLafReferences() { + return ContainerUtil.map(myLaFs.getValue(), LafManagerImpl::createLafReference); + } + + @NotNull + private static LafReference createLafReference(UIManager.LookAndFeelInfo laf) { + String themeId = null; + if (laf instanceof UIThemeBasedLookAndFeelInfo) { + themeId = ((UIThemeBasedLookAndFeelInfo) laf).getTheme().getId(); + } + return new LafReference(laf.getName(), laf.getClassName(), themeId); + } + + @Override + public UIManager.LookAndFeelInfo findLaf(LafReference reference) { + return findLaf(reference.getClassName(), reference.getThemeId()); + } + @Override public UIManager.LookAndFeelInfo getCurrentLookAndFeel() { return myCurrentLaf; } + @Override + public LafReference getCurrentLookAndFeelReference() { + return createLafReference(myCurrentLaf); + } + public UIManager.LookAndFeelInfo getDefaultLaf() { String wizardLafName = WelcomeWizardUtil.getWizardLAF(); if (wizardLafName != null) {