opendev/gerrit
Code Issues Proposed changes

Delay marking a file reviewed until its displaying

Browse Source 
Loading a patch screen currently takes 3 concurrent RPCs.  That is
quite a few requests, but only 2 of them are required to finish in
order to display the file.  Move the reviewed toggle bit onto the
end of the process makes sure it doesn't delay the other RPCs from
being able to start.

Change-Id: Ib4bc02bc69cbd3dbabe288d15090bdfbe1b82fbd
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce 2010-02-23 18:06:33 -08:00
parent 1e5fc1fdf6
commit 3c7c33d01a
1 changed files with 13 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -132,6 +132,7 @@ public abstract class PatchScreen extends Screen {
private DisclosurePanel historyPanel;
private HistoryTable historyTable;
private CheckBox reviewedFlag;
private FlowPanel contentPanel;
private Label noDifference;
private AbstractPatchContentTable contentTable;
@ -293,16 +294,14 @@ public abstract class PatchScreen extends Screen {
// "Reviewed" check box
if (Gerrit.isSignedIn()) {
final CheckBox ku = new CheckBox(PatchUtil.C.reviewed());
ku.addValueChangeHandler(new ValueChangeHandler<Boolean>() {
reviewedFlag = new CheckBox(PatchUtil.C.reviewed());
reviewedFlag.addValueChangeHandler(new ValueChangeHandler<Boolean>() {
@Override
public void onValueChange(ValueChangeEvent<Boolean> event) {
setReviewedByCurrentUser(event.getValue());
}
});
// Checked by default
ku.setValue(true);
parent.setWidget(row, col + 2, ku);
parent.setWidget(row, col + 2, reviewedFlag);
}
}
@ -314,7 +313,6 @@ public abstract class PatchScreen extends Screen {
PatchUtil.DETAIL_SVC.setReviewedByCurrentUser(patchKey, reviewed,
new AsyncCallback<VoidResult>() {
@Override
public void onFailure(Throwable arg0) {
// nop
@ -324,7 +322,6 @@ public abstract class PatchScreen extends Screen {
public void onSuccess(VoidResult result) {
// nop
}
});
}
@ -411,17 +408,12 @@ public abstract class PatchScreen extends Screen {
script = null;
comments = null;
// Mark this file reviewed as soon we display the diff screen
if (Gerrit.isSignedIn() && isFirst) {
setReviewedByCurrentUser(true /* reviewed */);
}
PatchUtil.DETAIL_SVC.patchScript(patchKey, idSideA, idSideB,
scriptSettings, new GerritCallback<PatchScript>() {
public void onSuccess(final PatchScript result) {
if (rpcSequence == rpcseq) {
script = result;
onResult();
onResult(isFirst);
}
}
@ -438,7 +430,7 @@ public abstract class PatchScreen extends Screen {
public void onSuccess(final CommentDetail result) {
if (rpcSequence == rpcseq) {
comments = result;
onResult();
onResult(isFirst);
}
}
@ -454,7 +446,7 @@ public abstract class PatchScreen extends Screen {
});
}
private void onResult() {
private void onResult(final boolean isFirst) {
if (script != null && comments != null) {
final Change.Key cid = script.getChangeId();
final String path = patchKey.get();
@ -500,6 +492,12 @@ public abstract class PatchScreen extends Screen {
script = null;
comments = null;
// Mark this file reviewed as soon we display the diff screen
if (Gerrit.isSignedIn() && isFirst) {
reviewedFlag.setValue(true);
setReviewedByCurrentUser(true /* reviewed */);
}
if (!isCurrentView()) {
display();
}