From d453eefa1f83a53159f66a3cd2d64460f5cec00e Mon Sep 17 00:00:00 2001 From: Andrey Loskutov Date: Mon, 2 May 2016 17:43:56 +0200 Subject: [PATCH] [memory leak] make sure we only allow one GitHistoryPageSource instance This should fix the issue that we can have multiple GitHistoryPage instances for same repository created and held by GenericHistoryView. The code in PageBookView.getPageRec(IWorkbenchPart) uses map with the part as a key to get appropriate page instance. In case an existing page is not found, the GenericHistoryView.showHistoryPageFor(Object, boolean, boolean, IHistoryPageSource) creates a new "dummy" HistoryPageSourceWorkbenchPart, which was used as a key in the map above. In the hashCode() and equals() methods the HistoryPageSourceWorkbenchPart uses hashCode() and equals() methods from IHistoryPageSource instances. Previously it could happen that multiple GitHistoryPageSource instances were created by the GitAdapterFactory because the factory instances are created for each adapter contribution in the plugin.xml, so that GitHistoryPageSource instances returned from different GitAdapterFactory instances were different. With different GitHistoryPageSource instances the HistoryPageSourceWorkbenchPart keys were always different (even for the same repository/file), the "old" existing GitHistoryPage instances were not found, new pages were created by GenericHistoryView and old pages left in memory (until closing the history view). Bug: 492099 Change-Id: I451c39589994387998053c5d09074ebf69cca0a4 Signed-off-by: Andrey Loskutov --- .../ui/internal/factories/GitAdapterFactory.java | 3 +-- .../ui/internal/history/GitHistoryPageSource.java | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/factories/GitAdapterFactory.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/factories/GitAdapterFactory.java index 8a17fe612..1d194607a 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/factories/GitAdapterFactory.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/factories/GitAdapterFactory.java @@ -64,7 +64,6 @@ import org.eclipse.ui.part.IShowInSource; */ public class GitAdapterFactory implements IAdapterFactory { - private Object historyPageSource = new GitHistoryPageSource(); private GitModelWorkbenchAdapter gitModelWorkbenchAdapter; private static final IWorkspaceRoot root = ResourcesPlugin.getWorkspace() @@ -73,7 +72,7 @@ public class GitAdapterFactory implements IAdapterFactory { @Override public Object getAdapter(Object adaptableObject, Class adapterType) { if (adapterType.isAssignableFrom(IHistoryPageSource.class)) { - return historyPageSource; + return GitHistoryPageSource.INSTANCE; } if (IWorkbenchAdapter.class == adapterType) { diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPageSource.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPageSource.java index 0ea3a768a..9fe54ff16 100644 --- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPageSource.java +++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/history/GitHistoryPageSource.java @@ -17,6 +17,16 @@ import org.eclipse.ui.part.Page; * A helper class for constructing the {@link GitHistoryPage}. */ public class GitHistoryPageSource extends HistoryPageSource { + + /** + * The instance to use if needed. + */ + public static final GitHistoryPageSource INSTANCE = new GitHistoryPageSource(); + + private GitHistoryPageSource() { + super(); + } + @Override public boolean canShowHistoryFor(final Object object) { return GitHistoryPage.canShowHistoryFor(object); @@ -27,4 +37,14 @@ public class GitHistoryPageSource extends HistoryPageSource { // don't set the input, the framework does this for us return new GitHistoryPage(); } + + @Override + public boolean equals(Object obj) { + return obj instanceof GitHistoryPageSource; + } + + @Override + public int hashCode() { + return 42; + } } -- 2.11.4.GIT