Merge changes I05a976fa,I1ab25e0a,I1ee8ea3f,I43fecd29,I5abc21eb

* changes: Defer failure in CallbackGroup until all are completed CodeMirror: Ensure both CSS and JavaScript are loaded CallbackGroup: use addEmpty() to setup a null callback ChangeScreen2: Preload CodeMirror CSS and JS Split GWT download of SideBySide1 from SideBySide2
2015-01-05 21:12:45 +00:00
parent 29dfd2ea0f a66c005995
commit 4ef96edfdc
7 changed files with 143 additions and 119 deletions
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/Dispatcher.java
@@ -672,85 +672,95 @@ public class Dispatcher {
        patchSetDetail, patchTable, topView, null);
  }

-  public static void patch(String token, final PatchSet.Id baseId, final Patch.Key id,
-      final DisplaySide side, final int line,
-      final int patchIndex, final PatchSetDetail patchSetDetail,
-      final PatchTable patchTable, final PatchScreen.TopView topView,
-      final String panelType) {
-    final PatchScreen.TopView top =  topView == null ?
-        Gerrit.getPatchScreenTopView() : topView;
+  public static void patch(String token, PatchSet.Id baseId,
+      Patch.Key id, DisplaySide side, int line,
+      int patchIndex, PatchSetDetail patchSetDetail,
+      PatchTable patchTable, PatchScreen.TopView topView,
+      String panelType) {
+    if (id == null) {
+      Gerrit.display(token, new NotFoundScreen());
+      return;
+    }

+    String panel = panelType;
+    if (panel == null) {
+      int c = token.lastIndexOf(',');
+      panel = 0 <= c ? token.substring(c + 1) : "";
+    }
+
+    if ("".equals(panel)) {
+      if (isChangeScreen2()) {
+        if (Gerrit.isSignedIn()
+            && DiffView.UNIFIED_DIFF.equals(Gerrit.getUserAccount()
+                .getGeneralPreferences().getDiffView())) {
+          sbs1(token, baseId, id, patchIndex, patchSetDetail, patchTable,
+              topView, PatchScreen.Type.UNIFIED);
+          return;
+        }
+        sbs2(token, baseId, id, side, line, false);
+        return;
+      }
+      sbs1(token, baseId, id, patchIndex, patchSetDetail, patchTable, topView,
+          PatchScreen.Type.SIDE_BY_SIDE);
+      return;
+    } else if ("unified".equals(panel)) {
+      sbs1(token, baseId, id, patchIndex, patchSetDetail, patchTable, topView,
+          PatchScreen.Type.UNIFIED);
+      return;
+    } else if ("cm".equals(panel) && Gerrit.getConfig().getNewFeatures()) {
+      if (Gerrit.isSignedIn()
+          && DiffView.UNIFIED_DIFF.equals(Gerrit.getUserAccount()
+              .getGeneralPreferences().getDiffView())) {
+        sbs1(token, baseId, id, patchIndex, patchSetDetail, patchTable,
+            topView, PatchScreen.Type.UNIFIED);
+        return;
+      }
+      sbs2(token, baseId, id, side, line, false);
+      return;
+    } else if ("".equals(panel) || "sidebyside".equals(panel)) {
+      sbs1(token, baseId, id, patchIndex, patchSetDetail, patchTable, topView,
+          PatchScreen.Type.SIDE_BY_SIDE);
+      return;
+    } else if (panel.equals("edit")) {
+      sbs2(token, null, id, null, 0, true);
+      return;
+    }
+    Gerrit.display(token, new NotFoundScreen());
+  }
+
+  private static void sbs1(final String token, final PatchSet.Id baseId,
+      final Patch.Key id, final int patchIndex,
+      final PatchSetDetail patchSetDetail, final PatchTable patchTable,
+      final PatchScreen.TopView topView, final PatchScreen.Type type) {
    GWT.runAsync(new AsyncSplit(token) {
      @Override
      public void onSuccess() {
-        Gerrit.display(token, select());
-      }
-
-      private Screen select() {
-        if (id != null) {
-          String panel = panelType;
-          if (panel == null) {
-            int c = token.lastIndexOf(',');
-            panel = 0 <= c ? token.substring(c + 1) : "";
-          }
-
-          if ("".equals(panel)) {
-            if (isChangeScreen2()) {
-              if (Gerrit.isSignedIn()
-                  && DiffView.UNIFIED_DIFF.equals(Gerrit.getUserAccount()
-                      .getGeneralPreferences().getDiffView())) {
-                return new PatchScreen.Unified(id, patchIndex, patchSetDetail,
-                    patchTable, top, baseId);
-              }
-              return new SideBySide2(baseId, id.getParentKey(), id.get(),
-                  side, line);
-            }
-            return new PatchScreen.SideBySide( //
-                id, //
-                patchIndex, //
-                patchSetDetail, //
-                patchTable, //
-                top, //
-                baseId //
-            );
-          } else if ("unified".equals(panel)) {
-            return new PatchScreen.Unified( //
-                id, //
-                patchIndex, //
-                patchSetDetail, //
-                patchTable, //
-                top, //
-                baseId //
-            );
-          } else if ("cm".equals(panel) && Gerrit.getConfig().getNewFeatures()) {
-            if (Gerrit.isSignedIn()
-                && DiffView.UNIFIED_DIFF.equals(Gerrit.getUserAccount()
-                    .getGeneralPreferences().getDiffView())) {
-              return new PatchScreen.Unified( //
-                  id, //
-                  patchIndex, //
-                  patchSetDetail, //
-                  patchTable, //
-                  top, //
-                  baseId //
-              );
-            }
-            return new SideBySide2(baseId, id.getParentKey(), id.get(),
-                side, line);
-          } else if ("".equals(panel) || "sidebyside".equals(panel)) {
-            return new PatchScreen.SideBySide(//
-                id, //
-                patchIndex,//
-                patchSetDetail,//
-                patchTable,//
-                top,//
-                baseId);//
-          } else if (panel.equals("edit")) {
-            return new EditScreen(id);
-          }
+        PatchScreen.TopView top =  topView == null
+            ? Gerrit.getPatchScreenTopView()
+            : topView;
+        switch (type) {
+          case SIDE_BY_SIDE:
+            Gerrit.display(token, new PatchScreen.SideBySide(id, patchIndex,
+                patchSetDetail, patchTable, top, baseId));
+            break;
+          case UNIFIED:
+            Gerrit.display(token, new PatchScreen.Unified(id, patchIndex,
+                patchSetDetail, patchTable, top, baseId));
+            break;
        }
+      }
+    });
+  }

-        return new NotFoundScreen();
+  private static void sbs2(final String token, final PatchSet.Id baseId,
+      final Patch.Key id, final DisplaySide side, final int line,
+      final boolean edit) {
+    GWT.runAsync(new AsyncSplit(token) {
+      @Override
+      public void onSuccess() {
+        Gerrit.display(token, edit
+            ? new EditScreen(id)
+            : new SideBySide2(baseId, id.getParentKey(), id.get(), side, line));
      }
    });
  }
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/ChangeScreen2.java
@@ -91,6 +91,8 @@ import com.google.gwtexpui.globalkey.client.KeyCommandSet;
 import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder;
 import com.google.gwtorm.client.KeyUtil;

+import net.codemirror.lib.CodeMirror;
+
 import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.EnumSet;
@@ -511,6 +513,7 @@ public class ChangeScreen2 extends Screen {
    }

    ChangeGlue.fireShowChange(changeInfo, changeInfo.revision(revision));
+    CodeMirror.preload();
    startPoller();
    if (NewChangeScreenBar.show()) {
      add(new NewChangeScreenBar(changeId));
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java
@@ -175,13 +175,11 @@ public class SideBySide2 extends Screen {
    super.onLoad();

    CallbackGroup cmGroup = new CallbackGroup();
-    CodeMirror.initLibrary(cmGroup.add(CallbackGroup.<Void> emptyCallback()));
+    CodeMirror.initLibrary(cmGroup.<Void> addEmpty());

    final CallbackGroup group = new CallbackGroup();
-    final AsyncCallback<Void> themeCallback =
-        group.add(CallbackGroup.<Void> emptyCallback());
-    final AsyncCallback<Void> modeInjectorCb =
-        group.add(CallbackGroup.<Void> emptyCallback());
+    final AsyncCallback<Void> themeCallback = group.addEmpty();
+    final AsyncCallback<Void> modeInjectorCb = group.addEmpty();

    DiffApi.diff(revision, path)
      .base(base)
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/editor/EditScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/editor/EditScreen.java
@@ -84,11 +84,10 @@ public class EditScreen extends Screen {
    super.onLoad();

    CallbackGroup cmGroup = new CallbackGroup();
-    CodeMirror.initLibrary(cmGroup.add(CallbackGroup.<Void> emptyCallback()));
+    CodeMirror.initLibrary(cmGroup.<Void> addEmpty());
    CallbackGroup group = new CallbackGroup();
    if (!Patch.COMMIT_MSG.equals(path)) {
-      final AsyncCallback<Void> modeInjectorCb =
-          group.add(CallbackGroup.<Void> emptyCallback());
+      final AsyncCallback<Void> modeInjectorCb = group.addEmpty();
      ChangeFileApi.getContentType(revision, path,
          cmGroup.add(new GerritCallback<String>() {
            @Override
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/CallbackGroup.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/CallbackGroup.java
@@ -66,6 +66,11 @@ public class CallbackGroup {
    remaining = new HashSet<>();
  }

+  public <T> Callback<T> addEmpty() {
+    Callback<T> cb = emptyCallback();
+    return add(cb);
+  }
+
  public <T> Callback<T> add(final AsyncCallback<T> cb) {
    checkFinalAdded();
    return handleAdd(cb);
@@ -91,7 +96,7 @@ public class CallbackGroup {
  }

  public void addListener(CallbackGroup group) {
-    addListener(group.add(CallbackGroup.<Void> emptyCallback()));
+    addListener(group.<Void> addEmpty());
  }

  private void applyAllSuccess() {
@@ -103,6 +108,15 @@ public class CallbackGroup {
    }
  }

+  private void applyAllFailed() {
+    if (failed && finalAdded && remaining.isEmpty()) {
+      for (CallbackImpl<?> cb : callbacks) {
+        cb.applyFailed();
+      }
+      callbacks.clear();
+    }
+  }
+
  private <T> Callback<T> handleAdd(AsyncCallback<T> cb) {
    if (failed) {
      cb.onFailure(failedThrowable);
@@ -135,10 +149,6 @@ public class CallbackGroup {

    @Override
    public void onSuccess(T value) {
-      if (failed) {
-        return;
-      }
-
      this.result = value;
      remaining.remove(this);
      CallbackGroup.this.applyAllSuccess();
@@ -146,19 +156,12 @@ public class CallbackGroup {

    @Override
    public void onFailure(Throwable caught) {
-      if (failed) {
-        return;
+      if (!failed) {
+        failed = true;
+        failedThrowable = caught;
      }
-
-      failed = true;
-      failedThrowable = caught;
-      for (CallbackImpl<?> cb : callbacks) {
-        cb.delegate.onFailure(failedThrowable);
-        cb.delegate = null;
-        cb.result = null;
-      }
-      callbacks.clear();
-      remaining.clear();
+      remaining.remove(this);
+      CallbackGroup.this.applyAllFailed();
    }

    void applySuccess() {
@@ -169,5 +172,14 @@ public class CallbackGroup {
        result = null;
      }
    }
+
+    void applyFailed() {
+      AsyncCallback<T> cb = delegate;
+      if (cb != null) {
+        delegate = null;
+        result = null;
+        cb.onFailure(failedThrowable);
+      }
+    }
  }
 }
--- a/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
@@ -15,6 +15,7 @@
 package net.codemirror.lib;

 import com.google.gerrit.client.diff.DisplaySide;
+import com.google.gerrit.client.rpc.CallbackGroup;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.NativeEvent;
@@ -28,6 +29,10 @@ import net.codemirror.lib.TextMarker.FromTo;
 * @see <a href="http://codemirror.net/doc/manual.html#api">CodeMirror API</a>
 */
 public class CodeMirror extends JavaScriptObject {
+  public static void preload() {
+    initLibrary(CallbackGroup.<Void> emptyCallback());
+  }
+
  public static void initLibrary(AsyncCallback<Void> cb) {
    Loader.initLibrary(cb);
  }
--- a/gerrit-gwtui/src/main/java/net/codemirror/lib/Loader.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/Loader.java
@@ -14,6 +14,7 @@

 package net.codemirror.lib;

+import com.google.gerrit.client.rpc.CallbackGroup;
 import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gwt.core.client.Callback;
 import com.google.gwt.core.client.ScriptInjector;
@@ -26,9 +27,6 @@ import com.google.gwt.resources.client.TextResource;
 import com.google.gwt.safehtml.shared.SafeUri;
 import com.google.gwt.user.client.rpc.AsyncCallback;

-import java.util.logging.Level;
-import java.util.logging.Logger;
-
 public class Loader {
  private static native boolean isLibLoaded()
  /*-{ return $wnd.hasOwnProperty('CodeMirror'); }-*/;
@@ -36,33 +34,38 @@ public class Loader {
  static void initLibrary(final AsyncCallback<Void> cb) {
    if (isLibLoaded()) {
      cb.onSuccess(null);
-    } else {
-      injectCss(Lib.I.css());
-      injectScript(Lib.I.js().getSafeUri(), new GerritCallback<Void>(){
-        @Override
-        public void onSuccess(Void result) {
-          Vim.initKeyMap();
-          cb.onSuccess(null);
-        }
-      });
+      return;
    }
+
+    CallbackGroup group = new CallbackGroup();
+    injectCss(Lib.I.css(), group.<Void> addEmpty());
+    injectScript(Lib.I.js().getSafeUri(), group.add(new GerritCallback<Void>() {
+      @Override
+      public void onSuccess(Void result) {
+        Vim.initKeyMap();
+      }
+    }));
+    group.addListener(cb);
+    group.done();
  }

-  private static void injectCss(ExternalTextResource css) {
+  private static void injectCss(ExternalTextResource css,
+      final AsyncCallback<Void> cb) {
    try {
      css.getText(new ResourceCallback<TextResource>() {
        @Override
        public void onSuccess(TextResource resource) {
          StyleInjector.inject(resource.getText());
+          cb.onSuccess(null);
        }

        @Override
        public void onError(ResourceException e) {
-          error(e);
+          cb.onFailure(e);
        }
      });
    } catch (ResourceException e) {
-      error(e);
+      cb.onFailure(e);
    }
  }

@@ -79,7 +82,6 @@ public class Loader {

        @Override
        public void onFailure(Exception reason) {
-          error(reason);
          callback.onFailure(reason);
        }
       })
@@ -87,11 +89,6 @@ public class Loader {
      .cast();
  }

-  private static void error(Exception e) {
-    Logger log = Logger.getLogger("net.codemirror");
-    log.log(Level.SEVERE, "Cannot load portions of CodeMirror", e);
-  }
-
  private Loader() {
  }
 }