SideBySide2: Fix alignment for long insertion/deletion blocks

In commit 1dbc53126cafc0f553b709 the line-height was changed to
"normal", allowing for taller text lines. Padding for insertion
or deletion blocks was no longer tall enough, which caused the
two sides to stop aligning.

Correct this by computing the line height on the fly after the
UI is displayed and the browser was able to compute font metrics
to a sampling of 10 lines of text.

Use the guessed value from a prior rendering for the new one,
reducing any reflow that has to occur upon viewing another
file in the same application session.

Chrome on Linux is showing the lines are 15px tall, so use
that as the current default guess, reducing initial reflow
for any platform that has this font height.

Bug: issue 2970
Change-Id: Ic5f7dbbd1cc3582388cfcdf6faf9dca1f7a85fa0
(cherry picked from commit 941d32cff210e3bec4e114ea35822b017faf18f1)

3 files changed, 57 insertions, 10 deletions

diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java
index dcea18c74f..bc13ac64ea 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/ChunkManager.java
@@ -46,6 +46,8 @@ import java.util.List;
 
 /** Colors modified regions for {@link SideBySide2}. */
 class ChunkManager {
+  private static final String DATA_LINES = "_cs2h";
+  private static double guessedLineHeightPx = 15;
   private static final JavaScriptObject focusA = initOnClick(A);
   private static final JavaScriptObject focusB = initOnClick(B);
   private static final native JavaScriptObject initOnClick(DisplaySide s) /*-{
@@ -84,6 +86,7 @@ class ChunkManager {
   private List<TextMarker> markers;
   private List<Runnable> undo;
   private List<LineWidget> padding;
+  private List<Element> paddingDivs;
 
   ChunkManager(SideBySide2 host,
       CodeMirror cmA,
@@ -122,6 +125,7 @@ class ChunkManager {
     markers = new ArrayList<>();
     undo = new ArrayList<>();
     padding = new ArrayList<>();
+    paddingDivs = new ArrayList<>();
 
     String diffColor = diff.meta_a() == null || diff.meta_b() == null
         ? DiffTable.style.intralineBg()
@@ -138,6 +142,25 @@ class ChunkManager {
         render(current, diffColor);
       }
     }
+
+    if (paddingDivs.isEmpty()) {
+      paddingDivs = null;
+    }
+  }
+
+  void adjustPadding() {
+    if (paddingDivs != null) {
+      double h = host.getLineHeightPx();
+      for (Element div : paddingDivs) {
+        int lines = div.getPropertyInt(DATA_LINES);
+        div.getStyle().setHeight(lines * h, Unit.PX);
+      }
+      for (LineWidget w : padding) {
+        w.changed();
+      }
+      paddingDivs = null;
+      guessedLineHeightPx = h;
+    }
   }
 
   private void render(Region region, String diffColor) {
@@ -243,17 +266,14 @@ class ChunkManager {
    * @param len number of lines to pad. Padding is inserted only if
    *        {@code len >= 1}.
    */
-  private void addPadding(CodeMirror cm, int line, int len) {
+  private void addPadding(CodeMirror cm, int line, final int len) {
     if (0 < len) {
-      // DiffTable adds 1px bottom padding to each line to preserve
-      // sufficient space for underscores commonly appearing in code.
-      // Padding should be 1em + 1px high for each line. Add within
-      // the browser using height + padding-bottom.
       Element pad = DOM.createDiv();
       pad.setClassName(DiffTable.style.padding());
-      pad.getStyle().setHeight(len, Unit.EM);
-      pad.getStyle().setPaddingBottom(len, Unit.PX);
+      pad.setPropertyInt(DATA_LINES, len);
+      pad.getStyle().setHeight(guessedLineHeightPx * len, Unit.PX);
       focusOnClick(pad, cm.side());
+      paddingDivs.add(pad);
       padding.add(cm.addLineWidget(
         line == -1 ? 0 : line,
         pad,
diff --git 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
index a8e4df051d..30306f236a 100644
--- 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
@@ -116,6 +116,8 @@ public class SideBySide2 extends Screen {
   private Element columnMarginA;
   private Element columnMarginB;
   private double charWidthPx;
+  private double lineHeightPx;
+
   private HandlerRegistration resizeHandler;
   private ScrollSynchronizer scrollSynchronizer;
   private DiffInfo diff;
@@ -243,6 +245,7 @@ public class SideBySide2 extends Screen {
       public void run() {
         cmA.setHeight(height);
         cmB.setHeight(height);
+        chunkManager.adjustPadding();
         cmA.refresh();
         cmB.refresh();
       }
@@ -636,6 +639,26 @@ public class SideBySide2 extends Screen {
     columnMarginB.getStyle().setMarginLeft(w, Style.Unit.PX);
   }
 
+  double getLineHeightPx() {
+    if (lineHeightPx <= 1) {
+      Element p = DOM.createDiv();
+      int lines = 1;
+      for (int i = 0; i < lines; i++) {
+        Element e = DOM.createDiv();
+        p.appendChild(e);
+
+        Element pre = DOM.createElement("pre");
+        pre.setInnerText("gqyŚŻŹŃ");
+        e.appendChild(pre);
+      }
+
+      cmB.getMeasureElement().appendChild(p);
+      lineHeightPx = ((double) p.getOffsetHeight()) / lines;
+      p.removeFromParent();
+    }
+    return lineHeightPx;
+  }
+
   private double getCharWidthPx() {
     if (charWidthPx <= 1) {
       int len = 100;
@@ -647,11 +670,11 @@ public class SideBySide2 extends Screen {
       e.getStyle().setDisplay(Style.Display.INLINE_BLOCK);
       e.setInnerText(s.toString());
 
-      cmA.getMoverElement().appendChild(e);
+      cmA.getMeasureElement().appendChild(e);
       double a = ((double) e.getOffsetWidth()) / len;
       e.removeFromParent();
 
-      cmB.getMoverElement().appendChild(e);
+      cmB.getMeasureElement().appendChild(e);
       double b = ((double) e.getOffsetWidth()) / len;
       e.removeFromParent();
       charWidthPx = Math.max(a, b);
@@ -1001,6 +1024,7 @@ public class SideBySide2 extends Screen {
                 diffTable.overview.clearDiffMarkers();
                 setShowIntraline(prefs.intralineDifference());
                 render(diff);
+                chunkManager.adjustPadding();
                 skipManager.render(prefs.context(), diff);
               }
             });
diff --git a/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java b/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
index 941ef7a30b..85b1fa67f8 100644
--- a/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
+++ b/gerrit-gwtui/src/main/java/net/codemirror/lib/CodeMirror.java
@@ -64,7 +64,6 @@ public class CodeMirror extends JavaScriptObject {
   public final native void setWidth(String w) /*-{ this.setSize(w, null); }-*/;
   public final native void setHeight(double h) /*-{ this.setSize(null, h); }-*/;
   public final native void setHeight(String h) /*-{ this.setSize(null, h); }-*/;
-  public final native double defaultCharWidth() /*-{ return this.defaultCharWidth() }-*/;
   public final native String getLine(int n) /*-{ return this.getLine(n) }-*/;
 
   public final native void refresh() /*-{ this.refresh(); }-*/;
@@ -291,6 +290,10 @@ public class CodeMirror extends JavaScriptObject {
     return this.display.mover;
   }-*/;
 
+  public final native Element getMeasureElement() /*-{
+    return this.display.measure;
+  }-*/;
+
   public final native Element getScrollbarV() /*-{
     return this.display.scrollbarV;
   }-*/;