commit 92817e8784d44be8949db300b7ecb29389e8c7df
author Shawn Pearce <sop@google.com> Fri Apr 26 10:57:40 2013 -0700
committer Shawn Pearce <sop@google.com> Fri Apr 26 10:57:40 2013 -0700
tree 0a109962257c76abff7f941b1abb0e71a078c3bf
parent a5ec9845f691ed760fbc14ff08e58cec8b8d2c6d

Fix PatchScreen leak when moving between files

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

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

diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
index 6f73d11..676cea6 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/PatchScreen.java
@@ -113,6 +113,7 @@
   /** 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 @@
     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 @@
 
   @Override
   protected void onUnload() {
+    if (prefsHandler != null) {
+      prefsHandler.removeHandler();
+      prefsHandler = null;
+    }
     if (regNavigation != null) {
       regNavigation.removeHandler();
       regNavigation = null;
@@ -475,6 +473,15 @@
   @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();