opendev/gerrit
Code Issues Proposed changes

Fix PatchScreen leak when moving between files

Browse Source 
PatchScreen added a reference to itself to the shared copy of the
ListenableAccountDiffPreference for a change. Every file contained
by that PatchSet uses the same ListenableAccountDiffPreference,
so the PatchScreen and all of its DOM elements and widgets and even
the last cached PatchScript leaked across file navigation.

Bug: issue 1370
Change-Id: Iae588a34878f83d95bd1b03913d3a8523afdf58c
This commit is contained in:
Shawn Pearce
2013-04-26 10:57:40 -07:00
parent a5ec9845f6
commit 92817e8784
1 changed files with 17 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
View File

@@ -113,6 +113,7 @@ public abstract class PatchScreen extends Screen implements
/** The index of the file we are currently looking at among the fileList */
private int patchIndex;
private ListenableAccountDiffPreference prefs;
private HandlerRegistration prefsHandler;
/** Keys that cause an action on this screen */
private KeyCommandSet keysNavigation;
@@ -141,19 +142,12 @@ public abstract class PatchScreen extends Screen implements
idSideB = id.getParentKey();
this.patchIndex = patchIndex;
prefs = fileList != null ? fileList.getPreferences() :
new ListenableAccountDiffPreference();
prefs = fileList != null
? fileList.getPreferences()
: new ListenableAccountDiffPreference();
if (Gerrit.isSignedIn()) {
prefs.reset();
}
prefs.addValueChangeHandler(
new ValueChangeHandler<AccountDiffPreference>() {
@Override
public void onValueChange(ValueChangeEvent<AccountDiffPreference> event) {
update(event.getValue());
}
});
reviewedPanels = new ReviewedPanels();
settingsPanel = new PatchScriptSettingsPanel(prefs);
}
@@ -301,6 +295,10 @@ public abstract class PatchScreen extends Screen implements
@Override
protected void onUnload() {
if (prefsHandler != null) {
prefsHandler.removeHandler();
prefsHandler = null;
}
if (regNavigation != null) {
regNavigation.removeHandler();
regNavigation = null;
@@ -475,6 +473,15 @@ public abstract class PatchScreen extends Screen implements
@Override
public void onShowView() {
super.onShowView();
if (prefsHandler == null) {
prefsHandler = prefs.addValueChangeHandler(
new ValueChangeHandler<AccountDiffPreference>() {
@Override
public void onValueChange(ValueChangeEvent<AccountDiffPreference> event) {
update(event.getValue());
}
});
}
if (intralineFailure) {
intralineFailure = false;
new ErrorDialog(PatchUtil.C.intralineFailure()).show();