SideBySide2: Refactor comment management to new top level class

Yank all of the code related to comment management out of SideBySide2
and into its own new top level class. This moves about 500 lines
of code from a 1800 line class to a new 400 line class, which may
make it easier to follow SideBySide2.

Change-Id: Ie24b3e3af38a9dea38a756fc5b635f1cc54f7f8a

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentBox.java

@@ -36,18 +36,17 @@ abstract class CommentBox extends Composite {
 
   private PaddingManager widgetManager;
   private PaddingWidgetWrapper selfWidgetWrapper;
-  private SideBySide2 diffScreen;
+  private CommentManager commentManager;
   private CodeMirror cm;
-  private DisplaySide side;
   private DiffChunkInfo diffChunkInfo;
   private GutterWrapper gutterWrapper;
   private FromTo fromTo;
   private TextMarker rangeMarker;
   private TextMarker rangeHighlightMarker;
 
-  CommentBox(CodeMirror cm, CommentInfo info, DisplaySide side) {
+  CommentBox(CommentManager commentManager, CodeMirror cm, CommentInfo info) {
+    this.commentManager = commentManager;
     this.cm = cm;
-    this.side = side;
     CommentRange range = info.range();
     if (range != null) {
       fromTo = FromTo.create(range);
@@ -80,13 +79,15 @@ abstract class CommentBox extends Composite {
     if (!getCommentInfo().has_line()) {
       return;
     }
-    diffScreen.defer(new Runnable() {
+    commentManager.getSideBySide2().defer(new Runnable() {
       @Override
       public void run() {
         assert selfWidgetWrapper != null;
         selfWidgetWrapper.getWidget().changed();
         if (diffChunkInfo != null) {
-          diffScreen.resizePaddingOnOtherSide(side, diffChunkInfo.getEnd());
+          commentManager.getSideBySide2().resizePaddingOnOtherSide(
+              cm.side(),
+              diffChunkInfo.getEnd());
         } else {
           assert widgetManager != null;
           widgetManager.resizePaddingWidget();
@@ -104,6 +105,10 @@ abstract class CommentBox extends Composite {
     getCm().focus();
   }
 
+  CommentManager getCommentManager() {
+    return commentManager;
+  }
+
   PaddingManager getPaddingManager() {
     return widgetManager;
   }
@@ -124,14 +129,6 @@ abstract class CommentBox extends Composite {
     this.diffChunkInfo = info;
   }
 
-  SideBySide2 getDiffScreen() {
-    return diffScreen;
-  }
-
-  void setDiffScreen(SideBySide2 diffScreen) {
-    this.diffScreen = diffScreen;
-  }
-
   void setGutterWrapper(GutterWrapper wrapper) {
     gutterWrapper = wrapper;
   }
@@ -161,10 +158,6 @@ abstract class CommentBox extends Composite {
     return gutterWrapper;
   }
 
-  DisplaySide getSide() {
-    return side;
-  }
-
   CodeMirror getCm() {
     return cm;
   }

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java

@@ -0,0 +1,431 @@
+// Copyright (C) 2013 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.client.diff;
+
+import com.google.gerrit.client.Gerrit;
+import com.google.gerrit.client.changes.CommentInfo;
+import com.google.gerrit.client.diff.PaddingManager.PaddingWidgetWrapper;
+import com.google.gerrit.client.patches.SkippedLine;
+import com.google.gerrit.client.rpc.CallbackGroup;
+import com.google.gerrit.client.rpc.Natives;
+import com.google.gerrit.client.ui.CommentLinkProcessor;
+import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gwt.core.client.JsArray;
+import com.google.gwt.core.client.Scheduler;
+import com.google.gwt.core.client.Scheduler.ScheduledCommand;
+import com.google.gwt.dom.client.Style.Unit;
+
+import net.codemirror.lib.CodeMirror;
+import net.codemirror.lib.Configuration;
+import net.codemirror.lib.LineWidget;
+import net.codemirror.lib.CodeMirror.LineHandle;
+import net.codemirror.lib.TextMarker.FromTo;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/** Tracks comment widgets for {@link SideBySide2}. */
+class CommentManager {
+  private final SideBySide2 host;
+  private final PatchSet.Id base;
+  private final PatchSet.Id revision;
+  private final String path;
+  private final CommentLinkProcessor commentLinkProcessor;
+
+  private final Map<String, PublishedBox> published;
+  private final Map<LineHandle, CommentBox> lineActiveBox;
+  private final Map<LineHandle, List<PublishedBox>> linePublishedBoxes;
+  private final Map<LineHandle, PaddingManager> linePaddingManager;
+  private final Set<DraftBox> unsavedDrafts;
+
+  CommentManager(SideBySide2 host,
+      PatchSet.Id base, PatchSet.Id revision,
+      String path,
+      CommentLinkProcessor clp) {
+    this.host = host;
+    this.base = base;
+    this.revision = revision;
+    this.path = path;
+    this.commentLinkProcessor = clp;
+
+    published = new HashMap<String, PublishedBox>();
+    lineActiveBox = new HashMap<LineHandle, CommentBox>();
+    linePublishedBoxes = new HashMap<LineHandle, List<PublishedBox>>();
+    linePaddingManager = new HashMap<LineHandle, PaddingManager>();
+    unsavedDrafts = new HashSet<DraftBox>();
+  }
+
+  SideBySide2 getSideBySide2() {
+    return host;
+  }
+
+  void setExpandAllComments(boolean b) {
+    for (PublishedBox box : published.values()) {
+      box.setOpen(b);
+    }
+  }
+
+  void render(CommentsCollections in) {
+    if (in.publishedBase != null) {
+      renderPublished(DisplaySide.A, in.publishedBase);
+    }
+    if (in.publishedRevision != null) {
+      renderPublished(DisplaySide.B, in.publishedRevision);
+    }
+    if (in.draftsBase != null) {
+      renderDrafts(DisplaySide.A, in.draftsBase);
+    }
+    if (in.draftsRevision != null) {
+      renderDrafts(DisplaySide.B, in.draftsRevision);
+    }
+  }
+
+  private void renderPublished(DisplaySide forSide, JsArray<CommentInfo> in) {
+    for (CommentInfo info : Natives.asList(in)) {
+      DisplaySide side = displaySide(info, forSide);
+      if (side == null) {
+        continue;
+      }
+
+      CodeMirror cm = host.getCmFromSide(side);
+      PublishedBox box = new PublishedBox(this, cm, commentLinkProcessor,
+          getPatchSetIdFromSide(side), info);
+      published.put(info.id(), box);
+      if (!info.has_line()) {
+        host.diffTable.addFileCommentBox(box);
+        continue;
+      }
+
+      int line = info.line() - 1;
+      LineHandle handle = cm.getLineHandle(line);
+      if (linePublishedBoxes.containsKey(handle)) {
+        linePublishedBoxes.get(handle).add(box);
+      } else {
+        List<PublishedBox> list = new ArrayList<PublishedBox>(4);
+        list.add(box);
+        linePublishedBoxes.put(handle, list);
+      }
+      lineActiveBox.put(handle, box);
+      addCommentBox(info, box);
+    }
+  }
+
+  private void renderDrafts(DisplaySide forSide, JsArray<CommentInfo> in) {
+    for (CommentInfo info : Natives.asList(in)) {
+      DisplaySide side = displaySide(info, forSide);
+      if (side == null) {
+        continue;
+      }
+
+      CodeMirror cm = host.getCmFromSide(side);
+      DraftBox box = new DraftBox(this, cm, commentLinkProcessor,
+          getPatchSetIdFromSide(side), info);
+      if (info.in_reply_to() != null) {
+        PublishedBox r = published.get(info.in_reply_to());
+        if (r != null) {
+          r.registerReplyBox(box);
+        }
+      }
+      if (!info.has_line()) {
+        host.diffTable.addFileCommentBox(box);
+        continue;
+      }
+
+      lineActiveBox.put(cm.getLineHandle(info.line() - 1), box);
+      addCommentBox(info, box);
+    }
+  }
+
+  private DisplaySide displaySide(CommentInfo info, DisplaySide forSide) {
+    if (info.side() == Side.PARENT) {
+      return base == null ? DisplaySide.A : null;
+    }
+    return forSide;
+  }
+
+  List<SkippedLine> splitSkips(int context, List<SkippedLine> skips) {
+    // TODO: This is not optimal, but shouldn't be too costly in most cases.
+    // Maybe rewrite after done keeping track of diff chunk positions.
+    for (CommentBox box : lineActiveBox.values()) {
+      int boxLine = box.getCommentInfo().line();
+      boolean sideA = box.getCm().side() == DisplaySide.A;
+
+      List<SkippedLine> temp = new ArrayList<SkippedLine>(skips.size() + 2);
+      for (SkippedLine skip : skips) {
+        int startLine = sideA ? skip.getStartA() : skip.getStartB();
+        int deltaBefore = boxLine - startLine;
+        int deltaAfter = startLine + skip.getSize() - boxLine;
+        if (deltaBefore < -context || deltaAfter < -context) {
+          temp.add(skip); // Size guaranteed to be greater than 1
+        } else if (deltaBefore > context && deltaAfter > context) {
+          SkippedLine before = new SkippedLine(
+              skip.getStartA(), skip.getStartB(),
+              skip.getSize() - deltaAfter - context);
+          skip.incrementStart(deltaBefore + context);
+          checkAndAddSkip(temp, before);
+          checkAndAddSkip(temp, skip);
+        } else if (deltaAfter > context) {
+          skip.incrementStart(deltaBefore + context);
+          checkAndAddSkip(temp, skip);
+        } else if (deltaBefore > context) {
+          skip.reduceSize(deltaAfter + context);
+          checkAndAddSkip(temp, skip);
+        }
+      }
+      if (temp.isEmpty()) {
+        return temp;
+      }
+      skips = temp;
+    }
+    return skips;
+  }
+
+  private static void checkAndAddSkip(List<SkippedLine> out, SkippedLine s) {
+    if (s.getSize() > 1) {
+      out.add(s);
+    }
+  }
+
+  Runnable toggleOpenBox(final CodeMirror cm) {
+    return new Runnable() {
+      public void run() {
+        CommentBox box = lineActiveBox.get(cm.getActiveLine());
+        if (box != null) {
+          box.setOpen(!box.isOpen());
+        }
+      }
+    };
+  }
+
+  Runnable openClosePublished(final CodeMirror cm) {
+    return new Runnable() {
+      @Override
+      public void run() {
+        if (cm.hasActiveLine()) {
+          List<PublishedBox> list =
+              linePublishedBoxes.get(cm.getActiveLine());
+          if (list == null) {
+            return;
+          }
+          boolean open = false;
+          for (PublishedBox box : list) {
+            if (!box.isOpen()) {
+              open = true;
+              break;
+            }
+          }
+          for (PublishedBox box : list) {
+            box.setOpen(open);
+          }
+        }
+      }
+    };
+  }
+
+  Runnable insertNewDraft(final CodeMirror cm) {
+    if (!Gerrit.isSignedIn()) {
+      return new Runnable() {
+        @Override
+        public void run() {
+          Gerrit.doSignIn(host.getToken());
+        }
+     };
+    }
+
+    return new Runnable() {
+      public void run() {
+        LineHandle handle = cm.getActiveLine();
+        int line = cm.getLineNumber(handle);
+        CommentBox box = lineActiveBox.get(handle);
+        FromTo fromTo = cm.getSelectedRange();
+        if (cm.somethingSelected()) {
+          lineActiveBox.put(handle,
+              newRangeDraft(cm, line, fromTo.getTo().getLine() == line ? fromTo : null));
+          cm.setSelection(cm.getCursor());
+        } else if (box == null) {
+          lineActiveBox.put(handle, newRangeDraft(cm, line, null));
+        } else if (box instanceof DraftBox) {
+          ((DraftBox) box).setEdit(true);
+        } else {
+          ((PublishedBox) box).doReply();
+        }
+      }
+    };
+  }
+
+  private DraftBox newRangeDraft(CodeMirror cm, int line, FromTo fromTo) {
+    DisplaySide side = cm.side();
+    return addDraftBox(CommentInfo.createRange(
+        path,
+        getStoredSideFromDisplaySide(side),
+        line + 1,
+        null,
+        null,
+        CommentRange.create(fromTo)), side);
+  }
+
+  DraftBox newFileDraft(DisplaySide side) {
+    return addDraftBox(CommentInfo.createFile(
+      path,
+      getStoredSideFromDisplaySide(side),
+      null, null), side);
+  }
+
+  CommentInfo createReply(CommentInfo replyTo) {
+    if (!replyTo.has_line() && replyTo.range() == null) {
+      return CommentInfo.createFile(path, replyTo.side(), replyTo.id(), null);
+    } else {
+      return CommentInfo.createRange(path, replyTo.side(), replyTo.line(),
+          replyTo.id(), null, replyTo.range());
+    }
+  }
+
+  DraftBox addDraftBox(CommentInfo info, DisplaySide side) {
+    CodeMirror cm = host.getCmFromSide(side);
+    final DraftBox box = new DraftBox(this, cm, commentLinkProcessor,
+        getPatchSetIdFromSide(side), info);
+    if (info.id() == null) {
+      Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+        @Override
+        public void execute() {
+          box.setOpen(true);
+          box.setEdit(true);
+        }
+      });
+    }
+    if (!info.has_line()) {
+      return box;
+    }
+    addCommentBox(info, box);
+    box.setVisible(true);
+    LineHandle handle = cm.getLineHandle(info.line() - 1);
+    lineActiveBox.put(handle, box);
+    return box;
+  }
+
+  private CommentBox addCommentBox(CommentInfo info, final CommentBox box) {
+    host.diffTable.add(box);
+    CodeMirror cm = box.getCm();
+    CodeMirror other = host.otherCm(cm);
+    int line = info.line() - 1; // CommentInfo is 1-based, but CM is 0-based
+    LineHandle handle = cm.getLineHandle(line);
+    PaddingManager manager;
+    if (linePaddingManager.containsKey(handle)) {
+      manager = linePaddingManager.get(handle);
+    } else {
+      // Estimated height at 28px, fixed by deferring after display
+      manager = new PaddingManager(host.addPaddingWidget(cm, line, 0, Unit.PX, 0));
+      linePaddingManager.put(handle, manager);
+    }
+
+    int lineToPad = host.lineOnOther(cm.side(), line).getLine();
+    LineHandle otherHandle = other.getLineHandle(lineToPad);
+    DiffChunkInfo myChunk = host.getDiffChunk(cm.side(), line);
+    DiffChunkInfo otherChunk = host.getDiffChunk(other.side(), lineToPad);
+    PaddingManager otherManager;
+    if (linePaddingManager.containsKey(otherHandle)) {
+      otherManager = linePaddingManager.get(otherHandle);
+    } else {
+      otherManager =
+          new PaddingManager(host.addPaddingWidget(other, lineToPad, 0, Unit.PX, 0));
+      linePaddingManager.put(otherHandle, otherManager);
+    }
+    if ((myChunk == null && otherChunk == null) || (myChunk != null && otherChunk != null)) {
+      PaddingManager.link(manager, otherManager);
+    }
+
+    int index = manager.getCurrentCount();
+    manager.insert(box, index);
+    Configuration config = Configuration.create()
+      .set("coverGutter", true)
+      .set("insertAt", index)
+      .set("noHScroll", true);
+    LineWidget boxWidget = host.addLineWidget(cm, line, box, config);
+    box.setPaddingManager(manager);
+    box.setSelfWidgetWrapper(new PaddingWidgetWrapper(boxWidget, box.getElement()));
+    if (otherChunk == null) {
+      box.setDiffChunkInfo(myChunk);
+    }
+    box.setGutterWrapper(host.diffTable.sidePanel.addGutter(cm, info.line() - 1,
+        box instanceof DraftBox
+          ? SidePanel.GutterType.DRAFT
+          : SidePanel.GutterType.COMMENT));
+    if (box instanceof DraftBox) {
+      boxWidget.onRedraw(new Runnable() {
+        @Override
+        public void run() {
+          DraftBox draftBox = (DraftBox) box;
+          if (draftBox.isEdit()) {
+            draftBox.editArea.setFocus(true);
+          }
+        }
+      });
+    }
+    return box;
+  }
+
+  void removeDraft(DraftBox box) {
+    int line = box.getCommentInfo().line() - 1;
+    LineHandle handle = box.getCm().getLineHandle(line);
+    lineActiveBox.remove(handle);
+    if (linePublishedBoxes.containsKey(handle)) {
+      List<PublishedBox> list = linePublishedBoxes.get(handle);
+      lineActiveBox.put(handle, list.get(list.size() - 1));
+    }
+    unsavedDrafts.remove(box);
+  }
+
+  void addFileCommentBox(CommentBox box) {
+    host.diffTable.addFileCommentBox(box);
+  }
+
+  void removeFileCommentBox(DraftBox box) {
+    host.diffTable.onRemoveDraftBox(box);
+  }
+
+  void resizePadding(LineHandle handle) {
+    CommentBox box = lineActiveBox.get(handle);
+    if (box != null) {
+      box.resizePaddingWidget();
+    }
+  }
+
+  void setUnsaved(DraftBox box, boolean isUnsaved) {
+    if (isUnsaved) {
+      unsavedDrafts.add(box);
+    } else {
+      unsavedDrafts.remove(box);
+    }
+  }
+
+  void saveAllDrafts(CallbackGroup cb) {
+    for (DraftBox box : unsavedDrafts) {
+      box.save(cb);
+    }
+  }
+
+  private Side getStoredSideFromDisplaySide(DisplaySide side) {
+    return side == DisplaySide.A && base == null ? Side.PARENT : Side.REVISION;
+  }
+
+  private PatchSet.Id getPatchSetIdFromSide(DisplaySide side) {
+    return side == DisplaySide.A && base != null ? base : revision;
+  }
+}

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DiffTable.java

@@ -92,19 +92,20 @@ class DiffTable extends Composite {
   @UiField
   static DiffTableStyle style;
 
-  private SideBySide2 host;
+  private SideBySide2 parent;
   private boolean headerVisible;
 
-  DiffTable(SideBySide2 host, PatchSet.Id base, PatchSet.Id revision, String path) {
+  DiffTable(SideBySide2 parent, PatchSet.Id base, PatchSet.Id revision, String path) {
     patchSetSelectBoxA = new PatchSetSelectBox2(
         this, DisplaySide.A, revision.getParentKey(), base, path);
     patchSetSelectBoxB = new PatchSetSelectBox2(
         this, DisplaySide.B, revision.getParentKey(), revision, path);
     PatchSetSelectBox2.link(patchSetSelectBoxA, patchSetSelectBoxB);
-    fileCommentPanelA = new FileCommentPanel(host, this, path, DisplaySide.A);
+
-    fileCommentPanelB = new FileCommentPanel(host, this, path, DisplaySide.B);
+    fileCommentPanelA = new FileCommentPanel(parent, this, DisplaySide.A);
+    fileCommentPanelB = new FileCommentPanel(parent, this, DisplaySide.B);
     initWidget(uiBinder.createAndBindUi(this));
-    this.host = host;
+    this.parent = parent;
     this.headerVisible = true;
   }
 
@@ -114,17 +115,17 @@ class DiffTable extends Composite {
 
   void setHeaderVisible(boolean show) {
     headerVisible = show;
-    Gerrit.setHeaderVisible(show && !host.getPrefs().hideTopMenu());
+    Gerrit.setHeaderVisible(show && !parent.getPrefs().hideTopMenu());
     UIObject.setVisible(patchSetNavRow, show);
     UIObject.setVisible(fileCommentRow, show
         && (fileCommentPanelA.getBoxCount() > 0
             || fileCommentPanelB.getBoxCount() > 0));
     if (show) {
-      host.header.removeStyleName(style.fullscreen());
+      parent.header.removeStyleName(style.fullscreen());
     } else {
-      host.header.addStyleName(style.fullscreen());
+      parent.header.addStyleName(style.fullscreen());
     }
-    host.resizeCodeMirror();
+    parent.resizeCodeMirror();
   }
 
   private FileCommentPanel getPanelFromSide(DisplaySide side) {
@@ -137,11 +138,11 @@ class DiffTable extends Composite {
   }
 
   void addFileCommentBox(CommentBox box) {
-    getPanelFromSide(box.getSide()).addFileComment(box);
+    getPanelFromSide(box.getCm().side()).addFileComment(box);
   }
 
   void onRemoveDraftBox(DraftBox box) {
-    getPanelFromSide(box.getSide()).onRemoveDraftBox(box);
+    getPanelFromSide(box.getCm().side()).onRemoveDraftBox(box);
   }
 
   int getHeaderHeight() {

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/DraftBox.java

@@ -80,15 +80,13 @@ class DraftBox extends CommentBox {
   @UiField Button discard2;
 
   DraftBox(
-      SideBySide2 sideBySide,
+      CommentManager manager,
       CodeMirror cm,
-      DisplaySide side,
       CommentLinkProcessor clp,
       PatchSet.Id id,
       CommentInfo info) {
-    super(cm, info, side);
+    super(manager, cm, info);
 
-    setDiffScreen(sideBySide);
     linkProcessor = clp;
     psId = id;
     initWidget(uiBinder.createAndBindUi(this));
@@ -205,7 +203,7 @@ class DraftBox extends CommentBox {
     } else {
       expandTimer.cancel();
     }
-    getDiffScreen().updateUnsaved(this, edit);
+    getCommentManager().setUnsaved(this, edit);
     resizePaddingWidget();
   }
 
@@ -227,12 +225,12 @@ class DraftBox extends CommentBox {
     setRangeHighlight(false);
     removeFromParent();
     if (!getCommentInfo().has_line()) {
-      getDiffScreen().removeFileCommentBox(this);
+      getCommentManager().removeFileCommentBox(this);
       return;
     }
     PaddingManager manager = getPaddingManager();
     manager.remove(this);
-    getDiffScreen().removeDraft(this, comment.line() - 1);
+    getCommentManager().removeDraft(this);
     getCm().focus();
     getSelfWidgetWrapper().getWidget().clear();
     getGutterWrapper().remove();
@@ -281,7 +279,7 @@ class DraftBox extends CommentBox {
         if (autoClosed) {
           setOpen(false);
         }
-        getDiffScreen().updateUnsaved(DraftBox.this, false);
+        getCommentManager().setUnsaved(DraftBox.this, false);
       }
 
       @Override

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/FileCommentPanel.java

@@ -15,7 +15,6 @@
 package com.google.gerrit.client.diff;
 
 import com.google.gerrit.client.Gerrit;
-import com.google.gerrit.client.changes.CommentInfo;
 import com.google.gwt.user.client.ui.Composite;
 import com.google.gwt.user.client.ui.FlowPanel;
 
@@ -28,18 +27,15 @@ import java.util.List;
  * system scrollbar.
  */
 class FileCommentPanel extends Composite {
-
+  private final SideBySide2 parent;
-  private SideBySide2 parent;
   private DiffTable table;
-  private String path;
   private DisplaySide side;
   private List<CommentBox> boxes;
   private FlowPanel body;
 
-  FileCommentPanel(SideBySide2 host, DiffTable table, String path, DisplaySide side) {
+  FileCommentPanel(SideBySide2 host, DiffTable table, DisplaySide side) {
     this.parent = host;
     this.table = table;
-    this.path = path;
     this.side = side;
     boxes = new ArrayList<CommentBox>();
     initWidget(body = new FlowPanel());
@@ -50,13 +46,9 @@ class FileCommentPanel extends Composite {
       Gerrit.doSignIn(parent.getToken());
       return;
     }
+
     if (boxes.isEmpty()) {
-      CommentInfo info = CommentInfo.createFile(
+      addFileComment(parent.getCommentManager().newFileDraft(side));
-          path,
-          parent.getStoredSideFromDisplaySide(side),
-          null,
-          null);
-      addFileComment(parent.addDraftBox(info, side));
     } else {
       CommentBox box = boxes.get(boxes.size() - 1);
       if (box instanceof DraftBox) {

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/Header.java

@@ -61,6 +61,10 @@ class Header extends Composite {
     Resources.I.style().ensureInjected();
   }
 
+  private static enum ReviewedState {
+    AUTO_REVIEW, LOADED;
+  }
+
   @UiField CheckBox reviewed;
   @UiField Element project;
   @UiField Element filePath;
@@ -80,6 +84,7 @@ class Header extends Composite {
   private boolean hasNext;
   private String nextPath;
   private PreferencesAction prefsAction;
+  private ReviewedState reviewedState;
 
   Header(KeyCommandSet keys, PatchSet.Id base, PatchSet.Id patchSetId,
       String path) {
@@ -157,12 +162,12 @@ class Header extends Composite {
         .get(new AsyncCallback<JsArrayString>() {
             @Override
             public void onSuccess(JsArrayString result) {
-              for (int i = 0; i < result.length(); i++) {
+              boolean b = Natives.asList(result).contains(path);
-                if (path.equals(result.get(i))) {
+              reviewed.setValue(b, false);
-                  reviewed.setValue(true, false);
+              if (!b && reviewedState == ReviewedState.AUTO_REVIEW) {
-                  break;
+                postAutoReviewed();
-                }
               }
+              reviewedState = ReviewedState.LOADED;
             }
 
             @Override
@@ -172,6 +177,14 @@ class Header extends Composite {
     }
   }
 
+  void autoReview() {
+    if (reviewedState == ReviewedState.LOADED && !reviewed.getValue()) {
+      postAutoReviewed();
+    } else {
+      reviewedState = ReviewedState.AUTO_REVIEW;
+    }
+  }
+
   void setChangeInfo(ChangeInfo info) {
     GitwebLink gw = Gerrit.getGitwebLink();
     if (gw != null) {
@@ -198,25 +211,33 @@ class Header extends Composite {
     prefsAction.setPartner(preferences);
   }
 
-  void setReviewed(boolean r) {
-    reviewed.setValue(r, true);
-  }
-
-  boolean isReviewed() {
-    return reviewed.getValue();
-  }
-
   @UiHandler("reviewed")
   void onValueChange(ValueChangeEvent<Boolean> event) {
-    RestApi api = ChangeApi.revision(patchSetId)
+    if (event.getValue()) {
+      reviewed().put(CallbackGroup.<ReviewInfo> emptyCallback());
+    } else {
+      reviewed().delete(CallbackGroup.<ReviewInfo> emptyCallback());
+    }
+  }
+
+  private void postAutoReviewed() {
+    reviewed().background().put(new AsyncCallback<ReviewInfo>() {
+        @Override
+        public void onSuccess(ReviewInfo result) {
+          reviewed.setValue(true, false);
+        }
+
+        @Override
+        public void onFailure(Throwable caught) {
+        }
+      });
+  }
+
+  private RestApi reviewed() {
+    return ChangeApi.revision(patchSetId)
         .view("files")
         .id(path)
         .view("reviewed");
-    if (event.getValue()) {
-      api.put(CallbackGroup.<ReviewInfo>emptyCallback());
-    } else {
-      api.delete(CallbackGroup.<ReviewInfo>emptyCallback());
-    }
   }
 
   @UiHandler("preferences")
@@ -255,6 +276,14 @@ class Header extends Composite {
     }
   }
 
+  Runnable toggleReviewed() {
+    return new Runnable() {
+      public void run() {
+        reviewed.setValue(!reviewed.getValue(), true);
+      }
+    };
+  }
+
   Runnable navigate(Direction dir) {
     switch (dir) {
       case PREV:

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PaddingManager.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.client.diff;
 
 import com.google.gwt.dom.client.Element;
+import com.google.gwt.dom.client.Style.Unit;
 
 import net.codemirror.lib.LineWidget;
 
@@ -88,7 +89,7 @@ class PaddingManager {
   }
 
   private void setPaddingHeight(int height) {
-    SideBySide2.setHeightInPx(wrapper.element, height);
+    wrapper.element.getStyle().setHeight((double) height, Unit.PX);
     wrapper.widget.changed();
   }
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesAction.java

@@ -56,7 +56,7 @@ class PreferencesAction {
     popup.addCloseHandler(new CloseHandler<PopupPanel>() {
       @Override
       public void onClose(CloseEvent<PopupPanel> event) {
-        view.getCmB().focus();
+        view.getCmFromSide(DisplaySide.B).focus();
         popup = null;
         current = null;
       }

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PreferencesBox.java

@@ -51,8 +51,6 @@ import com.google.gwt.user.client.ui.ListBox;
 import com.google.gwt.user.client.ui.PopupPanel;
 import com.google.gwt.user.client.ui.ToggleButton;
 
-import net.codemirror.lib.CodeMirror;
-
 /** Displays current diff preferences. */
 class PreferencesBox extends Composite {
   interface Binder extends UiBinder<HTMLPanel, PreferencesBox> {}
@@ -63,8 +61,6 @@ class PreferencesBox extends Composite {
   }
 
   private final SideBySide2 view;
-  private final CodeMirror cmA;
-  private final CodeMirror cmB;
   private DiffPreferences prefs;
   private int contextLastValue;
   private Timer updateContextTimer;
@@ -88,8 +84,6 @@ class PreferencesBox extends Composite {
 
   PreferencesBox(SideBySide2 view) {
     this.view = view;
-    this.cmA = view.getCmA();
-    this.cmB = view.getCmB();
 
     initWidget(uiBinder.createAndBindUi(this));
     initIgnoreWhitespace();
@@ -221,8 +215,9 @@ class PreferencesBox extends Composite {
       view.operation(new Runnable() {
         @Override
         public void run() {
-          cmA.setOption("tabSize", prefs.tabSize());
+          int v = prefs.tabSize();
-          cmB.setOption("tabSize", prefs.tabSize());
+          view.getCmFromSide(DisplaySide.A).setOption("tabSize", v);
+          view.getCmFromSide(DisplaySide.B).setOption("tabSize", v);
         }
       });
     }
@@ -231,7 +226,7 @@ class PreferencesBox extends Composite {
   @UiHandler("expandAllComments")
   void onExpandAllComments(ValueChangeEvent<Boolean> e) {
     prefs.expandAllComments(e.getValue());
-    view.setExpandAllComments(prefs.expandAllComments());
+    view.getCommentManager().setExpandAllComments(prefs.expandAllComments());
   }
 
   @UiHandler("showTabs")
@@ -271,8 +266,9 @@ class PreferencesBox extends Composite {
     view.operation(new Runnable() {
       @Override
       public void run() {
-        cmA.setOption("showTrailingSpace", prefs.showWhitespaceErrors());
+        boolean s = prefs.showWhitespaceErrors();
-        cmB.setOption("showTrailingSpace", prefs.showWhitespaceErrors());
+        view.getCmFromSide(DisplaySide.A).setOption("showTrailingSpace", s);
+        view.getCmFromSide(DisplaySide.B).setOption("showTrailingSpace", s);
       }
     });
   }

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/PublishedBox.java

@@ -68,15 +68,13 @@ class PublishedBox extends CommentBox {
   AvatarImage avatar;
 
   PublishedBox(
-      SideBySide2 parent,
+      CommentManager manager,
       CodeMirror cm,
-      DisplaySide side,
       CommentLinkProcessor clp,
       PatchSet.Id psId,
       CommentInfo info) {
-    super(cm, info, side);
+    super(manager, cm, info);
 
-    setDiffScreen(parent);
     this.psId = psId;
     this.comment = info;
 
@@ -142,19 +140,19 @@ class PublishedBox extends CommentBox {
   }
 
   DraftBox addReplyBox() {
-    DraftBox box = getDiffScreen().addDraftBox(
+    DraftBox box = getCommentManager().addDraftBox(
-        getDiffScreen().createReply(comment), getSide());
+        getCommentManager().createReply(comment), getCm().side());
     registerReplyBox(box);
     return box;
   }
 
   void doReply() {
     if (!Gerrit.isSignedIn()) {
-      Gerrit.doSignIn(getDiffScreen().getToken());
+      Gerrit.doSignIn(getCommentManager().getSideBySide2().getToken());
     } else if (replyBox == null) {
       DraftBox box = addReplyBox();
       if (!getCommentInfo().has_line()) {
-        getDiffScreen().addFileCommentBox(box);
+        getCommentManager().addFileCommentBox(box);
       }
     } else {
       openReplyBox();
@@ -171,10 +169,10 @@ class PublishedBox extends CommentBox {
   void onReplyDone(ClickEvent e) {
     e.stopPropagation();
     if (!Gerrit.isSignedIn()) {
-      Gerrit.doSignIn(getDiffScreen().getToken());
+      Gerrit.doSignIn(getCommentManager().getSideBySide2().getToken());
     } else if (replyBox == null) {
       done.setEnabled(false);
-      CommentInput input = CommentInput.create(getDiffScreen().createReply(comment));
+      CommentInput input = CommentInput.create(getCommentManager().createReply(comment));
       input.setMessage(PatchUtil.C.cannedReplyDone());
       CommentApi.createDraft(psId, input,
           new GerritCallback<CommentInfo>() {
@@ -182,10 +180,10 @@ class PublishedBox extends CommentBox {
             public void onSuccess(CommentInfo result) {
               done.setEnabled(true);
               setOpen(false);
-              DraftBox box = getDiffScreen().addDraftBox(result, getSide());
+              DraftBox box = getCommentManager().addDraftBox(result, getCm().side());
               registerReplyBox(box);
               if (!getCommentInfo().has_line()) {
-                getDiffScreen().addFileCommentBox(box);
+                getCommentManager().addFileCommentBox(box);
               }
             }
           });

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ScrollSynchronizer.java

@@ -26,7 +26,7 @@ class ScrollSynchronizer {
   private LineMapper mapper;
   private ScrollCallback active;
 
-  void init(DiffTable diffTable,
+  ScrollSynchronizer(DiffTable diffTable,
       CodeMirror cmA, CodeMirror cmB,
       LineMapper mapper) {
     this.diffTable = diffTable;

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java

@@ -21,8 +21,6 @@ import com.google.gerrit.client.changes.ChangeApi;
 import com.google.gerrit.client.changes.ChangeInfo;
 import com.google.gerrit.client.changes.ChangeInfo.RevisionInfo;
 import com.google.gerrit.client.changes.ChangeList;
-import com.google.gerrit.client.changes.CommentInfo;
-import com.google.gerrit.client.changes.ReviewInfo;
 import com.google.gerrit.client.diff.DiffInfo.Region;
 import com.google.gerrit.client.diff.DiffInfo.Span;
 import com.google.gerrit.client.diff.LineMapper.LineOnOtherInfo;
@@ -33,14 +31,11 @@ import com.google.gerrit.client.patches.SkippedLine;
 import com.google.gerrit.client.projects.ConfigInfoCache;
 import com.google.gerrit.client.rpc.CallbackGroup;
 import com.google.gerrit.client.rpc.GerritCallback;
-import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.rpc.RestApi;
 import com.google.gerrit.client.rpc.ScreenLoadCallback;
-import com.google.gerrit.client.ui.CommentLinkProcessor;
 import com.google.gerrit.client.ui.Screen;
 import com.google.gerrit.common.PageLinks;
 import com.google.gerrit.common.changes.ListChangesOption;
-import com.google.gerrit.common.changes.Side;
 import com.google.gerrit.reviewdb.client.AccountDiffPreference;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -89,7 +84,6 @@ import net.codemirror.lib.LineWidget;
 import net.codemirror.lib.ModeInjector;
 import net.codemirror.lib.Rect;
 import net.codemirror.lib.TextMarker;
-import net.codemirror.lib.TextMarker.FromTo;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -124,22 +118,16 @@ public class SideBySide2 extends Screen {
 
   private CodeMirror cmA;
   private CodeMirror cmB;
-  private ScrollSynchronizer scrollingGlue;
   private HandlerRegistration resizeHandler;
   private DiffInfo diff;
   private boolean largeFile;
   private LineMapper mapper;
   private List<TextMarker> markers;
   private List<Runnable> undoLineClass;
-  private CommentLinkProcessor commentLinkProcessor;
+  private CommentManager commentManager;
-  private Map<String, PublishedBox> publishedMap;
-  private Map<LineHandle, CommentBox> lineActiveBoxMap;
-  private Map<LineHandle, List<PublishedBox>> linePublishedBoxesMap;
-  private Map<LineHandle, PaddingManager> linePaddingManagerMap;
   private Map<LineHandle, LinePaddingWidgetWrapper> linePaddingOnOtherSideMap;
   private List<DiffChunkInfo> diffChunks;
   private Set<SkipBar> skipBars;
-  private Set<DraftBox> unsaved;
 
   private KeyCommandSet keysNavigation;
   private KeyCommandSet keysAction;
@@ -163,7 +151,6 @@ public class SideBySide2 extends Screen {
     this.startLine = startLine;
 
     prefs = DiffPreferences.create(Gerrit.getAccountDiffPreference());
-    unsaved = new HashSet<DraftBox>();
     handlers = new ArrayList<HandlerRegistration>(6);
     // TODO: Re-implement necessary GlobalKey bindings.
     addDomHandler(GlobalKey.STOP_PROPAGATION, KeyPressEvent.getType());
@@ -231,7 +218,10 @@ public class SideBySide2 extends Screen {
         new ScreenLoadCallback<ConfigInfoCache.Entry>(SideBySide2.this) {
           @Override
           protected void preDisplay(ConfigInfoCache.Entry result) {
-            commentLinkProcessor = result.getCommentLinkProcessor();
+            commentManager = new CommentManager(
+                SideBySide2.this,
+                base, revision, path,
+                result.getCommentLinkProcessor());
             setTheme(result.getTheme());
             display(comments);
           }
@@ -245,6 +235,12 @@ public class SideBySide2 extends Screen {
     if (prefs.hideTopMenu()) {
       Gerrit.setHeaderVisible(false);
     }
+    resizeHandler = Window.addResizeHandler(new ResizeHandler() {
+      @Override
+      public void onResize(ResizeEvent event) {
+        resizeCodeMirror();
+      }
+    });
 
     final int height = getCodeMirrorHeight();
     operation(new Runnable() {
@@ -282,8 +278,9 @@ public class SideBySide2 extends Screen {
       cmA.setCursor(LineCharacter.create(0));
       cmA.focus();
     }
-
+    if (Gerrit.isSignedIn() && prefs.autoReview()) {
-    autoReview();
+      header.autoReview();
+    }
     prefetchNextFile();
   }
 
@@ -291,8 +288,10 @@ public class SideBySide2 extends Screen {
   protected void onUnload() {
     super.onUnload();
 
-    saveAllDrafts(null);
+    removeKeyHandlerRegistrations();
-    removeKeyHandlerRegs();
+    if (commentManager != null) {
+      commentManager.saveAllDrafts(null);
+    }
     if (resizeHandler != null) {
       resizeHandler.removeHandler();
       resizeHandler = null;
@@ -311,7 +310,7 @@ public class SideBySide2 extends Screen {
     Gerrit.setHeaderVisible(true);
   }
 
-  private void removeKeyHandlerRegs() {
+  private void removeKeyHandlerRegistrations() {
     for (HandlerRegistration h : handlers) {
       h.removeHandler();
     }
@@ -322,7 +321,7 @@ public class SideBySide2 extends Screen {
     cm.on("beforeSelectionChange", onSelectionChange(cm));
     cm.on("cursorActivity", updateActiveLine(cm));
     cm.on("gutterClick", onGutterClick(cm));
-    cm.on("renderLine", resizeLinePadding(getSideFromCm(cm)));
+    cm.on("renderLine", resizeLinePadding(cm.side()));
     cm.on("viewportChange", adjustGutters(cm));
     cm.on("focus", new Runnable() {
       @Override
@@ -335,13 +334,13 @@ public class SideBySide2 extends Screen {
         .on("'u'", upToChange(false))
         .on("[", header.navigate(Direction.PREV))
         .on("]", header.navigate(Direction.NEXT))
-        .on("'r'", toggleReviewed())
+        .on("'r'", header.toggleReviewed())
-        .on("'o'", toggleOpenBox(cm))
+        .on("'o'", commentManager.toggleOpenBox(cm))
-        .on("Enter", toggleOpenBox(cm))
+        .on("Enter", commentManager.toggleOpenBox(cm))
-        .on("'c'", insertNewDraft(cm))
+        .on("'c'", commentManager.insertNewDraft(cm))
         .on("N", maybeNextVimSearch(cm))
         .on("P", diffChunkNav(cm, Direction.PREV))
-        .on("Shift-O", openClosePublished(cm))
+        .on("Shift-O", commentManager.openClosePublished(cm))
         .on("Shift-Left", moveCursorToSide(cm, DisplaySide.A))
         .on("Shift-Right", moveCursorToSide(cm, DisplaySide.B))
         .on("'i'", new Runnable() {
@@ -414,7 +413,7 @@ public class SideBySide2 extends Screen {
           @Override
           public void onClick(ClickEvent event) {
             icon.setVisible(false);
-            insertNewDraft(cm).run();
+            commentManager.insertNewDraft(cm).run();
           }
         });
         add(icon);
@@ -444,7 +443,7 @@ public class SideBySide2 extends Screen {
     keysAction.add(new KeyCommand(0, 'r', PatchUtil.C.toggleReviewed()) {
       @Override
       public void onKeyPress(KeyPressEvent event) {
-        toggleReviewed().run();
+        header.toggleReviewed().run();
       }
     });
     keysAction.add(new KeyCommand(0, 'a', PatchUtil.C.openReply()) {
@@ -479,7 +478,7 @@ public class SideBySide2 extends Screen {
     } else {
       keysComment = null;
     }
-    removeKeyHandlerRegs();
+    removeKeyHandlerRegistrations();
     handlers.add(GlobalKey.add(this, keysNavigation));
     if (keysComment != null) {
       handlers.add(GlobalKey.add(this, keysComment));
@@ -487,20 +486,15 @@ public class SideBySide2 extends Screen {
     handlers.add(GlobalKey.add(this, keysAction));
   }
 
-  private void display(final CommentsCollections data) {
+  private void display(final CommentsCollections comments) {
-    lineActiveBoxMap = new HashMap<LineHandle, CommentBox>();
-    linePublishedBoxesMap = new HashMap<LineHandle, List<PublishedBox>>();
-    linePaddingManagerMap = new HashMap<LineHandle, PaddingManager>();
-    publishedMap = new HashMap<String, PublishedBox>();
-
     setShowTabs(prefs.showTabs());
     setShowIntraline(prefs.intralineDifference());
     if (prefs.showLineNumbers()) {
       diffTable.addStyleName(DiffTable.style.showLineNumbers());
     }
 
-    cmA = createCodeMirror(diff.meta_a(), diff.text_a(), diffTable.cmA);
+    cmA = newCM(diff.meta_a(), diff.text_a(), DisplaySide.A, diffTable.cmA);
-    cmB = createCodeMirror(diff.meta_b(), diff.text_b(), diffTable.cmB);
+    cmB = newCM(diff.meta_b(), diff.text_b(), DisplaySide.B, diffTable.cmB);
 
     operation(new Runnable() {
       public void run() {
@@ -511,17 +505,9 @@ public class SideBySide2 extends Screen {
         cmB.setHeight(height);
 
         render(diff);
-        if (data.publishedBase != null) {
+        commentManager.render(comments);
-          renderPublished(DisplaySide.A, data.publishedBase);
+        if (prefs.expandAllComments()) {
-        }
+          commentManager.setExpandAllComments(true);
-        if (data.publishedRevision != null) {
-          renderPublished(DisplaySide.B, data.publishedRevision);
-        }
-        if (data.draftsBase != null) {
-          renderDrafts(DisplaySide.A, data.draftsBase);
-        }
-        if (data.draftsRevision != null) {
-          renderDrafts(DisplaySide.B, data.draftsRevision);
         }
         renderSkips(prefs.context());
       }
@@ -529,17 +515,10 @@ public class SideBySide2 extends Screen {
 
     registerCmEvents(cmA);
     registerCmEvents(cmB);
+    new ScrollSynchronizer(diffTable, cmA, cmB, mapper);
 
     prefsAction = new PreferencesAction(this, prefs);
-    scrollingGlue = GWT.create(ScrollSynchronizer.class);
-    scrollingGlue.init(diffTable, cmA, cmB, mapper);
     header.init(prefsAction);
-    resizeHandler = Window.addResizeHandler(new ResizeHandler() {
-      @Override
-      public void onResize(ResizeEvent event) {
-        resizeCodeMirror();
-      }
-    });
 
     if (largeFile && prefs.syntaxHighlighting()) {
       Scheduler.get().scheduleFixedDelay(new RepeatingCommand() {
@@ -554,9 +533,10 @@ public class SideBySide2 extends Screen {
     }
   }
 
-  private CodeMirror createCodeMirror(
+  private CodeMirror newCM(
       DiffInfo.FileMeta meta,
       String contents,
+      DisplaySide side,
       Element parent) {
     Configuration cfg = Configuration.create()
       .set("readOnly", true)
@@ -570,7 +550,7 @@ public class SideBySide2 extends Screen {
       .set("showTrailingSpace", prefs.showWhitespaceErrors())
       .set("keyMap", "vim_ro")
       .set("value", meta != null ? contents : "");
-    return CodeMirror.create(parent, cfg);
+    return CodeMirror.create(side, parent, cfg);
   }
 
   DiffInfo.IntraLineStatus getIntraLineStatus() {
@@ -642,12 +622,6 @@ public class SideBySide2 extends Screen {
     });
   }
 
-  void setExpandAllComments(boolean b) {
-    for (PublishedBox box : publishedMap.values()) {
-      box.setOpen(b);
-    }
-  }
-
   private void render(DiffInfo diff) {
     JsArray<Region> regions = diff.content();
 
@@ -710,212 +684,6 @@ public class SideBySide2 extends Screen {
     }
   }
 
-  private DraftBox addNewDraft(CodeMirror cm, int line, FromTo fromTo) {
-    DisplaySide side = getSideFromCm(cm);
-    return addDraftBox(CommentInfo.createRange(
-        path,
-        getStoredSideFromDisplaySide(side),
-        line + 1,
-        null,
-        null,
-        CommentRange.create(fromTo)), side);
-  }
-
-  CommentInfo createReply(CommentInfo replyTo) {
-    if (!replyTo.has_line() && replyTo.range() == null) {
-      return CommentInfo.createFile(path, replyTo.side(), replyTo.id(), null);
-    } else {
-      return CommentInfo.createRange(path, replyTo.side(), replyTo.line(),
-          replyTo.id(), null, replyTo.range());
-    }
-  }
-
-  DraftBox addDraftBox(CommentInfo info, DisplaySide side) {
-    CodeMirror cm = getCmFromSide(side);
-    final DraftBox box = new DraftBox(this, cm, side, commentLinkProcessor,
-        getPatchSetIdFromSide(side), info);
-    if (info.id() == null) {
-      Scheduler.get().scheduleDeferred(new ScheduledCommand() {
-        @Override
-        public void execute() {
-          box.setOpen(true);
-          box.setEdit(true);
-        }
-      });
-    }
-    if (!info.has_line()) {
-      return box;
-    }
-    addCommentBox(info, box);
-    box.setVisible(true);
-    LineHandle handle = cm.getLineHandle(info.line() - 1);
-    lineActiveBoxMap.put(handle, box);
-    return box;
-  }
-
-  CommentBox addCommentBox(CommentInfo info, final CommentBox box) {
-    diffTable.add(box);
-    DisplaySide side = box.getSide();
-    CodeMirror cm = getCmFromSide(side);
-    CodeMirror other = otherCm(cm);
-    int line = info.line() - 1; // CommentInfo is 1-based, but CM is 0-based
-    LineHandle handle = cm.getLineHandle(line);
-    PaddingManager manager;
-    if (linePaddingManagerMap.containsKey(handle)) {
-      manager = linePaddingManagerMap.get(handle);
-    } else {
-      // Estimated height at 28px, fixed by deferring after display
-      manager = new PaddingManager(addPaddingWidget(cm, line, 0, Unit.PX, 0));
-      linePaddingManagerMap.put(handle, manager);
-    }
-    int lineToPad = mapper.lineOnOther(side, line).getLine();
-    LineHandle otherHandle = other.getLineHandle(lineToPad);
-    DiffChunkInfo myChunk = getDiffChunk(side, line);
-    DiffChunkInfo otherChunk = getDiffChunk(getSideFromCm(other), lineToPad);
-    PaddingManager otherManager;
-    if (linePaddingManagerMap.containsKey(otherHandle)) {
-      otherManager = linePaddingManagerMap.get(otherHandle);
-    } else {
-      otherManager =
-          new PaddingManager(addPaddingWidget(other, lineToPad, 0, Unit.PX, 0));
-      linePaddingManagerMap.put(otherHandle, otherManager);
-    }
-    if ((myChunk == null && otherChunk == null) || (myChunk != null && otherChunk != null)) {
-      PaddingManager.link(manager, otherManager);
-    }
-    int index = manager.getCurrentCount();
-    manager.insert(box, index);
-    Configuration config = Configuration.create()
-      .set("coverGutter", true)
-      .set("insertAt", index)
-      .set("noHScroll", true);
-    LineWidget boxWidget = addLineWidget(cm, line, box, config);
-    box.setPaddingManager(manager);
-    box.setSelfWidgetWrapper(new PaddingWidgetWrapper(boxWidget, box.getElement()));
-    if (otherChunk == null) {
-      box.setDiffChunkInfo(myChunk);
-    }
-    box.setGutterWrapper(diffTable.sidePanel.addGutter(cm, info.line() - 1,
-        box instanceof DraftBox
-          ? SidePanel.GutterType.DRAFT
-          : SidePanel.GutterType.COMMENT));
-    if (box instanceof DraftBox) {
-      boxWidget.onRedraw(new Runnable() {
-        @Override
-        public void run() {
-          DraftBox draftBox = (DraftBox) box;
-          if (draftBox.isEdit()) {
-            draftBox.editArea.setFocus(true);
-          }
-        }
-      });
-    }
-    return box;
-  }
-
-  void removeDraft(DraftBox box, int line) {
-    LineHandle handle = getCmFromSide(box.getSide()).getLineHandle(line);
-    lineActiveBoxMap.remove(handle);
-    if (linePublishedBoxesMap.containsKey(handle)) {
-      List<PublishedBox> list = linePublishedBoxesMap.get(handle);
-      lineActiveBoxMap.put(handle, list.get(list.size() - 1));
-    }
-    unsaved.remove(box);
-  }
-
-  void updateUnsaved(DraftBox box, boolean isUnsaved) {
-    if (isUnsaved) {
-      unsaved.add(box);
-    } else {
-      unsaved.remove(box);
-    }
-  }
-
-  private void saveAllDrafts(CallbackGroup cb) {
-    for (DraftBox box : unsaved) {
-      box.save(cb);
-    }
-  }
-
-  void addFileCommentBox(CommentBox box) {
-    diffTable.addFileCommentBox(box);
-  }
-
-  void removeFileCommentBox(DraftBox box) {
-    diffTable.onRemoveDraftBox(box);
-  }
-
-  private void renderPublished(DisplaySide forSide, JsArray<CommentInfo> in) {
-    for (CommentInfo info : Natives.asList(in)) {
-      DisplaySide side;
-      if (info.side() == Side.PARENT) {
-        if (base != null) {
-          continue;
-        }
-        side = DisplaySide.A;
-      } else {
-        side = forSide;
-      }
-
-      CodeMirror cm = getCmFromSide(side);
-      PublishedBox box = new PublishedBox(this, cm, side, commentLinkProcessor,
-          getPatchSetIdFromSide(side), info);
-      if (prefs.expandAllComments()) {
-        box.setOpen(true);
-      }
-      publishedMap.put(info.id(), box);
-      if (!info.has_line()) {
-        diffTable.addFileCommentBox(box);
-        continue;
-      }
-      int line = info.line() - 1;
-      LineHandle handle = cm.getLineHandle(line);
-      if (linePublishedBoxesMap.containsKey(handle)) {
-        linePublishedBoxesMap.get(handle).add(box);
-      } else {
-        List<PublishedBox> list = new ArrayList<PublishedBox>();
-        list.add(box);
-        linePublishedBoxesMap.put(handle, list);
-      }
-      lineActiveBoxMap.put(handle, box);
-      addCommentBox(info, box);
-    }
-  }
-
-  private void renderDrafts(DisplaySide forSide, JsArray<CommentInfo> in) {
-    for (CommentInfo info : Natives.asList(in)) {
-      DisplaySide side;
-      if (info.side() == Side.PARENT) {
-        if (base != null) {
-          continue;
-        }
-        side = DisplaySide.A;
-      } else {
-        side = forSide;
-      }
-
-      DraftBox box = new DraftBox(
-          this, getCmFromSide(side), side, commentLinkProcessor,
-          getPatchSetIdFromSide(side), info);
-      if (prefs.expandAllComments()) {
-        box.setOpen(true);
-      }
-      if (info.in_reply_to() != null) {
-        PublishedBox replyToBox = publishedMap.get(info.in_reply_to());
-        if (replyToBox != null) {
-          replyToBox.registerReplyBox(box);
-        }
-      }
-      if (!info.has_line()) {
-        diffTable.addFileCommentBox(box);
-        continue;
-      }
-      lineActiveBoxMap.put(
-          getCmFromSide(side).getLineHandle(info.line() - 1), box);
-      addCommentBox(info, box);
-    }
-  }
-
   private void renderSkips(int context) {
     clearSkipBars();
 
@@ -946,42 +714,9 @@ public class SideBySide2 extends Screen {
         lineB += current.b() != null ? current.b().length() : 0;
       }
     }
+    skips = commentManager.splitSkips(context, skips);
 
-    // TODO: This is not optimal, but shouldn't be too costly in most cases.
-    // Maybe rewrite after done keeping track of diff chunk positions.
     skipBars = new HashSet<SkipBar>();
-    for (CommentBox box : lineActiveBoxMap.values()) {
-      List<SkippedLine> temp = new ArrayList<SkippedLine>();
-      for (SkippedLine skip : skips) {
-        CommentInfo info = box.getCommentInfo();
-        int startLine = box.getSide() == DisplaySide.A
-            ? skip.getStartA()
-            : skip.getStartB();
-        int boxLine = info.line();
-        int deltaBefore = boxLine - startLine;
-        int deltaAfter = startLine + skip.getSize() - boxLine;
-        if (deltaBefore < -context || deltaAfter < -context) {
-          temp.add(skip); // Size guaranteed to be greater than 1
-        } else if (deltaBefore > context && deltaAfter > context) {
-          SkippedLine before = new SkippedLine(
-              skip.getStartA(), skip.getStartB(),
-              skip.getSize() - deltaAfter - context);
-          skip.incrementStart(deltaBefore + context);
-          checkAndAddSkip(temp, before);
-          checkAndAddSkip(temp, skip);
-        } else if (deltaAfter > context) {
-          skip.incrementStart(deltaBefore + context);
-          checkAndAddSkip(temp, skip);
-        } else if (deltaBefore > context) {
-          skip.reduceSize(deltaAfter + context);
-          checkAndAddSkip(temp, skip);
-        }
-      }
-      if (temp.isEmpty()) {
-        return;
-      }
-      skips = temp;
-    }
     for (SkippedLine skip : skips) {
       SkipBar barA = renderSkipHelper(cmA, skip);
       SkipBar barB = renderSkipHelper(cmB, skip);
@@ -1034,12 +769,6 @@ public class SideBySide2 extends Screen {
     skipBars.remove(bar);
   }
 
-  private void checkAndAddSkip(List<SkippedLine> list, SkippedLine toAdd) {
-    if (toAdd.getSize() > 1) {
-      list.add(toAdd);
-    }
-  }
-
   private SkipBar renderSkipHelper(CodeMirror cm, SkippedLine skip) {
     int size = skip.getSize();
     int markStart = cm == cmA ? skip.getStartA() : skip.getStartB();
@@ -1065,24 +794,16 @@ public class SideBySide2 extends Screen {
     return bar;
   }
 
-  private CodeMirror otherCm(CodeMirror me) {
+  CodeMirror otherCm(CodeMirror me) {
     return me == cmA ? cmB : cmA;
   }
 
-  private PatchSet.Id getPatchSetIdFromSide(DisplaySide side) {
+  CodeMirror getCmFromSide(DisplaySide side) {
-    return side == DisplaySide.A && base != null ? base : revision;
-  }
-
-  private CodeMirror getCmFromSide(DisplaySide side) {
     return side == DisplaySide.A ? cmA : cmB;
   }
 
-  private DisplaySide getSideFromCm(CodeMirror cm) {
+  LineOnOtherInfo lineOnOther(DisplaySide side, int line) {
-    return cm == cmA ? DisplaySide.A : DisplaySide.B;
+    return mapper.lineOnOther(side, line);
-  }
-
-  Side getStoredSideFromDisplaySide(DisplaySide side) {
-    return side == DisplaySide.A && base == null ? Side.PARENT : Side.REVISION;
   }
 
   private void markEdit(CodeMirror cm, JsArrayString lines,
@@ -1143,11 +864,11 @@ public class SideBySide2 extends Screen {
     linePaddingOnOtherSideMap.put(otherCm.getLineHandle(lineOnOther),
         new LinePaddingWidgetWrapper(addPaddingWidget(cmToPad,
             lineToPad, 0, Unit.EM, null), lineToPad, chunkSize));
-    diffChunks.add(new DiffChunkInfo(getSideFromCm(otherCm),
+    diffChunks.add(new DiffChunkInfo(otherCm.side(),
         lineOnOther - chunkSize + 1, lineOnOther, edit));
   }
 
-  private PaddingWidgetWrapper addPaddingWidget(CodeMirror cm,
+  PaddingWidgetWrapper addPaddingWidget(CodeMirror cm,
       int line, double height, Unit unit, Integer index) {
     SimplePanel padding = new SimplePanel();
     padding.setStyleName(DiffTable.style.padding());
@@ -1170,7 +891,7 @@ public class SideBySide2 extends Screen {
    * DOM upon loading. Fix by hiding the widget until it is first scrolled into
    * view (when CodeMirror fires a "redraw" event on the widget).
    */
-  private LineWidget addLineWidget(CodeMirror cm, int line,
+  LineWidget addLineWidget(CodeMirror cm, int line,
       final Widget widget, Configuration options) {
     widget.setVisible(false);
     LineWidget lineWidget = cm.addLineWidget(line, widget.getElement(), options);
@@ -1232,7 +953,7 @@ public class SideBySide2 extends Screen {
             cm.addLineClass(
                 handle, LineClassWhere.WRAP, DiffTable.style.activeLine());
             LineOnOtherInfo info =
-                mapper.lineOnOther(getSideFromCm(cm), cm.getLineNumber(handle));
+                mapper.lineOnOther(cm.side(), cm.getLineNumber(handle));
             if (info.isAligned()) {
               LineHandle oLineHandle = other.getLineHandle(info.getLine());
               other.setActiveLine(oLineHandle);
@@ -1262,7 +983,7 @@ public class SideBySide2 extends Screen {
           Scheduler.get().scheduleDeferred(new ScheduledCommand() {
             @Override
             public void execute() {
-              insertNewDraft(cm).run();
+              commentManager.insertNewDraft(cm).run();
             }
           });
         }
@@ -1270,107 +991,25 @@ public class SideBySide2 extends Screen {
     };
   }
 
-  private Runnable insertNewDraft(final CodeMirror cm) {
-    if (!Gerrit.isSignedIn()) {
-      return new Runnable() {
-        @Override
-        public void run() {
-          Gerrit.doSignIn(getToken());
-        }
-     };
-    }
-    return new Runnable() {
-      public void run() {
-        LineHandle handle = cm.getActiveLine();
-        int line = cm.getLineNumber(handle);
-        CommentBox box = lineActiveBoxMap.get(handle);
-        FromTo fromTo = cm.getSelectedRange();
-        if (cm.somethingSelected()) {
-          lineActiveBoxMap.put(handle,
-              addNewDraft(cm, line, fromTo.getTo().getLine() == line ? fromTo : null));
-          cm.setSelection(cm.getCursor());
-        } else if (box == null) {
-          lineActiveBoxMap.put(handle, addNewDraft(cm, line, null));
-        } else if (box instanceof DraftBox) {
-          ((DraftBox) box).setEdit(true);
-        } else {
-          ((PublishedBox) box).doReply();
-        }
-      }
-    };
-  }
-
-  private Runnable toggleOpenBox(final CodeMirror cm) {
-    return new Runnable() {
-      public void run() {
-        CommentBox box = lineActiveBoxMap.get(cm.getActiveLine());
-        if (box != null) {
-          box.setOpen(!box.isOpen());
-        }
-      }
-    };
-  }
-
   private Runnable upToChange(final boolean openReplyBox) {
     return new Runnable() {
       public void run() {
-        if (unsaved.isEmpty()) {
+        CallbackGroup group = new CallbackGroup();
-          goUpToChange(openReplyBox);
+        commentManager.saveAllDrafts(group);
-        } else {
+        group.addFinal(new GerritCallback<Void>() {
-          CallbackGroup group = new CallbackGroup();
+          @Override
-          saveAllDrafts(group);
+          public void onSuccess(Void result) {
-          group.addFinal(new GerritCallback<Void>() {
+            String b = base != null ? String.valueOf(base.get()) : null;
-            @Override
+            String rev = String.valueOf(revision.get());
-            public void onSuccess(Void result) {
+            Gerrit.display(
-              goUpToChange(openReplyBox);
+              PageLinks.toChange(changeId, rev),
-            }
+              new ChangeScreen2(changeId, b, rev, openReplyBox));
-          }).onSuccess(null);
+          }
-        }
+        }).onSuccess(null);
       }
     };
   }
 
-  private void goUpToChange(boolean openReplyBox) {
-    String b = base != null ? String.valueOf(base.get()) : null;
-    String rev = String.valueOf(revision.get());
-    Gerrit.display(
-      PageLinks.toChange(changeId, rev),
-      new ChangeScreen2(changeId, b, rev, openReplyBox));
-  }
-
-  private Runnable openClosePublished(final CodeMirror cm) {
-    return new Runnable() {
-      @Override
-      public void run() {
-        if (cm.hasActiveLine()) {
-          List<PublishedBox> list =
-              linePublishedBoxesMap.get(cm.getActiveLine());
-          if (list == null) {
-            return;
-          }
-          boolean open = false;
-          for (PublishedBox box : list) {
-            if (!box.isOpen()) {
-              open = true;
-              break;
-            }
-          }
-          for (PublishedBox box : list) {
-            box.setOpen(open);
-          }
-        }
-      }
-    };
-  }
-
-  private Runnable toggleReviewed() {
-    return new Runnable() {
-     public void run() {
-       header.setReviewed(!header.isReviewed());
-     }
-    };
-  }
-
   private Runnable moveCursorToSide(final CodeMirror cmSrc, DisplaySide sideDst) {
     final CodeMirror cmDst = getCmFromSide(sideDst);
     if (cmDst == cmSrc) {
@@ -1381,7 +1020,7 @@ public class SideBySide2 extends Screen {
       };
     }
 
-    final DisplaySide sideSrc = getSideFromCm(cmSrc);
+    final DisplaySide sideSrc = cmSrc.side();
     return new Runnable() {
       public void run() {
         if (cmSrc.hasActiveLine()) {
@@ -1414,7 +1053,7 @@ public class SideBySide2 extends Screen {
         int line = cm.hasActiveLine() ? cm.getLineNumber(cm.getActiveLine()) : 0;
         int res = Collections.binarySearch(
                 diffChunks,
-                new DiffChunkInfo(getSideFromCm(cm), line, 0, false),
+                new DiffChunkInfo(cm.side(), line, 0, false),
                 getDiffChunkComparator());
         if (res < 0) {
           res = -res - (dir == Direction.PREV ? 1 : 2);
@@ -1469,7 +1108,7 @@ public class SideBySide2 extends Screen {
     };
   }
 
-  private DiffChunkInfo getDiffChunk(DisplaySide side, int line) {
+  DiffChunkInfo getDiffChunk(DisplaySide side, int line) {
     int res = Collections.binarySearch(
         diffChunks,
         new DiffChunkInfo(side, line, 0, false), // Dummy DiffChunkInfo
@@ -1532,25 +1171,25 @@ public class SideBySide2 extends Screen {
       double delta = myChunkHeight - otherChunkHeight;
       if (delta > 0) {
         if (myPaddingHeight != 0) {
-          setHeightInPx(myPadding, 0);
+          myPadding.getStyle().setHeight((double) 0, Unit.PX);
           myWrapper.getWidget().changed();
         }
         if (otherPaddingHeight != delta) {
-          setHeightInPx(otherPadding, delta);
+          otherPadding.getStyle().setHeight(delta, Unit.PX);
           otherWrapper.getWidget().changed();
         }
       } else {
         if (myPaddingHeight != -delta) {
-          setHeightInPx(myPadding, -delta);
+          myPadding.getStyle().setHeight(-delta, Unit.PX);
           myWrapper.getWidget().changed();
         }
         if (otherPaddingHeight != 0) {
-          setHeightInPx(otherPadding, 0);
+          otherPadding.getStyle().setHeight((double) 0, Unit.PX);
           otherWrapper.getWidget().changed();
         }
       }
     } else if (otherPaddingHeight != myChunkHeight) {
-      setHeightInPx(otherPadding, myChunkHeight);
+      otherPadding.getStyle().setHeight(myChunkHeight, Unit.PX);
       otherWrapper.getWidget().changed();
     }
   }
@@ -1561,9 +1200,7 @@ public class SideBySide2 extends Screen {
       @Override
       public void handle(final CodeMirror instance, final LineHandle handle,
           Element element) {
-        if (lineActiveBoxMap.containsKey(handle)) {
+        commentManager.resizePadding(handle);
-          lineActiveBoxMap.get(handle).resizePaddingWidget();
-        }
         if (linePaddingOnOtherSideMap.containsKey(handle)) {
           defer(new Runnable() {
             @Override
@@ -1591,10 +1228,6 @@ public class SideBySide2 extends Screen {
     return Window.getClientHeight() - rest;
   }
 
-  static void setHeightInPx(Element ele, double height) {
-    ele.getStyle().setHeight(height, Unit.PX);
-  }
-
   private String getContentType(DiffInfo.FileMeta meta) {
     return prefs.syntaxHighlighting()
           && meta != null
@@ -1614,12 +1247,8 @@ public class SideBySide2 extends Screen {
     return prefs;
   }
 
-  CodeMirror getCmA() {
+  CommentManager getCommentManager() {
-    return cmA;
+    return commentManager;
-  }
-
-  CodeMirror getCmB() {
-    return cmB;
   }
 
   void operation(final Runnable apply) {
@@ -1636,25 +1265,6 @@ public class SideBySide2 extends Screen {
     });
   }
 
-  private void autoReview() {
-    if (Gerrit.isSignedIn() && prefs.autoReview()) {
-      ChangeApi.revision(revision)
-        .view("files")
-        .id(path)
-        .view("reviewed")
-        .background()
-        .put(new AsyncCallback<ReviewInfo>() {
-            @Override
-            public void onSuccess(ReviewInfo result) {
-              header.reviewed.setValue(true, false);
-            }
-            @Override
-            public void onFailure(Throwable caught) {
-            }
-          });
-    }
-  }
-
   private void prefetchNextFile() {
     String nextPath = header.getNextPath();
     if (nextPath != null) {

gerrit-gwtui/src/main/java/com/google/gerrit/client/rpc/Natives.java

@@ -16,6 +16,7 @@ package com.google.gerrit.client.rpc;
 
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.JsArray;
+import com.google.gwt.core.client.JsArrayString;
 import com.google.gwt.json.client.JSONObject;
 
 import java.util.AbstractList;
@@ -35,6 +36,30 @@ public class Natives {
     return Collections.emptySet();
   }
 
+  public static List<String> asList(final JsArrayString arr) {
+    if (arr == null) {
+      return null;
+    }
+    return new AbstractList<String>() {
+      @Override
+      public String set(int index, String element) {
+        String old = arr.get(index);
+        arr.set(index, element);
+        return old;
+      }
+
+      @Override
+      public String get(int index) {
+        return arr.get(index);
+      }
+
+      @Override
+      public int size() {
+        return arr.length();
+      }
+    };
+  }
+
   public static <T extends JavaScriptObject> List<T> asList(
       final JsArray<T> arr) {
     if (arr == null) {

gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java

@@ -14,6 +14,7 @@
 
 package net.codemirror.lib;
 
+import com.google.gerrit.client.diff.DisplaySide;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.dom.client.Element;
 import com.google.gwt.dom.client.NativeEvent;
@@ -31,9 +32,13 @@ public class CodeMirror extends JavaScriptObject {
     Loader.initLibrary(cb);
   }
 
-  public static native CodeMirror create(Element parent,
+  public static native CodeMirror create(
+      DisplaySide side,
+      Element parent,
       Configuration cfg) /*-{
-    return $wnd.CodeMirror(parent, cfg);
+    var m = $wnd.CodeMirror(parent, cfg);
+    m._sbs2_side = side;
+    return m;
   }-*/;
 
   public final native void setOption(String option, boolean value) /*-{
@@ -327,6 +332,8 @@ public class CodeMirror extends JavaScriptObject {
         !!this.state.vim.searchState_.getOverlay();
   }-*/;
 
+  public final native DisplaySide side() /*-{ return this._sbs2_side }-*/;
+
   protected CodeMirror() {
   }