summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn Pearce <sop@google.com>2013-08-13 09:59:24 -0700
committerShawn Pearce <sop@google.com>2013-08-14 12:12:46 -0700
commitf238e6fefa9715422ceeabb4ba875ebf12cb50b8 (patch)
tree82927897362bcdb6a76ee797a07a09ea1c24e82f
parent78c978e40741fdbd5103449a14a5b60614d6fcff (diff)
Fix browser NPE when ChangeCache is incomplete
The ChangeCache is buggy and does not always populate its contents. Unfortunately it is also designed to be unable to load missing contents on demand, causing consumers to NPE. ConfigInfoCache is a better design for this sort of lazy loading and reuse of data. ChangeCache can be missing information if: 1) opens side-by-side view in a new tab; 2) user presses 'r' to open publish comment screen Instead of using the unreliable ChangeCache, stub out a ChangeDetail to feed to the info block on the publish comments screen. Bug: issue 2039 Change-Id: Id542528d93af1cab49b001ca5e90addfc5d05b78 (cherry picked from commit f646aa77fb67b73c8724d7a632cafd441365e901)
-rw-r--r--gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java6
-rw-r--r--gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java13
-rw-r--r--gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java2
-rw-r--r--gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java6
4 files changed, 15 insertions, 12 deletions
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java
index 5cd6cdb131..8395907a3e 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeDescriptionBlock.java
@@ -15,8 +15,8 @@
package com.google.gerrit.client.changes;
import com.google.gerrit.common.data.AccountInfoCache;
+import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.SubmitTypeRecord;
-import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gwt.user.client.ui.Composite;
import com.google.gwt.user.client.ui.HorizontalPanel;
@@ -36,11 +36,11 @@ public class ChangeDescriptionBlock extends Composite {
initWidget(hp);
}
- public void display(Change chg, Boolean starred, Boolean canEditCommitMessage,
+ public void display(ChangeDetail chg, Boolean starred, Boolean canEditCommitMessage,
PatchSetInfo info,
final AccountInfoCache acc, SubmitTypeRecord submitTypeRecord) {
infoBlock.display(chg, acc, submitTypeRecord);
- messageBlock.display(chg.currentPatchSetId(), starred,
+ messageBlock.display(chg.getChange().currentPatchSetId(), starred,
canEditCommitMessage, info.getMessage());
}
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
index e4dbc3218d..afc17f7c60 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfoBlock.java
@@ -95,8 +95,9 @@ public class ChangeInfoBlock extends Composite {
table.getCellFormatter().addStyleName(row, 0, Gerrit.RESOURCES.css().header());
}
- public void display(final Change chg, final AccountInfoCache acc,
- SubmitTypeRecord submitTypeRecord) {
+ public void display(final ChangeDetail changeDetail,
+ final AccountInfoCache acc, SubmitTypeRecord submitTypeRecord) {
+ final Change chg = changeDetail.getChange();
final Branch.NameKey dst = chg.getDest();
CopyableLabel changeIdLabel =
@@ -114,7 +115,7 @@ public class ChangeInfoBlock extends Composite {
table.setWidget(R_BRANCH, 1, new BranchLink(dst.getShortName(), chg
.getProject(), chg.getStatus(), dst.get(), null));
- table.setWidget(R_TOPIC, 1, topic(chg));
+ table.setWidget(R_TOPIC, 1, topic(changeDetail));
table.setText(R_UPLOADED, 1, mediumFormat(chg.getCreatedOn()));
table.setText(R_UPDATED, 1, mediumFormat(chg.getLastUpdatedOn()));
table.setText(R_STATUS, 1, Util.toLongString(chg.getStatus()));
@@ -146,7 +147,8 @@ public class ChangeInfoBlock extends Composite {
}
}
- public Widget topic(final Change chg) {
+ public Widget topic(final ChangeDetail changeDetail) {
+ final Change chg = changeDetail.getChange();
final Branch.NameKey dst = chg.getDest();
FlowPanel fp = new FlowPanel();
@@ -154,9 +156,6 @@ public class ChangeInfoBlock extends Composite {
fp.add(new BranchLink(chg.getTopic(), chg.getProject(), chg.getStatus(),
dst.get(), chg.getTopic()));
- ChangeDetailCache detailCache = ChangeCache.get(chg.getId()).getChangeDetailCache();
- ChangeDetail changeDetail = detailCache.get();
-
if (changeDetail.canEditTopicName()) {
final Image edit = new Image(Gerrit.RESOURCES.edit());
edit.addStyleName(Gerrit.RESOURCES.css().link());
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
index 7983512898..91c885691a 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeScreen.java
@@ -301,7 +301,7 @@ public class ChangeScreen extends Screen
dependencies.setAccountInfoCache(detail.getAccounts());
- descriptionBlock.display(detail.getChange(),
+ descriptionBlock.display(detail,
detail.isStarred(),
detail.canEditCommitMessage(),
detail.getCurrentPatchSetDetail().getInfo(),
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
index 97949e3b8b..7f61977c38 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java
@@ -30,6 +30,7 @@ import com.google.gerrit.client.ui.CommentLinkProcessor;
import com.google.gerrit.client.ui.PatchLink;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.common.PageLinks;
+import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.PatchSetPublishDetail;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Patch;
@@ -298,9 +299,12 @@ public class PublishCommentScreen extends AccountScreen implements
}
private void display(final PatchSetPublishDetail r) {
+ ChangeDetail changeDetail = new ChangeDetail();
+ changeDetail.setChange(r.getChange());
+
setPageTitle(Util.M.publishComments(r.getChange().getKey().abbreviate(),
patchSetId.get()));
- descBlock.display(r.getChange(), null, false, r.getPatchSetInfo(), r.getAccounts(),
+ descBlock.display(changeDetail, null, false, r.getPatchSetInfo(), r.getAccounts(),
r.getSubmitTypeRecord());
if (r.getChange().getStatus().isOpen()) {