Merge branch 'stable-2.6'

* stable-2.6:
  Reduce DOM nodes when updating reviewed status
  Fix PatchScreen leak when moving between files
  Use try/finally to ensure oldValue is cleared
  Remove unused HasValueChangeHandlers from PatchScriptSettingsPanel
  Make ListenableAccountDiffPreference final in PatchScriptSettingsPanel
  Remove unused setPreferences method on PatchTable
  init: Guess JDBC driver class if not configured
This commit is contained in:
Shawn Pearce
2013-04-26 13:04:39 -07:00
5 changed files with 73 additions and 56 deletions

View File

@@ -48,7 +48,9 @@ import com.google.gwtexpui.safehtml.client.SafeHtml;
import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map;
public class PatchTable extends Composite { public class PatchTable extends Composite {
public interface PatchValidator { public interface PatchValidator {
@@ -80,6 +82,7 @@ public class PatchTable extends Composite {
private String savePointerId; private String savePointerId;
private PatchSet.Id base; private PatchSet.Id base;
private List<Patch> patchList; private List<Patch> patchList;
private Map<Patch.Key, Integer> patchMap;
private ListenableAccountDiffPreference listenablePrefs; private ListenableAccountDiffPreference listenablePrefs;
private List<ClickHandler> clickHandlers; private List<ClickHandler> clickHandlers;
@@ -97,18 +100,25 @@ public class PatchTable extends Composite {
} }
public int indexOf(Patch.Key patch) { public int indexOf(Patch.Key patch) {
for (int i = 0; i < patchList.size(); i++) { Integer i = patchMap().get(patch);
if (patchList.get(i).getKey().equals(patch)) { return i != null ? i : -1;
return i; }
private Map<Key, Integer> patchMap() {
if (patchMap == null) {
patchMap = new HashMap<Patch.Key, Integer>();
for (int i = 0; i < patchList.size(); i++) {
patchMap.put(patchList.get(i).getKey(), i);
} }
} }
return -1; return patchMap;
} }
public void display(PatchSet.Id base, PatchSetDetail detail) { public void display(PatchSet.Id base, PatchSetDetail detail) {
this.base = base; this.base = base;
this.detail = detail; this.detail = detail;
this.patchList = detail.getPatches(); this.patchList = detail.getPatches();
this.patchMap = null;
myTable = null; myTable = null;
final DisplayCommand cmd = new DisplayCommand(patchList, base); final DisplayCommand cmd = new DisplayCommand(patchList, base);
@@ -293,10 +303,6 @@ public class PatchTable extends Composite {
return listenablePrefs; return listenablePrefs;
} }
public void setPreferences(ListenableAccountDiffPreference prefs) {
listenablePrefs = prefs;
}
private class MyTable extends NavigationTable<Patch> { private class MyTable extends NavigationTable<Patch> {
private static final int C_PATH = 2; private static final int C_PATH = 2;
private static final int C_DRAFT = 3; private static final int C_DRAFT = 3;
@@ -330,36 +336,33 @@ public class PatchTable extends Composite {
} }
void updateReviewedStatus(final Patch.Key patchKey, boolean reviewed) { void updateReviewedStatus(final Patch.Key patchKey, boolean reviewed) {
final int row = findRow(patchKey); int idx = patchMap().get(patchKey);
if (0 <= row) { if (0 <= idx) {
final Patch patch = getRowItem(row); Patch patch = patchList.get(idx);
if (patch != null) { if (patch.isReviewedByCurrentUser() != reviewed) {
patch.setReviewedByCurrentUser(reviewed); int row = idx + 1;
int col = C_SIDEBYSIDE + 2; int col = C_SIDEBYSIDE + 2;
if (patch.getPatchType() == Patch.PatchType.BINARY) { if (patch.getPatchType() == Patch.PatchType.BINARY) {
col = C_SIDEBYSIDE + 3; col = C_SIDEBYSIDE + 3;
} }
if (reviewed) { if (reviewed) {
table.setWidget(row, col, new Image(Gerrit.RESOURCES.greenCheck())); table.setWidget(row, col, new Image(Gerrit.RESOURCES.greenCheck()));
} else { } else {
table.clearCell(row, col); table.clearCell(row, col);
} }
patch.setReviewedByCurrentUser(reviewed);
} }
} }
} }
void notifyDraftDelta(final Patch.Key key, final int delta) { void notifyDraftDelta(final Patch.Key key, final int delta) {
final int row = findRow(key); int idx = patchMap().get(key);
if (0 <= row) { if (0 <= idx) {
final Patch p = getRowItem(row); Patch p = patchList.get(idx);
if (p != null) { p.setDraftCount(p.getDraftCount() + delta);
p.setDraftCount(p.getDraftCount() + delta); SafeHtmlBuilder m = new SafeHtmlBuilder();
final SafeHtmlBuilder m = new SafeHtmlBuilder(); appendCommentCount(m, p);
appendCommentCount(m, p); SafeHtml.set(table, idx + 1, C_DRAFT, m);
SafeHtml.set(table, row, C_DRAFT, m);
}
} }
} }

View File

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

View File

@@ -27,10 +27,6 @@ import com.google.gwt.event.dom.client.ClickEvent;
import com.google.gwt.event.dom.client.KeyCodes; import com.google.gwt.event.dom.client.KeyCodes;
import com.google.gwt.event.dom.client.KeyPressEvent; import com.google.gwt.event.dom.client.KeyPressEvent;
import com.google.gwt.event.dom.client.KeyPressHandler; import com.google.gwt.event.dom.client.KeyPressHandler;
import com.google.gwt.event.logical.shared.HasValueChangeHandlers;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.event.shared.HandlerRegistration;
import com.google.gwt.uibinder.client.UiBinder; import com.google.gwt.uibinder.client.UiBinder;
import com.google.gwt.uibinder.client.UiField; import com.google.gwt.uibinder.client.UiField;
import com.google.gwt.uibinder.client.UiHandler; import com.google.gwt.uibinder.client.UiHandler;
@@ -43,14 +39,13 @@ import com.google.gwt.user.client.ui.ListBox;
import com.google.gwt.user.client.ui.Widget; import com.google.gwt.user.client.ui.Widget;
import com.google.gwtjsonrpc.common.VoidResult; import com.google.gwtjsonrpc.common.VoidResult;
public class PatchScriptSettingsPanel extends Composite implements public class PatchScriptSettingsPanel extends Composite {
HasValueChangeHandlers<AccountDiffPreference> {
private static MyUiBinder uiBinder = GWT.create(MyUiBinder.class); private static MyUiBinder uiBinder = GWT.create(MyUiBinder.class);
interface MyUiBinder extends UiBinder<Widget, PatchScriptSettingsPanel> { interface MyUiBinder extends UiBinder<Widget, PatchScriptSettingsPanel> {
} }
private ListenableAccountDiffPreference listenablePrefs; private final ListenableAccountDiffPreference listenablePrefs;
private boolean enableIntralineDifference = true; private boolean enableIntralineDifference = true;
private boolean enableSmallFileFeatures = true; private boolean enableSmallFileFeatures = true;
@@ -139,12 +134,6 @@ public class PatchScriptSettingsPanel extends Composite implements
display(); display();
} }
@Override
public HandlerRegistration addValueChangeHandler(
ValueChangeHandler<AccountDiffPreference> handler) {
return super.addHandler(handler, ValueChangeEvent.getType());
}
public void setEnabled(final boolean on) { public void setEnabled(final boolean on) {
if (on) { if (on) {
setEnabledCounter++; setEnabledCounter++;

View File

@@ -23,8 +23,11 @@ public class ListenableOldValue<T> extends ListenableValue<T> {
} }
public void set(final T value) { public void set(final T value) {
oldValue = get(); try {
super.set(value); oldValue = get();
oldValue = null; // allow it to be gced before the next event super.set(value);
} finally {
oldValue = null; // allow it to be gced before the next event
}
} }
} }

View File

@@ -16,13 +16,28 @@ package com.google.gerrit.pgm.init;
import static com.google.gerrit.pgm.init.InitUtil.username; import static com.google.gerrit.pgm.init.InitUtil.username;
class JDBCInitializer implements DatabaseConfigInitializer { import com.google.common.base.Strings;
class JDBCInitializer implements DatabaseConfigInitializer {
@Override @Override
public void initConfig(Section databaseSection) { public void initConfig(Section database) {
databaseSection.string("Driver class name", "driver", null); database.string("URL", "url", null);
databaseSection.string("URL", "url", null); guessDriver(database);
databaseSection.string("Database username", "username", username()); database.string("Driver class name", "driver", null);
databaseSection.password("username", "password"); database.string("Database username", "username", username());
database.password("username", "password");
}
private void guessDriver(Section database) {
String url = Strings.emptyToNull(database.get("url"));
if (url != null && Strings.isNullOrEmpty(database.get("driver"))) {
if (url.startsWith("jdbc:h2:")) {
database.set("driver", "org.h2.Driver");
} else if (url.startsWith("jdbc:mysql:")) {
database.set("driver", "com.mysql.jdbc.Driver");
} else if (url.startsWith("jdbc:postgresql:")) {
database.set("driver", "org.postgresql.Driver");
}
}
} }
} }