diff --git a/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionMenu.java b/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionMenu.java index 255fb1adce28..c35e65bf7e7f 100644 --- a/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionMenu.java +++ b/platform/platform-impl/src/com/intellij/openapi/actionSystem/impl/ActionMenu.java @@ -1,5 +1,5 @@ /* - * Copyright 2000-2013 JetBrains s.r.o. + * Copyright 2000-2015 JetBrains s.r.o. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,7 +57,7 @@ public final class ActionMenu extends JMenu { private MenuItemSynchronizer myMenuItemSynchronizer; private StubItem myStubItem; // A PATCH!!! Do not remove this code, otherwise you will lose all keyboard navigation in JMenuBar. private final boolean myTopLevel; - private final Disposable myDisposable; + private Disposable myDisposable; public ActionMenu(final DataContext context, @NotNull final String place, @@ -84,11 +84,6 @@ public final class ActionMenu extends JMenu { if (UIUtil.isUnderIntelliJLaF()) { setOpaque(true); } - myDisposable = new Disposable() { - @Override - public void dispose() { - } - }; // Triggering initialization of private field "popupMenu" from JMenu with our own JBPopupMenu getPopupMenu(); @@ -98,6 +93,7 @@ public final class ActionMenu extends JMenu { myContext = context; } + @Override public void addNotify() { super.addNotify(); installSynchronizer(); @@ -115,7 +111,10 @@ public final class ActionMenu extends JMenu { public void removeNotify() { uninstallSynchronizer(); super.removeNotify(); - Disposer.dispose(myDisposable); + if (myDisposable != null) { + Disposer.dispose(myDisposable); + myDisposable = null; + } } private void uninstallSynchronizer() { @@ -126,7 +125,7 @@ public final class ActionMenu extends JMenu { } } - private JPopupMenu mySpecialMenu = null; + private JPopupMenu mySpecialMenu; @Override public JPopupMenu getPopupMenu() { if (mySpecialMenu == null) { @@ -184,7 +183,7 @@ public final class ActionMenu extends JMenu { } private void init() { - boolean macSystemMenu = SystemInfo.isMacSystemMenu && myPlace == ActionPlaces.MAIN_MENU; + boolean macSystemMenu = SystemInfo.isMacSystemMenu && myPlace.equals(ActionPlaces.MAIN_MENU); myStubItem = macSystemMenu ? null : new StubItem(); addStubItem(); @@ -252,25 +251,35 @@ public final class ActionMenu extends JMenu { } private class MenuListenerImpl implements MenuListener { + @Override public void menuCanceled(MenuEvent e) { clearItems(); addStubItem(); } + @Override public void menuDeselected(MenuEvent e) { - Disposer.dispose(myDisposable); + if (myDisposable != null) { + Disposer.dispose(myDisposable); + myDisposable = null; + } clearItems(); addStubItem(); } + @Override public void menuSelected(MenuEvent e) { - new UsabilityHelper(ActionMenu.this, myDisposable); + UsabilityHelper helper = new UsabilityHelper(ActionMenu.this); + if (myDisposable == null) { + myDisposable = Disposer.newDisposable(); + } + Disposer.register(myDisposable, helper); fillMenu(); } } private void clearItems() { - if (SystemInfo.isMacSystemMenu && myPlace == ActionPlaces.MAIN_MENU) { + if (SystemInfo.isMacSystemMenu && myPlace.equals(ActionPlaces.MAIN_MENU)) { for (Component menuComponent : getMenuComponents()) { if (menuComponent instanceof ActionMenu) { ((ActionMenu)menuComponent).clearItems(); @@ -315,6 +324,7 @@ public final class ActionMenu extends JMenu { } private class MenuItemSynchronizer implements PropertyChangeListener { + @Override public void propertyChange(PropertyChangeEvent e) { String name = e.getPropertyName(); if (Presentation.PROP_VISIBLE.equals(name)) { @@ -343,14 +353,13 @@ public final class ActionMenu extends JMenu { private static class UsabilityHelper implements IdeEventQueue.EventDispatcher, AWTEventListener, Disposable { private Component myComponent; - private Point myLastMousePoint = null; - private Point myUpperTargetPoint = null; - private Point myLowerTargetPoint = null; + private Point myLastMousePoint; + private Point myUpperTargetPoint; + private Point myLowerTargetPoint; private SingleAlarm myCallbackAlarm; - private MouseEvent myEventToRedispatch = null; + private MouseEvent myEventToRedispatch; - private UsabilityHelper(Component component, @NotNull Disposable disposable) { - Disposer.register(disposable, this); + private UsabilityHelper(Component component) { myCallbackAlarm = new SingleAlarm(new Runnable() { @Override public void run() { diff --git a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java index 6340aafb237a..9170919028a6 100644 --- a/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java +++ b/platform/util/src/com/intellij/openapi/util/objectTree/ObjectTree.java @@ -17,9 +17,11 @@ package com.intellij.openapi.util.objectTree; import com.intellij.openapi.Disposable; import com.intellij.openapi.diagnostic.Logger; +import com.intellij.openapi.util.Disposer; import com.intellij.util.ArrayUtil; import com.intellij.util.IncorrectOperationException; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.WeakHashMap; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -35,6 +37,7 @@ public final class ObjectTree { // identity used here to prevent problems with hashCode/equals overridden by not very bright minds private final Set myRootObjects = ContainerUtil.newIdentityTroveSet(); // guarded by treeLock private final Map> myObject2NodeMap = ContainerUtil.newIdentityTroveMap(); // guarded by treeLock + private final Map myDisposedObjects = new WeakHashMap(100, 0.5f, ContainerUtil.identityStrategy()); // guarded by treeLock private final List> myExecutedNodes = new ArrayList>(); // guarded by myExecutedNodes private final List myExecutedUnregisteredNodes = new ArrayList(); // guarded by myExecutedUnregisteredNodes @@ -58,6 +61,13 @@ public final class ObjectTree { public final void register(@NotNull T parent, @NotNull T child) { synchronized (treeLock) { + Object wasDisposed = myDisposedObjects.get(parent); + if (wasDisposed != null) { + throw new IncorrectOperationException("Sorry but parent: " + parent + " has already been disposed " + + "(see the cause for stacktrace) so the child: "+child+" will never be disposed", + wasDisposed instanceof Throwable ? (Throwable)wasDisposed : null); + } + myDisposedObjects.remove(child); // if we dispose thing and then register it back it means it's not disposed anymore ObjectNode parentNode = getNode(parent); if (parentNode == null) parentNode = createNodeFor(parent, null); @@ -202,7 +212,7 @@ public final class ObjectTree { } } } - + @TestOnly public boolean isEmpty() { synchronized (treeLock) { @@ -235,6 +245,7 @@ public final class ObjectTree { for (ObjectTreeListener each : myListeners) { each.objectExecuted(object); } + myDisposedObjects.put(object, Disposer.isDebugMode() ? ThrowableInterner.intern(new Throwable()) : Boolean.TRUE); } int size() { diff --git a/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java b/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java index 59af18cd1bf2..628471027e65 100644 --- a/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java +++ b/platform/util/testSrc/com/intellij/openapi/util/objectTree/DisposerTest.java @@ -27,7 +27,6 @@ import java.util.List; import static com.intellij.openapi.util.Disposer.newDisposable; public class DisposerTest extends TestCase { - private MyDisposable myRoot; private MyDisposable myFolder1; @@ -222,7 +221,7 @@ public class DisposerTest extends TestCase { private boolean myDisposed; protected String myName; - MyDisposable(@NonNls String aName) { + private MyDisposable(@NonNls String aName) { myName = aName; } @@ -301,4 +300,28 @@ public class DisposerTest extends TestCase { } } + + public void testMustNotRegisterWithAlreadyDisposed() { + Disposable disposable = Disposer.newDisposable(); + Disposer.register(myRoot, disposable); + + Disposer.dispose(disposable); + + try { + Disposer.register(disposable, Disposer.newDisposable()); + fail("Must not be able to register with already disposed parent"); + } + catch (IncorrectOperationException ignored) { + + } + } + + public void testRegisterThenDisposeThenRegisterAgain() { + Disposable disposable = Disposer.newDisposable(); + Disposer.register(myRoot, disposable); + + Disposer.dispose(disposable); + Disposer.register(myRoot, disposable); + Disposer.register(disposable, Disposer.newDisposable()); + } }