Merge "Document that single quotes are illegal in notify filters"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 0fc733a..67ae65c 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -6017,6 +6017,10 @@
 The numeric Change-Id of the change that this change reverts.
 |`submission_id`      |optional|
 ID of the submission of this change. Only set if the status is `MERGED`.
+This ID is equal to the numeric ID of the change that triggered the submission.
+If the change that triggered the submission also has a topic, it will be
+"<id>-<topic>" of the change that triggered the submission.
+The callers must not rely on the format of the submission ID.
 |==================================
 
 [[change-input]]
diff --git a/java/com/google/gerrit/entities/SubmissionId.java b/java/com/google/gerrit/entities/SubmissionId.java
new file mode 100644
index 0000000..eb03a5a
--- /dev/null
+++ b/java/com/google/gerrit/entities/SubmissionId.java
@@ -0,0 +1,34 @@
+// Copyright (C) 2019 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.entities;
+
+import org.eclipse.jgit.annotations.Nullable;
+
+public class SubmissionId {
+  private final String submissionId;
+
+  public SubmissionId(Change.Id changeId, @Nullable String topic) {
+    submissionId = topic != null ? String.format("%s-%s", changeId, topic) : changeId.toString();
+  }
+
+  public SubmissionId(Change change) {
+    this(change.getId(), change.getTopic());
+  }
+
+  @Override
+  public String toString() {
+    return submissionId;
+  }
+}
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 568fb60..da8411f 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -538,14 +538,14 @@
 
   private Injector createWebInjector() {
     final List<Module> modules = new ArrayList<>();
-    if (sshd) {
-      modules.add(new ProjectQoSFilter.Module());
-    }
     modules.add(RequestContextFilter.module());
     modules.add(RequestMetricsFilter.module());
     modules.add(H2CacheBasedWebSession.module());
     modules.add(sysInjector.getInstance(GerritAuthModule.class));
     modules.add(sysInjector.getInstance(GitOverHttpModule.class));
+    if (sshd) {
+      modules.add(new ProjectQoSFilter.Module());
+    }
     modules.add(AllRequestFilter.module());
     modules.add(sysInjector.getInstance(WebModule.class));
     modules.add(sysInjector.getInstance(RequireSslFilter.Module.class));
diff --git a/java/com/google/gerrit/prettify/BUILD b/java/com/google/gerrit/prettify/BUILD
index 7c1241a..0a15fda 100644
--- a/java/com/google/gerrit/prettify/BUILD
+++ b/java/com/google/gerrit/prettify/BUILD
@@ -7,5 +7,7 @@
     deps = [
         "//lib:guava",
         "//lib:jgit",
+        "//lib/auto:auto-value",
+        "//lib/auto:auto-value-annotations",
     ],
 )
diff --git a/java/com/google/gerrit/prettify/common/SparseFileContent.java b/java/com/google/gerrit/prettify/common/SparseFileContent.java
index 348f9b2..1249b65 100644
--- a/java/com/google/gerrit/prettify/common/SparseFileContent.java
+++ b/java/com/google/gerrit/prettify/common/SparseFileContent.java
@@ -14,160 +14,175 @@
 
 package com.google.gerrit.prettify.common;
 
-import java.util.ArrayList;
-import java.util.List;
+import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
 
-public class SparseFileContent {
-  protected List<Range> ranges;
-  protected int size;
+/**
+ * A class to store subset of a file's lines in a memory efficient way. Internally, it stores lines
+ * as a list of ranges. Each range represents continuous set of lines and has information about line
+ * numbers in original file (zero-based).
+ *
+ * <p>{@link SparseFileContent.Accessor} must be used to work with the stored content.
+ */
+@AutoValue
+public abstract class SparseFileContent {
+  abstract ImmutableList<Range> getRanges();
 
-  private transient int currentRangeIdx;
+  public abstract int getSize();
 
-  public SparseFileContent() {
-    ranges = new ArrayList<>();
+  public static SparseFileContent create(ImmutableList<Range> ranges, int size) {
+    return new AutoValue_SparseFileContent(ranges, size);
   }
 
-  public int size() {
-    return size;
+  @VisibleForTesting
+  public int getRangesCount() {
+    return getRanges().size();
   }
 
-  public void setSize(int s) {
-    size = s;
+  public Accessor createAccessor() {
+    return new Accessor(this);
   }
 
-  public String get(int idx) {
-    final String line = getLine(idx);
-    if (line == null) {
-      throw new ArrayIndexOutOfBoundsException(idx);
-    }
-    return line;
-  }
+  /**
+   * Provide a methods to work with the content of a {@link SparseFileContent}.
+   *
+   * <p>The class hides internal representation of a {@link SparseFileContent} and provides
+   * convenient way for accessing a content.
+   */
+  public static class Accessor {
+    private final SparseFileContent content;
+    private int currentRangeIdx;
 
-  public boolean contains(int idx) {
-    return getLine(idx) != null;
-  }
-
-  public int first() {
-    return ranges.isEmpty() ? size() : ranges.get(0).base;
-  }
-
-  public int next(int idx) {
-    // Most requests are sequential in nature, fetching the next
-    // line from the current range, or the immediate next range.
-    //
-    int high = ranges.size();
-    if (currentRangeIdx < high) {
-      Range cur = ranges.get(currentRangeIdx);
-      if (cur.contains(idx + 1)) {
-        return idx + 1;
-      }
-
-      if (++currentRangeIdx < high) {
-        // Its not plus one, its the base of the next range.
-        //
-        return ranges.get(currentRangeIdx).base;
-      }
+    private Accessor(SparseFileContent content) {
+      this.content = content;
     }
 
-    // Binary search for the current value, since we know its a sorted list.
-    //
-    int low = 0;
-    do {
-      final int mid = (low + high) / 2;
-      final Range cur = ranges.get(mid);
+    public String get(int idx) {
+      final String line = getLine(idx);
+      if (line == null) {
+        throw new ArrayIndexOutOfBoundsException(idx);
+      }
+      return line;
+    }
 
-      if (cur.contains(idx)) {
+    public int getSize() {
+      return content.getSize();
+    }
+
+    public boolean contains(int idx) {
+      return getLine(idx) != null;
+    }
+
+    public int first() {
+      return content.getRanges().isEmpty() ? getSize() : content.getRanges().get(0).getBase();
+    }
+
+    public int next(int idx) {
+      // Most requests are sequential in nature, fetching the next
+      // line from the current range, or the immediate next range.
+      //
+      ImmutableList<Range> ranges = content.getRanges();
+      int high = ranges.size();
+      if (currentRangeIdx < high) {
+        Range cur = ranges.get(currentRangeIdx);
         if (cur.contains(idx + 1)) {
-          // Trivial plus one case above failed due to wrong currentRangeIdx.
-          // Reset the cache so we don't miss in the future.
-          //
-          currentRangeIdx = mid;
           return idx + 1;
         }
 
-        if (mid + 1 < ranges.size()) {
-          // Its the base of the next range.
-          currentRangeIdx = mid + 1;
-          return ranges.get(currentRangeIdx).base;
-        }
-
-        // No more lines in the file.
-        //
-        return size();
-      }
-
-      if (idx < cur.base) {
-        high = mid;
-      } else {
-        low = mid + 1;
-      }
-    } while (low < high);
-
-    return size();
-  }
-
-  private String getLine(int idx) {
-    // Most requests are sequential in nature, fetching the next
-    // line from the current range, or the next range.
-    //
-    int high = ranges.size();
-    if (currentRangeIdx < high) {
-      Range cur = ranges.get(currentRangeIdx);
-      if (cur.contains(idx)) {
-        return cur.get(idx);
-      }
-
-      if (++currentRangeIdx < high) {
-        final Range next = ranges.get(currentRangeIdx);
-        if (next.contains(idx)) {
-          return next.get(idx);
+        if (++currentRangeIdx < high) {
+          // Its not plus one, its the base of the next range.
+          //
+          return ranges.get(currentRangeIdx).getBase();
         }
       }
+
+      // Binary search for the current value, since we know its a sorted list.
+      //
+      int low = 0;
+      do {
+        final int mid = (low + high) / 2;
+        final Range cur = ranges.get(mid);
+
+        if (cur.contains(idx)) {
+          if (cur.contains(idx + 1)) {
+            // Trivial plus one case above failed due to wrong currentRangeIdx.
+            // Reset the cache so we don't miss in the future.
+            //
+            currentRangeIdx = mid;
+            return idx + 1;
+          }
+
+          if (mid + 1 < ranges.size()) {
+            // Its the base of the next range.
+            currentRangeIdx = mid + 1;
+            return ranges.get(currentRangeIdx).getBase();
+          }
+
+          // No more lines in the file.
+          //
+          return getSize();
+        }
+
+        if (idx < cur.getBase()) {
+          high = mid;
+        } else {
+          low = mid + 1;
+        }
+      } while (low < high);
+
+      return getSize();
     }
 
-    // Binary search for the range, since we know its a sorted list.
-    //
-    if (ranges.isEmpty()) {
+    private String getLine(int idx) {
+      // Most requests are sequential in nature, fetching the next
+      // line from the current range, or the next range.
+      //
+      ImmutableList<Range> ranges = content.getRanges();
+      int high = ranges.size();
+      if (currentRangeIdx < high) {
+        Range cur = ranges.get(currentRangeIdx);
+        if (cur.contains(idx)) {
+          return cur.get(idx);
+        }
+
+        if (++currentRangeIdx < high) {
+          final Range next = ranges.get(currentRangeIdx);
+          if (next.contains(idx)) {
+            return next.get(idx);
+          }
+        }
+      }
+
+      // Binary search for the range, since we know its a sorted list.
+      //
+      if (ranges.isEmpty()) {
+        return null;
+      }
+
+      int low = 0;
+      do {
+        final int mid = (low + high) / 2;
+        final Range cur = ranges.get(mid);
+        if (cur.contains(idx)) {
+          currentRangeIdx = mid;
+          return cur.get(idx);
+        }
+        if (idx < cur.getBase()) {
+          high = mid;
+        } else {
+          low = mid + 1;
+        }
+      } while (low < high);
       return null;
     }
-
-    int low = 0;
-    do {
-      final int mid = (low + high) / 2;
-      final Range cur = ranges.get(mid);
-      if (cur.contains(idx)) {
-        currentRangeIdx = mid;
-        return cur.get(idx);
-      }
-      if (idx < cur.base) {
-        high = mid;
-      } else {
-        low = mid + 1;
-      }
-    } while (low < high);
-    return null;
-  }
-
-  public void addLine(int i, String content) {
-    final Range r;
-    if (!ranges.isEmpty() && i == last().end()) {
-      r = last();
-    } else {
-      r = new Range(i);
-      ranges.add(r);
-    }
-    r.lines.add(content);
-  }
-
-  private Range last() {
-    return ranges.get(ranges.size() - 1);
   }
 
   @Override
-  public String toString() {
+  public final String toString() {
     final StringBuilder b = new StringBuilder();
     b.append("SparseFileContent[\n");
-    for (Range r : ranges) {
+    for (Range r : getRanges()) {
       b.append("  ");
       b.append(r.toString());
       b.append('\n');
@@ -176,33 +191,32 @@
     return b.toString();
   }
 
-  static class Range {
-    protected int base;
-    protected List<String> lines;
-
-    private Range(int b) {
-      base = b;
-      lines = new ArrayList<>();
+  @AutoValue
+  abstract static class Range {
+    static Range create(int base, ImmutableList<String> lines) {
+      return new AutoValue_SparseFileContent_Range(base, lines);
     }
 
-    protected Range() {}
+    abstract int getBase();
+
+    abstract ImmutableList<String> getLines();
 
     private String get(int i) {
-      return lines.get(i - base);
+      return getLines().get(i - getBase());
     }
 
     private int end() {
-      return base + lines.size();
+      return getBase() + getLines().size();
     }
 
     private boolean contains(int i) {
-      return base <= i && i < end();
+      return getBase() <= i && i < end();
     }
 
     @Override
-    public String toString() {
+    public final String toString() {
       // Usage of [ and ) is intentional to denote inclusive/exclusive range
-      return "Range[" + base + "," + end() + ")";
+      return "Range[" + getBase() + "," + end() + ")";
     }
   }
 }
diff --git a/java/com/google/gerrit/prettify/common/SparseFileContentBuilder.java b/java/com/google/gerrit/prettify/common/SparseFileContentBuilder.java
new file mode 100644
index 0000000..04fb5d1
--- /dev/null
+++ b/java/com/google/gerrit/prettify/common/SparseFileContentBuilder.java
@@ -0,0 +1,90 @@
+// Copyright (C) 2019 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.prettify.common;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.prettify.common.SparseFileContent.Range;
+
+/**
+ * A builder for creating immutable {@link SparseFileContent}. Lines can be only be added in
+ * sequential (increased) order
+ */
+public class SparseFileContentBuilder {
+  private final ImmutableList.Builder<Range> ranges;
+  private final int size;
+  private int lastRangeBase;
+  private int lastRangeEnd;
+  private ImmutableList.Builder<String> lastRangeLines;
+
+  public SparseFileContentBuilder(int size) {
+    ranges = new ImmutableList.Builder<>();
+    startNextRange(0);
+    this.size = size;
+  }
+
+  public void addLine(int lineNumber, String content) {
+    if (lineNumber < 0) {
+      throw new IllegalArgumentException("Line number must be non-negative");
+    }
+    //    if (lineNumber >= size) {
+    //     The following 4 tests are failed if you uncomment this condition:
+    //
+    //
+    // diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+    //
+    // diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+    //
+    //
+    // diffOfFileWithMultilineRebaseHunkRemovingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+    //
+    // diffOfFileWithMultilineRebaseHunkAddingNewlineAtEndOfFileAndWithCommentReturnsFileContents
+    //     Tests are failed because there are some bug with diff calculation.
+    //     The condition must be uncommented after all these bugs are fixed.
+    //     Also don't forget to remove ignore from for SparseFileContentBuilder
+    //      throw new IllegalArgumentException(String.format("The zero-based line number %d is after
+    // the end of file. The file size is %d line(s).", lineNumber, size));
+    //    }
+    if (lineNumber < lastRangeEnd) {
+      throw new IllegalArgumentException(
+          String.format(
+              "Invalid line number %d. You are trying to add a line before an already added line"
+                  + " %d",
+              lineNumber, lastRangeEnd));
+    }
+    if (lineNumber > lastRangeEnd) {
+      finishLastRange();
+      startNextRange(lineNumber);
+    }
+    lastRangeLines.add(content);
+    lastRangeEnd++;
+  }
+
+  private void startNextRange(int base) {
+    lastRangeLines = new ImmutableList.Builder<>();
+    lastRangeBase = lastRangeEnd = base;
+  }
+
+  private void finishLastRange() {
+    if (lastRangeEnd > lastRangeBase) {
+      ranges.add(Range.create(lastRangeBase, lastRangeLines.build()));
+      lastRangeLines = null;
+    }
+  }
+
+  public SparseFileContent build() {
+    finishLastRange();
+    return SparseFileContent.create(ranges.build(), size);
+  }
+}
diff --git a/java/com/google/gerrit/prettify/common/testing/BUILD b/java/com/google/gerrit/prettify/common/testing/BUILD
new file mode 100644
index 0000000..5057fdb
--- /dev/null
+++ b/java/com/google/gerrit/prettify/common/testing/BUILD
@@ -0,0 +1,14 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+package(default_testonly = True)
+
+java_library(
+    name = "testing",
+    srcs = glob(["*.java"]),
+    visibility = ["//visibility:public"],
+    deps = [
+        "//java/com/google/gerrit/prettify:server",
+        "//lib:guava",
+        "//lib/truth",
+    ],
+)
diff --git a/java/com/google/gerrit/prettify/common/testing/SparseFileContentSubject.java b/java/com/google/gerrit/prettify/common/testing/SparseFileContentSubject.java
new file mode 100644
index 0000000..c1fe1ec
--- /dev/null
+++ b/java/com/google/gerrit/prettify/common/testing/SparseFileContentSubject.java
@@ -0,0 +1,65 @@
+// Copyright (C) 2019 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.prettify.common.testing;
+
+import static com.google.common.truth.Truth.assertAbout;
+
+import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.IntegerSubject;
+import com.google.common.truth.MapSubject;
+import com.google.common.truth.Subject;
+import com.google.gerrit.prettify.common.SparseFileContent;
+import java.util.HashMap;
+import java.util.Map;
+
+public class SparseFileContentSubject extends Subject {
+  public static SparseFileContentSubject assertThat(SparseFileContent sparseFileContent) {
+    return assertAbout(sparseFileContent()).that(sparseFileContent);
+  }
+
+  private final SparseFileContent sparseFileContent;
+
+  private SparseFileContentSubject(FailureMetadata metadata, SparseFileContent actual) {
+    super(metadata, actual);
+    this.sparseFileContent = actual;
+  }
+
+  private static Subject.Factory<SparseFileContentSubject, SparseFileContent> sparseFileContent() {
+    return SparseFileContentSubject::new;
+  }
+
+  public IntegerSubject getSize() {
+    isNotNull();
+    return check("size()").that(sparseFileContent.getSize());
+  }
+
+  public IntegerSubject getRangesCount() {
+    isNotNull();
+    return check("rangesCount()").that(sparseFileContent.getRangesCount());
+  }
+
+  public MapSubject lines() {
+    isNotNull();
+    Map<Integer, String> lines = new HashMap<>();
+    SparseFileContent.Accessor accessor = sparseFileContent.createAccessor();
+    int size = accessor.getSize();
+    int idx = accessor.first();
+    while (idx < size) {
+      lines.put(idx, accessor.get(idx));
+      idx = accessor.next(idx);
+    }
+    return check("lines()").that(lines);
+  }
+}
diff --git a/java/com/google/gerrit/server/ExceptionHook.java b/java/com/google/gerrit/server/ExceptionHook.java
index a2bade0..019a715 100644
--- a/java/com/google/gerrit/server/ExceptionHook.java
+++ b/java/com/google/gerrit/server/ExceptionHook.java
@@ -34,6 +34,12 @@
    * <p>Only affects operations that are executed with {@link
    * com.google.gerrit.server.update.RetryHelper}.
    *
+   * <p>Should return {@code true} only for exceptions that are caused by temporary issues where a
+   * retry of the operation has a chance to succeed.
+   *
+   * <p>If {@code false} is returned the operation is still retried once to capture a trace, unless
+   * {@link #skipRetryWithTrace(Throwable)} skips the auto-retry.
+   *
    * @param throwable throwable that was thrown while executing the operation
    * @return whether the operation should be retried
    */
@@ -42,6 +48,34 @@
   }
 
   /**
+   * Whether auto-retrying of an operation with tracing should be skipped for the given throwable.
+   *
+   * <p>Only affects operations that are executed with {@link
+   * com.google.gerrit.server.update.RetryHelper}.
+   *
+   * <p>This method is only called for exceptions for which the operation should not be retried
+   * ({@link #shouldRetry(Throwable)} returned {@code false}).
+   *
+   * <p>By default this method returns {@code false}, so that by default traces for unexpected
+   * exceptions are captured, which allows to investigate them.
+   *
+   * <p>Implementors may use this method to skip retry with tracing for exceptions that occur due to
+   * known causes that are permanent and where a trace is not needed for the investigation. For
+   * example, if an operation fails because persisted data is corrupt, it makes no sense to retry
+   * the operation with a trace, because the trace will not help with fixing the corrupt data.
+   *
+   * <p>This method is only invoked if retry with tracing is enabled on the server ({@code
+   * retry.retryWithTraceOnFailure} in {@code gerrit.config} is set to {@code true}).
+   *
+   * @param throwable throwable that was thrown while executing the operation
+   * @return whether auto-retrying of an operation with tracing should be skipped for the given
+   *     throwable
+   */
+  default boolean skipRetryWithTrace(Throwable throwable) {
+    return false;
+  }
+
+  /**
    * Formats the cause of an exception for use in metrics.
    *
    * <p>This method allows implementors to group exceptions that have the same cause into one metric
diff --git a/java/com/google/gerrit/server/change/ChangeInserter.java b/java/com/google/gerrit/server/change/ChangeInserter.java
index 4263373..d1c27d5 100644
--- a/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -37,6 +37,7 @@
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.PatchSetApproval;
 import com.google.gerrit.entities.PatchSetInfo;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.client.ReviewerState;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -55,7 +56,6 @@
 import com.google.gerrit.server.git.GroupCollector;
 import com.google.gerrit.server.git.validators.CommitValidationException;
 import com.google.gerrit.server.git.validators.CommitValidators;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.mail.send.CreateChangeSender;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -396,7 +396,7 @@
      * instead of setting the status directly?
      */
     if (change.getStatus() == Change.Status.MERGED) {
-      update.fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
+      update.fixStatusToMerged(new SubmissionId(change));
     } else {
       update.setStatus(change.getStatus());
     }
diff --git a/java/com/google/gerrit/server/change/ConsistencyChecker.java b/java/com/google/gerrit/server/change/ConsistencyChecker.java
index 19db5ee..513f5ca 100644
--- a/java/com/google/gerrit/server/change/ConsistencyChecker.java
+++ b/java/com/google/gerrit/server/change/ConsistencyChecker.java
@@ -33,6 +33,7 @@
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.api.changes.FixInput;
 import com.google.gerrit.extensions.common.ProblemInfo;
@@ -44,7 +45,6 @@
 import com.google.gerrit.server.PatchSetUtil;
 import com.google.gerrit.server.account.Accounts;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.PatchSetState;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -578,7 +578,7 @@
     public boolean updateChange(ChangeContext ctx) {
       ctx.getChange().setStatus(Change.Status.MERGED);
       ctx.getUpdate(ctx.getChange().currentPatchSetId())
-          .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
+          .fixStatusToMerged(new SubmissionId(ctx.getChange()));
       p.status = Status.FIXED;
       p.outcome = "Marked change as merged";
       return true;
diff --git a/java/com/google/gerrit/server/diff/DiffInfoCreator.java b/java/com/google/gerrit/server/diff/DiffInfoCreator.java
new file mode 100644
index 0000000..c29ffc8
--- /dev/null
+++ b/java/com/google/gerrit/server/diff/DiffInfoCreator.java
@@ -0,0 +1,299 @@
+// Copyright (C) 2019 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.server.diff;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.data.PatchScript;
+import com.google.gerrit.common.data.PatchScript.DisplayMethod;
+import com.google.gerrit.common.data.PatchScript.PatchScriptFileInfo;
+import com.google.gerrit.entities.Patch;
+import com.google.gerrit.extensions.common.ChangeType;
+import com.google.gerrit.extensions.common.DiffInfo;
+import com.google.gerrit.extensions.common.DiffInfo.ContentEntry;
+import com.google.gerrit.extensions.common.DiffInfo.FileMeta;
+import com.google.gerrit.extensions.common.DiffInfo.IntraLineStatus;
+import com.google.gerrit.extensions.common.DiffWebLinkInfo;
+import com.google.gerrit.extensions.common.WebLinkInfo;
+import com.google.gerrit.jgit.diff.ReplaceEdit;
+import com.google.gerrit.prettify.common.SparseFileContent;
+import com.google.gerrit.server.change.FileContentUtil;
+import com.google.gerrit.server.project.ProjectState;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+import org.eclipse.jgit.diff.Edit;
+
+/** Creates and fills a new {@link DiffInfo} object based on diff between files. */
+public class DiffInfoCreator {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+  private static final ImmutableMap<Patch.ChangeType, ChangeType> CHANGE_TYPE =
+      Maps.immutableEnumMap(
+          new ImmutableMap.Builder<Patch.ChangeType, ChangeType>()
+              .put(Patch.ChangeType.ADDED, ChangeType.ADDED)
+              .put(Patch.ChangeType.MODIFIED, ChangeType.MODIFIED)
+              .put(Patch.ChangeType.DELETED, ChangeType.DELETED)
+              .put(Patch.ChangeType.RENAMED, ChangeType.RENAMED)
+              .put(Patch.ChangeType.COPIED, ChangeType.COPIED)
+              .put(Patch.ChangeType.REWRITE, ChangeType.REWRITE)
+              .build());
+
+  private final DiffWebLinksProvider webLinksProvider;
+  private final boolean intraline;
+  private final ProjectState state;
+
+  public DiffInfoCreator(
+      ProjectState state, DiffWebLinksProvider webLinksProvider, boolean intraline) {
+    this.webLinksProvider = webLinksProvider;
+    this.state = state;
+    this.intraline = intraline;
+  }
+
+  /* Returns the {@link DiffInfo} to display for end-users */
+  public DiffInfo create(PatchScript ps, DiffSide sideA, DiffSide sideB) {
+    DiffInfo result = new DiffInfo();
+
+    ImmutableList<DiffWebLinkInfo> links = webLinksProvider.getDiffLinks();
+    result.webLinks = links.isEmpty() ? null : links;
+
+    if (ps.isBinary()) {
+      result.binary = true;
+    }
+    result.metaA = createFileMeta(sideA).orElse(null);
+    result.metaB = createFileMeta(sideB).orElse(null);
+
+    if (intraline) {
+      if (ps.hasIntralineTimeout()) {
+        result.intralineStatus = IntraLineStatus.TIMEOUT;
+      } else if (ps.hasIntralineFailure()) {
+        result.intralineStatus = IntraLineStatus.FAILURE;
+      } else {
+        result.intralineStatus = IntraLineStatus.OK;
+      }
+      logger.atFine().log("intralineStatus = %s", result.intralineStatus);
+    }
+
+    result.changeType = CHANGE_TYPE.get(ps.getChangeType());
+    logger.atFine().log("changeType = %s", result.changeType);
+    if (result.changeType == null) {
+      throw new IllegalStateException("unknown change type: " + ps.getChangeType());
+    }
+
+    if (ps.getPatchHeader().size() > 0) {
+      result.diffHeader = ps.getPatchHeader();
+    }
+    result.content = calculateDiffContentEntries(ps);
+    return result;
+  }
+
+  private static List<ContentEntry> calculateDiffContentEntries(PatchScript ps) {
+    ContentCollector contentCollector = new ContentCollector(ps);
+    Set<Edit> editsDueToRebase = ps.getEditsDueToRebase();
+    for (Edit edit : ps.getEdits()) {
+      logger.atFine().log("next edit = %s", edit);
+
+      if (edit.getType() == Edit.Type.EMPTY) {
+        logger.atFine().log("skip empty edit");
+        continue;
+      }
+      contentCollector.addCommon(edit.getBeginA());
+
+      checkState(
+          contentCollector.nextA == edit.getBeginA(),
+          "nextA = %s; want %s",
+          contentCollector.nextA,
+          edit.getBeginA());
+      checkState(
+          contentCollector.nextB == edit.getBeginB(),
+          "nextB = %s; want %s",
+          contentCollector.nextB,
+          edit.getBeginB());
+      switch (edit.getType()) {
+        case DELETE:
+        case INSERT:
+        case REPLACE:
+          List<Edit> internalEdit =
+              edit instanceof ReplaceEdit ? ((ReplaceEdit) edit).getInternalEdits() : null;
+          boolean dueToRebase = editsDueToRebase.contains(edit);
+          contentCollector.addDiff(edit.getEndA(), edit.getEndB(), internalEdit, dueToRebase);
+          break;
+        case EMPTY:
+        default:
+          throw new IllegalStateException();
+      }
+    }
+    contentCollector.addCommon(ps.getA().getSize());
+
+    return contentCollector.lines;
+  }
+
+  private Optional<FileMeta> createFileMeta(DiffSide side) {
+    PatchScriptFileInfo fileInfo = side.fileInfo();
+    if (fileInfo.displayMethod == DisplayMethod.NONE) {
+      return Optional.empty();
+    }
+    FileMeta result = new FileMeta();
+    result.name = side.fileName();
+    result.contentType =
+        FileContentUtil.resolveContentType(
+            state, side.fileName(), fileInfo.mode, fileInfo.mimeType);
+    result.lines = fileInfo.content.getSize();
+    ImmutableList<WebLinkInfo> links = webLinksProvider.getFileWebLinks(side.type());
+    result.webLinks = links.isEmpty() ? null : links;
+    result.commitId = fileInfo.commitId;
+    return Optional.of(result);
+  }
+
+  private static class ContentCollector {
+
+    private final List<ContentEntry> lines;
+    private final SparseFileContent.Accessor fileA;
+    private final SparseFileContent.Accessor fileB;
+    private final boolean ignoreWS;
+
+    private int nextA;
+    private int nextB;
+
+    ContentCollector(PatchScript ps) {
+      lines = Lists.newArrayListWithExpectedSize(ps.getEdits().size() + 2);
+      fileA = ps.getA().createAccessor();
+      fileB = ps.getB().createAccessor();
+      ignoreWS = ps.isIgnoreWhitespace();
+    }
+
+    void addCommon(int end) {
+      logger.atFine().log("addCommon: end = %d", end);
+
+      end = Math.min(end, fileA.getSize());
+      logger.atFine().log("end = %d", end);
+
+      if (nextA >= end) {
+        logger.atFine().log("nextA >= end: nextA = %d, end = %d", nextA, end);
+        return;
+      }
+
+      while (nextA < end) {
+        logger.atFine().log("nextA < end: nextA = %d, end = %d", nextA, end);
+
+        if (!fileA.contains(nextA)) {
+          logger.atFine().log("fileA does not contain nextA: nextA = %d", nextA);
+
+          int endRegion = Math.min(end, nextA == 0 ? fileA.first() : fileA.next(nextA - 1));
+          int len = endRegion - nextA;
+          entry().skip = len;
+          nextA = endRegion;
+          nextB += len;
+
+          logger.atFine().log("setting: nextA = %d, nextB = %d", nextA, nextB);
+          continue;
+        }
+
+        ContentEntry e = null;
+        for (int i = nextA; i == nextA && i < end; i = fileA.next(i), nextA++, nextB++) {
+          if (ignoreWS && fileB.contains(nextB)) {
+            if (e == null || e.common == null) {
+              logger.atFine().log("create new common entry: nextA = %d, nextB = %d", nextA, nextB);
+              e = entry();
+              e.a = Lists.newArrayListWithCapacity(end - nextA);
+              e.b = Lists.newArrayListWithCapacity(end - nextA);
+              e.common = true;
+            }
+            e.a.add(fileA.get(nextA));
+            e.b.add(fileB.get(nextB));
+          } else {
+            if (e == null || e.common != null) {
+              logger.atFine().log(
+                  "create new non-common entry: nextA = %d, nextB = %d", nextA, nextB);
+              e = entry();
+              e.ab = Lists.newArrayListWithCapacity(end - nextA);
+            }
+            e.ab.add(fileA.get(nextA));
+          }
+        }
+      }
+    }
+
+    void addDiff(int endA, int endB, List<Edit> internalEdit, boolean dueToRebase) {
+      logger.atFine().log(
+          "addDiff: endA = %d, endB = %d, numberOfInternalEdits = %d, dueToRebase = %s",
+          endA, endB, internalEdit != null ? internalEdit.size() : 0, dueToRebase);
+
+      int lenA = endA - nextA;
+      int lenB = endB - nextB;
+      logger.atFine().log("lenA = %d, lenB = %d", lenA, lenB);
+      checkState(lenA > 0 || lenB > 0);
+
+      logger.atFine().log("create non-common entry");
+      ContentEntry e = entry();
+      if (lenA > 0) {
+        logger.atFine().log("lenA > 0: lenA = %d", lenA);
+        e.a = Lists.newArrayListWithCapacity(lenA);
+        for (; nextA < endA; nextA++) {
+          e.a.add(fileA.get(nextA));
+        }
+      }
+      if (lenB > 0) {
+        logger.atFine().log("lenB > 0: lenB = %d", lenB);
+        e.b = Lists.newArrayListWithCapacity(lenB);
+        for (; nextB < endB; nextB++) {
+          e.b.add(fileB.get(nextB));
+        }
+      }
+      if (internalEdit != null && !internalEdit.isEmpty()) {
+        logger.atFine().log("processing internal edits");
+
+        e.editA = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
+        e.editB = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
+        int lastA = 0;
+        int lastB = 0;
+        for (Edit edit : internalEdit) {
+          logger.atFine().log("internal edit = %s", edit);
+
+          if (edit.getBeginA() != edit.getEndA()) {
+            logger.atFine().log(
+                "edit.getBeginA() != edit.getEndA(): edit.getBeginA() = %d, edit.getEndA() = %d",
+                edit.getBeginA(), edit.getEndA());
+            e.editA.add(
+                ImmutableList.of(edit.getBeginA() - lastA, edit.getEndA() - edit.getBeginA()));
+            lastA = edit.getEndA();
+            logger.atFine().log("lastA = %d", lastA);
+          }
+          if (edit.getBeginB() != edit.getEndB()) {
+            logger.atFine().log(
+                "edit.getBeginB() != edit.getEndB(): edit.getBeginB() = %d, edit.getEndB() = %d",
+                edit.getBeginB(), edit.getEndB());
+            e.editB.add(
+                ImmutableList.of(edit.getBeginB() - lastB, edit.getEndB() - edit.getBeginB()));
+            lastB = edit.getEndB();
+            logger.atFine().log("lastB = %d", lastB);
+          }
+        }
+      }
+      e.dueToRebase = dueToRebase ? true : null;
+    }
+
+    private ContentEntry entry() {
+      ContentEntry e = new ContentEntry();
+      lines.add(e);
+      return e;
+    }
+  }
+}
diff --git a/java/com/google/gerrit/server/diff/DiffSide.java b/java/com/google/gerrit/server/diff/DiffSide.java
new file mode 100644
index 0000000..28c7810
--- /dev/null
+++ b/java/com/google/gerrit/server/diff/DiffSide.java
@@ -0,0 +1,37 @@
+// Copyright (C) 2019 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.server.diff;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.common.data.PatchScript.PatchScriptFileInfo;
+
+/** Contains settings for one of two sides in diff view. Each diff view has exactly 2 sides. */
+@AutoValue
+public abstract class DiffSide {
+  public enum Type {
+    SIDE_A,
+    SIDE_B
+  }
+
+  public static DiffSide create(PatchScriptFileInfo fileInfo, String fileName, Type type) {
+    return new AutoValue_DiffSide(fileInfo, fileName, type);
+  }
+
+  public abstract PatchScriptFileInfo fileInfo();
+
+  public abstract String fileName();
+
+  public abstract Type type();
+}
diff --git a/java/com/google/gerrit/server/diff/DiffWebLinksProvider.java b/java/com/google/gerrit/server/diff/DiffWebLinksProvider.java
new file mode 100644
index 0000000..0f71b17
--- /dev/null
+++ b/java/com/google/gerrit/server/diff/DiffWebLinksProvider.java
@@ -0,0 +1,29 @@
+// Copyright (C) 2019 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.server.diff;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.extensions.common.DiffWebLinkInfo;
+import com.google.gerrit.extensions.common.WebLinkInfo;
+
+/** Provider for different types of links which can be displayed in a diff view. */
+public interface DiffWebLinksProvider {
+
+  /** Returns links associated with the diff view */
+  ImmutableList<DiffWebLinkInfo> getDiffLinks();
+
+  /** Returns links associated with the diff side */
+  ImmutableList<WebLinkInfo> getFileWebLinks(DiffSide.Type fileInfoType);
+}
diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java
index 9aebebf..01d5380 100644
--- a/java/com/google/gerrit/server/git/MergedByPushOp.java
+++ b/java/com/google/gerrit/server/git/MergedByPushOp.java
@@ -22,11 +22,11 @@
 import com.google.gerrit.entities.LabelId;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.PatchSetInfo;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.PatchSetUtil;
 import com.google.gerrit.server.config.SendEmailExecutor;
 import com.google.gerrit.server.extensions.events.ChangeMerged;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.mail.send.MergedSender;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -52,7 +52,7 @@
     MergedByPushOp create(
         RequestScopePropagator requestScopePropagator,
         PatchSet.Id psId,
-        @Assisted RequestId submissionId,
+        @Assisted SubmissionId submissionId,
         @Assisted("refName") String refName,
         @Assisted("mergeResultRevId") String mergeResultRevId);
   }
@@ -66,7 +66,7 @@
   private final ChangeMerged changeMerged;
 
   private final PatchSet.Id psId;
-  private final RequestId submissionId;
+  private final SubmissionId submissionId;
   private final String refName;
   private final String mergeResultRevId;
 
@@ -86,7 +86,7 @@
       ChangeMerged changeMerged,
       @Assisted RequestScopePropagator requestScopePropagator,
       @Assisted PatchSet.Id psId,
-      @Assisted RequestId submissionId,
+      @Assisted SubmissionId submissionId,
       @Assisted("refName") String refName,
       @Assisted("mergeResultRevId") String mergeResultRevId) {
     this.patchSetInfoFactory = patchSetInfoFactory;
@@ -137,7 +137,7 @@
     }
     change.setCurrentPatchSet(info);
     change.setStatus(Change.Status.MERGED);
-    change.setSubmissionId(submissionId.toStringForStorage());
+    change.setSubmissionId(submissionId.toString());
     // we cannot reconstruct the submit records for when this change was
     // submitted, this is why we must fix the status and other details.
     update.fixStatusToMerged(submissionId);
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index cec9e4e..5155925 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -74,6 +74,7 @@
 import com.google.gerrit.entities.PatchSetInfo;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.api.changes.HashtagsInput;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -3023,7 +3024,8 @@
                     info,
                     groups,
                     magicBranch,
-                    receivePack.getPushCertificate())
+                    receivePack.getPushCertificate(),
+                    notes.getChange())
                 .setRequestScopePropagator(requestScopePropagator);
         bu.addOp(notes.getChangeId(), replaceOp);
         if (progress != null) {
@@ -3281,7 +3283,7 @@
 
                 int existingPatchSets = 0;
                 int newPatchSets = 0;
-                RequestId submissionId = null;
+                SubmissionId submissionId = null;
                 COMMIT:
                 for (RevCommit c; (c = rw.next()) != null; ) {
                   rw.parseBody(c);
@@ -3290,10 +3292,10 @@
                       receivePackRefCache.tipsFromObjectId(c.copy(), RefNames.REFS_CHANGES)) {
                     PatchSet.Id psId = PatchSet.Id.fromRef(ref.getName());
                     Optional<ChangeNotes> notes = getChangeNotes(psId.changeId());
-                    if (submissionId == null) {
-                      submissionId = new RequestId(psId.changeId().toString());
-                    }
                     if (notes.isPresent() && notes.get().getChange().getDest().equals(branch)) {
+                      if (submissionId == null) {
+                        submissionId = new SubmissionId(notes.get().getChange());
+                      }
                       existingPatchSets++;
                       bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null));
                       bu.addOp(
@@ -3333,7 +3335,7 @@
                     continue;
                   }
                   if (submissionId == null) {
-                    submissionId = new RequestId(id.toString());
+                    submissionId = new SubmissionId(req.notes.getChange());
                   }
                   req.addOps(bu, null);
                   bu.addOp(id, setPrivateOpFactory.create(false, null));
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 6c0d5d3..24154d60 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -36,6 +36,7 @@
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.PatchSetApproval;
 import com.google.gerrit.entities.PatchSetInfo;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.client.ChangeKind;
@@ -61,7 +62,6 @@
 import com.google.gerrit.server.extensions.events.RevisionCreated;
 import com.google.gerrit.server.git.MergedByPushOp;
 import com.google.gerrit.server.git.receive.ReceiveCommits.MagicBranchInput;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.mail.MailUtil.MailRecipients;
 import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
 import com.google.gerrit.server.notedb.ChangeNotes;
@@ -110,7 +110,8 @@
         PatchSetInfo info,
         List<String> groups,
         @Nullable MagicBranchInput magicBranch,
-        @Nullable PushCertificate pushCertificate);
+        @Nullable PushCertificate pushCertificate,
+        Change change);
   }
 
   private static final String CHANGE_IS_CLOSED = "change is closed";
@@ -143,6 +144,7 @@
   private final PatchSetInfo info;
   private final MagicBranchInput magicBranch;
   private final PushCertificate pushCertificate;
+  private final Change change;
   private List<String> groups;
 
   private final Map<String, Short> approvals = new HashMap<>();
@@ -177,6 +179,7 @@
       ProjectCache projectCache,
       @SendEmailExecutor ExecutorService sendEmailExecutor,
       ReviewerAdder reviewerAdder,
+      Change change,
       @Assisted ProjectState projectState,
       @Assisted BranchNameKey dest,
       @Assisted boolean checkMergedInto,
@@ -218,6 +221,7 @@
     this.groups = groups;
     this.magicBranch = magicBranch;
     this.pushCertificate = pushCertificate;
+    this.change = change;
   }
 
   @Override
@@ -239,7 +243,7 @@
             mergedByPushOpFactory.create(
                 requestScopePropagator,
                 patchSetId,
-                new RequestId(patchSetId.changeId().toString()),
+                new SubmissionId(change),
                 mergedInto,
                 mergeResultRevId);
       }
diff --git a/java/com/google/gerrit/server/notedb/ChangeUpdate.java b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
index 02a4dcc..6a900c0 100644
--- a/java/com/google/gerrit/server/notedb/ChangeUpdate.java
+++ b/java/com/google/gerrit/server/notedb/ChangeUpdate.java
@@ -55,11 +55,11 @@
 import com.google.gerrit.entities.Comment;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RobotComment;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.mail.Address;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.util.LabelVote;
 import com.google.inject.assistedinject.Assisted;
@@ -220,10 +220,10 @@
     this.status = status;
   }
 
-  public void fixStatusToMerged(RequestId submissionId) {
+  public void fixStatusToMerged(SubmissionId submissionId) {
     checkArgument(submissionId != null, "submission id must be set for merged changes");
     this.status = Change.Status.MERGED;
-    this.submissionId = submissionId.toStringForStorage();
+    this.submissionId = submissionId.toString();
   }
 
   public void putApproval(String label, short value) {
@@ -242,9 +242,9 @@
     approvals.put(label, reviewer, Optional.empty());
   }
 
-  public void merge(RequestId submissionId, Iterable<SubmitRecord> submitRecords) {
+  public void merge(SubmissionId submissionId, Iterable<SubmitRecord> submitRecords) {
     this.status = Change.Status.MERGED;
-    this.submissionId = submissionId.toStringForStorage();
+    this.submissionId = submissionId.toString();
     this.submitRecords = ImmutableList.copyOf(submitRecords);
     checkArgument(!this.submitRecords.isEmpty(), "no submit records specified at submit time");
   }
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index 1435c5e..0a17303 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -17,6 +17,7 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Comparator.comparing;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.common.data.CommentDetail;
 import com.google.gerrit.common.data.PatchScript;
@@ -24,11 +25,11 @@
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Comment;
 import com.google.gerrit.entities.Patch;
-import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.Patch.ChangeType;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
 import com.google.gerrit.prettify.common.EditList;
-import com.google.gerrit.prettify.common.SparseFileContent;
+import com.google.gerrit.prettify.common.SparseFileContentBuilder;
 import com.google.gerrit.server.mime.FileTypeRegistry;
 import com.google.inject.Inject;
 import eu.medsea.mimeutil.MimeType;
@@ -39,10 +40,8 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.errors.CorruptObjectException;
-import org.eclipse.jgit.errors.IncorrectObjectTypeException;
-import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.FileMode;
 import org.eclipse.jgit.lib.ObjectId;
@@ -59,28 +58,17 @@
 
   private static final Comparator<Edit> EDIT_SORT = comparing(Edit::getBeginA);
 
-  private Repository db;
-  private Project.NameKey projectKey;
-  private ObjectReader reader;
   private Change change;
   private DiffPreferencesInfo diffPrefs;
-  private ComparisonType comparisonType;
-  private ObjectId aId;
-  private ObjectId bId;
   private List<Edit> edits;
   private final FileTypeRegistry registry;
-  private final PatchListCache patchListCache;
   private int context;
+  private IntraLineDiffCalculator intralineDiffCalculator;
+  private SidesResolver sidesResolver;
 
   @Inject
-  PatchScriptBuilder(FileTypeRegistry ftr, PatchListCache plc) {
+  PatchScriptBuilder(FileTypeRegistry ftr) {
     registry = ftr;
-    patchListCache = plc;
-  }
-
-  void setRepository(Repository r, Project.NameKey projectKey) {
-    this.db = r;
-    this.projectKey = projectKey;
   }
 
   void setChange(Change c) {
@@ -98,62 +86,39 @@
     }
   }
 
-  void setTrees(ComparisonType ct, ObjectId a, ObjectId b) {
-    comparisonType = ct;
-    aId = a;
-    bId = b;
+  void setIntraLineDiffCalculator(IntraLineDiffCalculator calculator) {
+    intralineDiffCalculator = calculator;
   }
 
-  PatchScript toPatchScript(PatchListEntry content, CommentDetail comments, List<Patch> history)
-      throws IOException {
-    reader = db.newObjectReader();
-    try {
-      return build(content, comments, history);
-    } finally {
-      reader.close();
-    }
+  void setSidesResolver(SidesResolver resolver) {
+    sidesResolver = resolver;
   }
 
-  private PatchScript build(PatchListEntry content, CommentDetail comments, List<Patch> history)
+  PatchScript toPatchScript(PatchFileChange content, CommentDetail comments, List<Patch> history)
       throws IOException {
-    boolean intralineFailure = false;
-    boolean intralineTimeout = false;
+    return build(content, comments, history);
+  }
 
-    SideResolver resolver = new SideResolver();
-    Side a = resolver.resolve(oldName(content), null, aId);
-    Side b = resolver.resolve(newName(content), a, bId);
+  private PatchScript build(PatchFileChange content, CommentDetail comments, List<Patch> history)
+      throws IOException {
 
-    edits = new ArrayList<>(content.getEdits());
+    ResolvedSides sides = sidesResolver.resolveSides(registry, oldName(content), newName(content));
+    PatchSide a = sides.a;
+    PatchSide b = sides.b;
+
+    ImmutableList<Edit> contentEdits = content.getEdits();
     ImmutableSet<Edit> editsDueToRebase = content.getEditsDueToRebase();
 
-    if (isModify(content) && diffPrefs.intralineDifference && isIntralineModeAllowed(b)) {
-      IntraLineDiff d =
-          patchListCache.getIntraLineDiff(
-              IntraLineDiffKey.create(a.id, b.id, diffPrefs.ignoreWhitespace),
-              IntraLineDiffArgs.create(
-                  a.src, b.src, edits, editsDueToRebase, projectKey, bId, b.path));
-      if (d != null) {
-        switch (d.getStatus()) {
-          case EDIT_LIST:
-            edits = new ArrayList<>(d.getEdits());
-            break;
+    IntraLineDiffCalculatorResult intralineResult = IntraLineDiffCalculatorResult.NO_RESULT;
 
-          case DISABLED:
-            break;
-
-          case ERROR:
-            intralineFailure = true;
-            break;
-
-          case TIMEOUT:
-            intralineTimeout = true;
-            break;
-        }
-      } else {
-        intralineFailure = true;
-      }
+    if (isModify(content) && intralineDiffCalculator != null && isIntralineModeAllowed(b)) {
+      intralineResult =
+          intralineDiffCalculator.calculateIntraLineDiff(
+              contentEdits, editsDueToRebase, a.id, b.id, a.src, b.src, b.treeId, b.path);
     }
 
+    edits = new ArrayList<>(intralineResult.edits.orElse(contentEdits));
+
     correctForDifferencesInNewlineAtEnd(a, b);
 
     if (comments != null) {
@@ -198,8 +163,8 @@
         b.fileMode,
         content.getHeaderLines(),
         diffPrefs,
-        a.dst,
-        b.dst,
+        a.dst.build(),
+        b.dst.build(),
         edits,
         editsDueToRebase,
         a.displayMethod,
@@ -209,14 +174,14 @@
         comments,
         history,
         hugeFile,
-        intralineFailure,
-        intralineTimeout,
+        intralineResult.failure,
+        intralineResult.timeout,
         content.getPatchType() == Patch.PatchType.BINARY,
-        aId == null ? null : aId.getName(),
-        bId == null ? null : bId.getName());
+        a.treeId == null ? null : a.treeId.getName(),
+        b.treeId == null ? null : b.treeId.getName());
   }
 
-  private static boolean isModify(PatchListEntry content) {
+  private static boolean isModify(PatchFileChange content) {
     switch (content.getChangeType()) {
       case MODIFIED:
       case COPIED:
@@ -231,7 +196,7 @@
     }
   }
 
-  private static String oldName(PatchListEntry entry) {
+  private static String oldName(PatchFileChange entry) {
     switch (entry.getChangeType()) {
       case ADDED:
         return null;
@@ -246,7 +211,7 @@
     }
   }
 
-  private static String newName(PatchListEntry entry) {
+  private static String newName(PatchFileChange entry) {
     switch (entry.getChangeType()) {
       case DELETED:
         return null;
@@ -260,7 +225,7 @@
     }
   }
 
-  private static boolean isIntralineModeAllowed(Side side) {
+  private static boolean isIntralineModeAllowed(PatchSide side) {
     // The intraline diff cache keys are the same for these cases. It's better to not show
     // intraline results than showing completely wrong diffs or to run into a server error.
     return !Patch.isMagic(side.path) && !isSubmoduleCommit(side.mode);
@@ -270,7 +235,7 @@
     return mode.getObjectType() == Constants.OBJ_COMMIT;
   }
 
-  private void correctForDifferencesInNewlineAtEnd(Side a, Side b) {
+  private void correctForDifferencesInNewlineAtEnd(PatchSide a, PatchSide b) {
     // a.src.size() is the size ignoring a newline at the end whereas a.size() considers it.
     int aSize = a.src.size();
     int bSize = b.src.size();
@@ -312,11 +277,11 @@
     return list.isEmpty() ? Optional.empty() : Optional.ofNullable(list.get(list.size() - 1));
   }
 
-  private boolean isNewlineAtEndDeleted(Side a, Side b) {
+  private boolean isNewlineAtEndDeleted(PatchSide a, PatchSide b) {
     return !a.src.isMissingNewlineAtEnd() && b.src.isMissingNewlineAtEnd();
   }
 
-  private boolean isNewlineAtEndAdded(Side a, Side b) {
+  private boolean isNewlineAtEndAdded(PatchSide a, PatchSide b) {
     return a.src.isMissingNewlineAtEnd() && !b.src.isMissingNewlineAtEnd();
   }
 
@@ -434,7 +399,7 @@
     return last.getEndA() + (b - last.getEndB());
   }
 
-  private void packContent(Side a, Side b, boolean ignoredWhitespace) {
+  private void packContent(PatchSide a, PatchSide b, boolean ignoredWhitespace) {
     EditList list = new EditList(edits, context, a.size(), b.size());
     for (EditList.Hunk hunk : list.getHunks()) {
       while (hunk.next()) {
@@ -468,8 +433,8 @@
     }
   }
 
-  private static class Side {
-
+  private static class PatchSide {
+    final ObjectId treeId;
     final String path;
     final ObjectId id;
     final FileMode mode;
@@ -478,9 +443,10 @@
     final MimeType mimeType;
     final DisplayMethod displayMethod;
     final PatchScript.FileMode fileMode;
-    final SparseFileContent dst;
+    final SparseFileContentBuilder dst;
 
-    public Side(
+    private PatchSide(
+        ObjectId treeId,
         String path,
         ObjectId id,
         FileMode mode,
@@ -489,6 +455,7 @@
         MimeType mimeType,
         DisplayMethod displayMethod,
         PatchScript.FileMode fileMode) {
+      this.treeId = treeId;
       this.path = path;
       this.id = id;
       this.mode = mode;
@@ -497,8 +464,7 @@
       this.mimeType = mimeType;
       this.displayMethod = displayMethod;
       this.fileMode = fileMode;
-      dst = new SparseFileContent();
-      dst.setSize(size());
+      dst = new SparseFileContentBuilder(size());
     }
 
     int size() {
@@ -521,15 +487,64 @@
     }
   }
 
-  private class SideResolver {
+  interface SidesResolver {
 
-    Side resolve(final String path, final Side other, final ObjectId within) throws IOException {
+    ResolvedSides resolveSides(FileTypeRegistry typeRegistry, String oldName, String newName)
+        throws IOException;
+  }
+
+  private static class ResolvedSides {
+    // Not an @AutoValue because PatchSide can't be AutoValue
+    public final PatchSide a;
+    public final PatchSide b;
+
+    ResolvedSides(PatchSide a, PatchSide b) {
+      this.a = a;
+      this.b = b;
+    }
+  }
+
+  static class SidesResolverImpl implements SidesResolver {
+
+    private final Repository db;
+    private ComparisonType comparisonType;
+    private ObjectId aId;
+    private ObjectId bId;
+
+    SidesResolverImpl(Repository db) {
+      this.db = db;
+    }
+
+    void setTrees(ComparisonType comparisonType, ObjectId a, ObjectId b) {
+      this.comparisonType = comparisonType;
+      this.aId = a;
+      this.bId = b;
+    }
+
+    @Override
+    public ResolvedSides resolveSides(FileTypeRegistry typeRegistry, String oldName, String newName)
+        throws IOException {
+      try (ObjectReader reader = db.newObjectReader()) {
+        PatchSide a = resolve(typeRegistry, reader, oldName, null, aId);
+        PatchSide b = resolve(typeRegistry, reader, newName, a, bId);
+        return new ResolvedSides(a, b);
+      }
+    }
+
+    PatchSide resolve(
+        final FileTypeRegistry registry,
+        final ObjectReader reader,
+        final String path,
+        final PatchSide other,
+        final ObjectId within)
+        throws IOException {
       try {
         boolean isCommitMsg = Patch.COMMIT_MSG.equals(path);
         boolean isMergeList = Patch.MERGE_LIST.equals(path);
         if (isCommitMsg || isMergeList) {
           if (comparisonType.isAgainstParentOrAutoMerge() && Objects.equals(aId, within)) {
             return createSide(
+                within,
                 path,
                 ObjectId.zeroId(),
                 FileMode.MISSING,
@@ -554,6 +569,7 @@
             displayMethod = DisplayMethod.DIFF;
           }
           return createSide(
+              within,
               path,
               within,
               mode,
@@ -563,7 +579,7 @@
               displayMethod,
               false);
         }
-        final TreeWalk tw = find(path, within);
+        final TreeWalk tw = find(reader, path, within);
         ObjectId id = tw != null ? tw.getObjectId(0) : ObjectId.zeroId();
         FileMode mode = tw != null ? tw.getFileMode(0) : FileMode.MISSING;
         boolean reuse =
@@ -598,13 +614,15 @@
             displayMethod = DisplayMethod.IMG;
           }
         }
-        return createSide(path, id, mode, srcContent, src, mimeType, displayMethod, reuse);
+        return createSide(within, path, id, mode, srcContent, src, mimeType, displayMethod, reuse);
+
       } catch (IOException err) {
         throw new IOException("Cannot read " + within.name() + ":" + path, err);
       }
     }
 
-    private Side createSide(
+    private PatchSide createSide(
+        ObjectId treeId,
         String path,
         ObjectId id,
         FileMode mode,
@@ -629,12 +647,11 @@
       } else if (mode == FileMode.GITLINK) {
         fileMode = PatchScript.FileMode.GITLINK;
       }
-      return new Side(path, id, mode, srcContent, src, mimeType, displayMethod, fileMode);
+      return new PatchSide(
+          treeId, path, id, mode, srcContent, src, mimeType, displayMethod, fileMode);
     }
 
-    private TreeWalk find(String path, ObjectId within)
-        throws MissingObjectException, IncorrectObjectTypeException, CorruptObjectException,
-            IOException {
+    private TreeWalk find(ObjectReader reader, String path, ObjectId within) throws IOException {
       if (path == null || within == null) {
         return null;
       }
@@ -649,4 +666,59 @@
     return (a.getBits() & FileMode.TYPE_FILE) == FileMode.TYPE_FILE
         && (b.getBits() & FileMode.TYPE_FILE) == FileMode.TYPE_FILE;
   }
+
+  static class IntraLineDiffCalculatorResult {
+    // Not an @AutoValue because Edit is mutable
+    final boolean failure;
+    final boolean timeout;
+    private final Optional<ImmutableList<Edit>> edits;
+
+    private IntraLineDiffCalculatorResult(
+        Optional<ImmutableList<Edit>> edits, boolean failure, boolean timeout) {
+      this.failure = failure;
+      this.timeout = timeout;
+      this.edits = edits;
+    }
+
+    static final IntraLineDiffCalculatorResult NO_RESULT =
+        new IntraLineDiffCalculatorResult(Optional.empty(), false, false);
+    static final IntraLineDiffCalculatorResult FAILURE =
+        new IntraLineDiffCalculatorResult(Optional.empty(), true, false);
+    static final IntraLineDiffCalculatorResult TIMEOUT =
+        new IntraLineDiffCalculatorResult(Optional.empty(), false, true);
+
+    static IntraLineDiffCalculatorResult success(ImmutableList<Edit> edits) {
+      return new IntraLineDiffCalculatorResult(Optional.of(edits), false, false);
+    }
+  }
+
+  interface IntraLineDiffCalculator {
+
+    IntraLineDiffCalculatorResult calculateIntraLineDiff(
+        ImmutableList<Edit> edits,
+        Set<Edit> editsDueToRebase,
+        ObjectId aId,
+        ObjectId bId,
+        Text aSrc,
+        Text bSrc,
+        ObjectId bTreeId,
+        String bPath);
+  }
+
+  interface PatchFileChange {
+
+    ImmutableList<Edit> getEdits();
+
+    ImmutableSet<Edit> getEditsDueToRebase();
+
+    List<String> getHeaderLines();
+
+    String getNewName();
+
+    String getOldName();
+
+    ChangeType getChangeType();
+
+    Patch.PatchType getPatchType();
+  }
 }
diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index ffeda3d..2c8de1d 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -17,6 +17,8 @@
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.CommentDetail;
@@ -27,6 +29,7 @@
 import com.google.gerrit.entities.Patch;
 import com.google.gerrit.entities.Patch.ChangeType;
 import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -38,6 +41,10 @@
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.LargeObjectException;
 import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.patch.PatchScriptBuilder.IntraLineDiffCalculator;
+import com.google.gerrit.server.patch.PatchScriptBuilder.IntraLineDiffCalculatorResult;
+import com.google.gerrit.server.patch.PatchScriptBuilder.PatchFileChange;
+import com.google.gerrit.server.patch.PatchScriptBuilder.SidesResolverImpl;
 import com.google.gerrit.server.permissions.ChangePermission;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -53,15 +60,19 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
 import java.util.concurrent.Callable;
+import org.eclipse.jgit.diff.Edit;
 import org.eclipse.jgit.errors.RepositoryNotFoundException;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
 
 public class PatchScriptFactory implements Callable<PatchScript> {
+
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   public interface Factory {
+
     PatchScriptFactory create(
         ChangeNotes notes,
         String fileName,
@@ -225,7 +236,7 @@
 
         loadCommentsAndHistory(content.getChangeType(), content.getOldName(), content.getNewName());
 
-        return b.toPatchScript(content, comments, history);
+        return b.toPatchScript(new PatchListFileChange(content), comments, history);
       } catch (PatchListNotAvailableException e) {
         throw new NoSuchChangeException(changeId, e);
       } catch (IOException e) {
@@ -256,10 +267,15 @@
 
   private PatchScriptBuilder newBuilder(PatchList list, Repository git) {
     final PatchScriptBuilder b = builderFactory.get();
-    b.setRepository(git, notes.getProjectName());
     b.setChange(notes.getChange());
     b.setDiffPrefs(diffPrefs);
-    b.setTrees(list.getComparisonType(), list.getOldId(), list.getNewId());
+    if (diffPrefs.intralineDifference) {
+      b.setIntraLineDiffCalculator(
+          new IntraLineDiffCalculatorImpl(patchListCache, notes.getProjectName(), diffPrefs));
+    }
+    SidesResolverImpl sidesResolver = new SidesResolverImpl(git);
+    sidesResolver.setTrees(list.getComparisonType(), list.getOldId(), list.getNewId());
+    b.setSidesResolver(sidesResolver);
     return b;
   }
 
@@ -402,4 +418,98 @@
       }
     }
   }
+
+  private static class IntraLineDiffCalculatorImpl implements IntraLineDiffCalculator {
+
+    private final PatchListCache patchListCache;
+    private final Project.NameKey projectKey;
+    private final DiffPreferencesInfo diffPrefs;
+
+    public IntraLineDiffCalculatorImpl(
+        PatchListCache patchListCache, Project.NameKey projectKey, DiffPreferencesInfo diffPrefs) {
+      this.patchListCache = patchListCache;
+      this.projectKey = projectKey;
+      this.diffPrefs = diffPrefs;
+    }
+
+    @Override
+    public IntraLineDiffCalculatorResult calculateIntraLineDiff(
+        ImmutableList<Edit> edits,
+        Set<Edit> editsDueToRebase,
+        ObjectId aId,
+        ObjectId bId,
+        Text aSrc,
+        Text bSrc,
+        ObjectId bTreeId,
+        String bPath) {
+      IntraLineDiff d =
+          patchListCache.getIntraLineDiff(
+              IntraLineDiffKey.create(aId, bId, diffPrefs.ignoreWhitespace),
+              IntraLineDiffArgs.create(
+                  aSrc, bSrc, edits, editsDueToRebase, projectKey, bTreeId, bPath));
+      if (d == null) {
+        return IntraLineDiffCalculatorResult.FAILURE;
+      }
+      switch (d.getStatus()) {
+        case EDIT_LIST:
+          return IntraLineDiffCalculatorResult.success(d.getEdits());
+
+        case DISABLED:
+          return IntraLineDiffCalculatorResult.NO_RESULT;
+
+        case ERROR:
+          return IntraLineDiffCalculatorResult.FAILURE;
+
+        case TIMEOUT:
+          return IntraLineDiffCalculatorResult.TIMEOUT;
+
+        default:
+          return IntraLineDiffCalculatorResult.NO_RESULT;
+      }
+    }
+  }
+
+  private static class PatchListFileChange implements PatchFileChange {
+
+    private final PatchListEntry patchListEntry;
+
+    PatchListFileChange(PatchListEntry patchListEntry) {
+      this.patchListEntry = patchListEntry;
+    }
+
+    @Override
+    public ImmutableList<Edit> getEdits() {
+      return patchListEntry.getEdits();
+    }
+
+    @Override
+    public ImmutableSet<Edit> getEditsDueToRebase() {
+      return patchListEntry.getEditsDueToRebase();
+    }
+
+    @Override
+    public String getNewName() {
+      return patchListEntry.getNewName();
+    }
+
+    @Override
+    public String getOldName() {
+      return patchListEntry.getOldName();
+    }
+
+    @Override
+    public ChangeType getChangeType() {
+      return patchListEntry.getChangeType();
+    }
+
+    @Override
+    public List<String> getHeaderLines() {
+      return patchListEntry.getHeaderLines();
+    }
+
+    @Override
+    public Patch.PatchType getPatchType() {
+      return patchListEntry.getPatchType();
+    }
+  }
 }
diff --git a/java/com/google/gerrit/server/restapi/change/GetDiff.java b/java/com/google/gerrit/server/restapi/change/GetDiff.java
index a6536ce..82bdc34 100644
--- a/java/com/google/gerrit/server/restapi/change/GetDiff.java
+++ b/java/com/google/gerrit/server/restapi/change/GetDiff.java
@@ -14,27 +14,18 @@
 
 package com.google.gerrit.server.restapi.change;
 
-import static com.google.common.base.Preconditions.checkState;
 import static com.google.gerrit.util.cli.Localizable.localizable;
 
 import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.PatchScript;
-import com.google.gerrit.common.data.PatchScript.DisplayMethod;
-import com.google.gerrit.entities.Patch;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo;
 import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
-import com.google.gerrit.extensions.common.ChangeType;
 import com.google.gerrit.extensions.common.DiffInfo;
-import com.google.gerrit.extensions.common.DiffInfo.ContentEntry;
-import com.google.gerrit.extensions.common.DiffInfo.FileMeta;
-import com.google.gerrit.extensions.common.DiffInfo.IntraLineStatus;
 import com.google.gerrit.extensions.common.DiffWebLinkInfo;
 import com.google.gerrit.extensions.common.WebLinkInfo;
 import com.google.gerrit.extensions.restapi.AuthException;
@@ -44,12 +35,12 @@
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.jgit.diff.ReplaceEdit;
-import com.google.gerrit.prettify.common.SparseFileContent;
 import com.google.gerrit.server.WebLinks;
-import com.google.gerrit.server.change.FileContentUtil;
 import com.google.gerrit.server.change.FileResource;
 import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.diff.DiffInfoCreator;
+import com.google.gerrit.server.diff.DiffSide;
+import com.google.gerrit.server.diff.DiffWebLinksProvider;
 import com.google.gerrit.server.git.LargeObjectException;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.patch.PatchScriptFactory;
@@ -60,10 +51,7 @@
 import com.google.gerrit.server.project.ProjectState;
 import com.google.inject.Inject;
 import java.io.IOException;
-import java.util.List;
-import java.util.Set;
 import java.util.concurrent.TimeUnit;
-import org.eclipse.jgit.diff.Edit;
 import org.kohsuke.args4j.CmdLineException;
 import org.kohsuke.args4j.CmdLineParser;
 import org.kohsuke.args4j.NamedOptionDef;
@@ -76,17 +64,6 @@
 public class GetDiff implements RestReadView<FileResource> {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
-  private static final ImmutableMap<Patch.ChangeType, ChangeType> CHANGE_TYPE =
-      Maps.immutableEnumMap(
-          new ImmutableMap.Builder<Patch.ChangeType, ChangeType>()
-              .put(Patch.ChangeType.ADDED, ChangeType.ADDED)
-              .put(Patch.ChangeType.MODIFIED, ChangeType.MODIFIED)
-              .put(Patch.ChangeType.DELETED, ChangeType.DELETED)
-              .put(Patch.ChangeType.RENAMED, ChangeType.RENAMED)
-              .put(Patch.ChangeType.COPIED, ChangeType.COPIED)
-              .put(Patch.ChangeType.REWRITE, ChangeType.REWRITE)
-              .build());
-
   private final ProjectCache projectCache;
   private final PatchScriptFactory.Factory patchScriptFactoryFactory;
   private final Revisions revisions;
@@ -164,111 +141,18 @@
       psf.setLoadHistory(false);
       psf.setLoadComments(context != DiffPreferencesInfo.WHOLE_FILE_CONTEXT);
       PatchScript ps = psf.call();
-      ContentCollector contentCollector = new ContentCollector(ps);
-      Set<Edit> editsDueToRebase = ps.getEditsDueToRebase();
-      for (Edit edit : ps.getEdits()) {
-        logger.atFine().log("next edit = %s", edit);
-
-        if (edit.getType() == Edit.Type.EMPTY) {
-          logger.atFine().log("skip empty edit");
-          continue;
-        }
-        contentCollector.addCommon(edit.getBeginA());
-
-        checkState(
-            contentCollector.nextA == edit.getBeginA(),
-            "nextA = %s; want %s",
-            contentCollector.nextA,
-            edit.getBeginA());
-        checkState(
-            contentCollector.nextB == edit.getBeginB(),
-            "nextB = %s; want %s",
-            contentCollector.nextB,
-            edit.getBeginB());
-        switch (edit.getType()) {
-          case DELETE:
-          case INSERT:
-          case REPLACE:
-            List<Edit> internalEdit =
-                edit instanceof ReplaceEdit ? ((ReplaceEdit) edit).getInternalEdits() : null;
-            boolean dueToRebase = editsDueToRebase.contains(edit);
-            contentCollector.addDiff(edit.getEndA(), edit.getEndB(), internalEdit, dueToRebase);
-            break;
-          case EMPTY:
-          default:
-            throw new IllegalStateException();
-        }
-      }
-      contentCollector.addCommon(ps.getA().size());
-
-      ProjectState state = projectCache.get(resource.getRevision().getChange().getProject());
-
-      DiffInfo result = new DiffInfo();
-      String revA = basePatchSet != null ? basePatchSet.refName() : ps.getFileInfoA().commitId;
-      String revB =
-          resource.getRevision().getEdit().isPresent()
-              ? resource.getRevision().getEdit().get().getRefName()
-              : resource.getRevision().getPatchSet().refName();
-      logger.atFine().log("revA = %s, revB = %s", revA, revB);
-
-      ImmutableList<DiffWebLinkInfo> links =
-          webLinks.getDiffLinks(
-              state.getName(),
-              resource.getPatchKey().patchSetId().changeId().get(),
-              basePatchSet != null ? basePatchSet.id().get() : null,
-              revA,
+      Project.NameKey projectName = resource.getRevision().getChange().getProject();
+      ProjectState state = projectCache.get(projectName);
+      DiffSide sideA =
+          DiffSide.create(
+              ps.getFileInfoA(),
               MoreObjects.firstNonNull(ps.getOldName(), ps.getNewName()),
-              resource.getPatchKey().patchSetId().get(),
-              revB,
-              ps.getNewName());
-      result.webLinks = links.isEmpty() ? null : links;
-
-      if (ps.isBinary()) {
-        result.binary = true;
-      }
-      if (ps.getDisplayMethodA() != DisplayMethod.NONE) {
-        result.metaA = new FileMeta();
-        result.metaA.name = MoreObjects.firstNonNull(ps.getOldName(), ps.getNewName());
-        result.metaA.contentType =
-            FileContentUtil.resolveContentType(
-                state, result.metaA.name, ps.getFileModeA(), ps.getMimeTypeA());
-        result.metaA.lines = ps.getA().size();
-        result.metaA.webLinks = getFileWebLinks(state.getProject(), revA, result.metaA.name);
-        result.metaA.commitId = ps.getFileInfoA().commitId;
-      }
-
-      if (ps.getDisplayMethodB() != DisplayMethod.NONE) {
-        result.metaB = new FileMeta();
-        result.metaB.name = ps.getNewName();
-        result.metaB.contentType =
-            FileContentUtil.resolveContentType(
-                state, result.metaB.name, ps.getFileModeB(), ps.getMimeTypeB());
-        result.metaB.lines = ps.getB().size();
-        result.metaB.webLinks = getFileWebLinks(state.getProject(), revB, result.metaB.name);
-        result.metaB.commitId = ps.getFileInfoB().commitId;
-      }
-
-      if (intraline) {
-        if (ps.hasIntralineTimeout()) {
-          result.intralineStatus = IntraLineStatus.TIMEOUT;
-        } else if (ps.hasIntralineFailure()) {
-          result.intralineStatus = IntraLineStatus.FAILURE;
-        } else {
-          result.intralineStatus = IntraLineStatus.OK;
-        }
-        logger.atFine().log("intralineStatus = %s", result.intralineStatus);
-      }
-
-      result.changeType = CHANGE_TYPE.get(ps.getChangeType());
-      logger.atFine().log("changeType = %s", result.changeType);
-      if (result.changeType == null) {
-        throw new IllegalStateException("unknown change type: " + ps.getChangeType());
-      }
-
-      if (ps.getPatchHeader().size() > 0) {
-        result.diffHeader = ps.getPatchHeader();
-      }
-      result.content = contentCollector.lines;
+              DiffSide.Type.SIDE_A);
+      DiffSide sideB = DiffSide.create(ps.getFileInfoB(), ps.getNewName(), DiffSide.Type.SIDE_B);
+      DiffWebLinksProvider webLinksProvider =
+          new DiffWebLinksProviderImpl(sideA, sideB, projectName, basePatchSet, webLinks, resource);
+      DiffInfoCreator diffInfoCreator = new DiffInfoCreator(state, webLinksProvider, intraline);
+      DiffInfo result = diffInfoCreator.create(ps, sideA, sideB);
 
       Response<DiffInfo> r = Response.ok(result);
       if (resource.isCacheable()) {
@@ -282,9 +166,69 @@
     }
   }
 
-  private List<WebLinkInfo> getFileWebLinks(Project project, String rev, String file) {
-    ImmutableList<WebLinkInfo> links = webLinks.getFileLinks(project.getName(), rev, file);
-    return links.isEmpty() ? null : links;
+  private static class DiffWebLinksProviderImpl implements DiffWebLinksProvider {
+
+    private final WebLinks webLinks;
+    private final Project.NameKey projectName;
+    private final DiffSide sideA;
+    private final DiffSide sideB;
+    private final String revA;
+    private final String revB;
+    private final FileResource resource;
+    @Nullable private final PatchSet basePatchSet;
+
+    DiffWebLinksProviderImpl(
+        DiffSide sideA,
+        DiffSide sideB,
+        Project.NameKey projectName,
+        @Nullable PatchSet basePatchSet,
+        WebLinks webLinks,
+        FileResource resource) {
+      this.projectName = projectName;
+      this.webLinks = webLinks;
+      this.basePatchSet = basePatchSet;
+      this.resource = resource;
+      this.sideA = sideA;
+      this.sideB = sideB;
+
+      revA = basePatchSet != null ? basePatchSet.refName() : sideA.fileInfo().commitId;
+
+      RevisionResource revision = resource.getRevision();
+      revB =
+          revision
+              .getEdit()
+              .map(edit -> edit.getRefName())
+              .orElseGet(() -> revision.getPatchSet().refName());
+
+      logger.atFine().log("revA = %s, revB = %s", revA, revB);
+    }
+
+    @Override
+    public ImmutableList<DiffWebLinkInfo> getDiffLinks() {
+      return webLinks.getDiffLinks(
+          projectName.get(),
+          resource.getPatchKey().patchSetId().changeId().get(),
+          basePatchSet != null ? basePatchSet.id().get() : null,
+          revA,
+          sideA.fileName(),
+          resource.getPatchKey().patchSetId().get(),
+          revB,
+          sideB.fileName());
+    }
+
+    @Override
+    public ImmutableList<WebLinkInfo> getFileWebLinks(DiffSide.Type type) {
+      String rev;
+      DiffSide side;
+      if (type == DiffSide.Type.SIDE_A) {
+        rev = revA;
+        side = sideA;
+      } else {
+        rev = revB;
+        side = sideB;
+      }
+      return webLinks.getFileLinks(projectName.get(), rev, side.fileName());
+    }
   }
 
   public GetDiff setBase(String base) {
@@ -312,141 +256,6 @@
     return this;
   }
 
-  private static class ContentCollector {
-    final List<ContentEntry> lines;
-    final SparseFileContent fileA;
-    final SparseFileContent fileB;
-    final boolean ignoreWS;
-
-    int nextA;
-    int nextB;
-
-    ContentCollector(PatchScript ps) {
-      lines = Lists.newArrayListWithExpectedSize(ps.getEdits().size() + 2);
-      fileA = ps.getA();
-      fileB = ps.getB();
-      ignoreWS = ps.isIgnoreWhitespace();
-    }
-
-    void addCommon(int end) {
-      logger.atFine().log("addCommon: end = %d", end);
-
-      end = Math.min(end, fileA.size());
-      logger.atFine().log("end = %d", end);
-
-      if (nextA >= end) {
-        logger.atFine().log("nextA >= end: nextA = %d, end = %d", nextA, end);
-        return;
-      }
-
-      while (nextA < end) {
-        logger.atFine().log("nextA < end: nextA = %d, end = %d", nextA, end);
-
-        if (!fileA.contains(nextA)) {
-          logger.atFine().log("fileA does not contain nextA: nextA = %d", nextA);
-
-          int endRegion = Math.min(end, nextA == 0 ? fileA.first() : fileA.next(nextA - 1));
-          int len = endRegion - nextA;
-          entry().skip = len;
-          nextA = endRegion;
-          nextB += len;
-
-          logger.atFine().log("setting: nextA = %d, nextB = %d", nextA, nextB);
-          continue;
-        }
-
-        ContentEntry e = null;
-        for (int i = nextA; i == nextA && i < end; i = fileA.next(i), nextA++, nextB++) {
-          if (ignoreWS && fileB.contains(nextB)) {
-            if (e == null || e.common == null) {
-              logger.atFine().log("create new common entry: nextA = %d, nextB = %d", nextA, nextB);
-              e = entry();
-              e.a = Lists.newArrayListWithCapacity(end - nextA);
-              e.b = Lists.newArrayListWithCapacity(end - nextA);
-              e.common = true;
-            }
-            e.a.add(fileA.get(nextA));
-            e.b.add(fileB.get(nextB));
-          } else {
-            if (e == null || e.common != null) {
-              logger.atFine().log(
-                  "create new non-common entry: nextA = %d, nextB = %d", nextA, nextB);
-              e = entry();
-              e.ab = Lists.newArrayListWithCapacity(end - nextA);
-            }
-            e.ab.add(fileA.get(nextA));
-          }
-        }
-        logger.atFine().log("nextA = %d, nextB = %d", nextA, nextB);
-      }
-    }
-
-    void addDiff(int endA, int endB, List<Edit> internalEdit, boolean dueToRebase) {
-      logger.atFine().log(
-          "addDiff: endA = %d, endB = %d, numberOfInternalEdits = %d, dueToRebase = %s",
-          endA, endB, internalEdit != null ? internalEdit.size() : 0, dueToRebase);
-
-      int lenA = endA - nextA;
-      int lenB = endB - nextB;
-      logger.atFine().log("lenA = %d, lenB = %d", lenA, lenB);
-      checkState(lenA > 0 || lenB > 0);
-
-      logger.atFine().log("create non-common entry");
-      ContentEntry e = entry();
-      if (lenA > 0) {
-        logger.atFine().log("lenA > 0: lenA = %d", lenA);
-        e.a = Lists.newArrayListWithCapacity(lenA);
-        for (; nextA < endA; nextA++) {
-          e.a.add(fileA.get(nextA));
-        }
-      }
-      if (lenB > 0) {
-        logger.atFine().log("lenB > 0: lenB = %d", lenB);
-        e.b = Lists.newArrayListWithCapacity(lenB);
-        for (; nextB < endB; nextB++) {
-          e.b.add(fileB.get(nextB));
-        }
-      }
-      if (internalEdit != null && !internalEdit.isEmpty()) {
-        logger.atFine().log("processing internal edits");
-
-        e.editA = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
-        e.editB = Lists.newArrayListWithCapacity(internalEdit.size() * 2);
-        int lastA = 0;
-        int lastB = 0;
-        for (Edit edit : internalEdit) {
-          logger.atFine().log("internal edit = %s", edit);
-
-          if (edit.getBeginA() != edit.getEndA()) {
-            logger.atFine().log(
-                "edit.getBeginA() != edit.getEndA(): edit.getBeginA() = %d, edit.getEndA() = %d",
-                edit.getBeginA(), edit.getEndA());
-            e.editA.add(
-                ImmutableList.of(edit.getBeginA() - lastA, edit.getEndA() - edit.getBeginA()));
-            lastA = edit.getEndA();
-            logger.atFine().log("lastA = %d", lastA);
-          }
-          if (edit.getBeginB() != edit.getEndB()) {
-            logger.atFine().log(
-                "edit.getBeginB() != edit.getEndB(): edit.getBeginB() = %d, edit.getEndB() = %d",
-                edit.getBeginB(), edit.getEndB());
-            e.editB.add(
-                ImmutableList.of(edit.getBeginB() - lastB, edit.getEndB() - edit.getBeginB()));
-            lastB = edit.getEndB();
-            logger.atFine().log("lastB = %d", lastB);
-          }
-        }
-      }
-      e.dueToRebase = dueToRebase ? true : null;
-    }
-
-    private ContentEntry entry() {
-      ContentEntry e = new ContentEntry();
-      lines.add(e);
-      return e;
-    }
-  }
-
   @Deprecated
   enum IgnoreWhitespace {
     NONE(DiffPreferencesInfo.Whitespace.IGNORE_NONE),
@@ -462,6 +271,7 @@
   }
 
   public static class ContextOptionHandler extends OptionHandler<Short> {
+
     public ContextOptionHandler(CmdLineParser parser, OptionDef option, Setter<Short> setter) {
       super(parser, option, setter);
     }
diff --git a/java/com/google/gerrit/server/submit/MergeOp.java b/java/com/google/gerrit/server/submit/MergeOp.java
index 75ae62d..8bdd98e 100644
--- a/java/com/google/gerrit/server/submit/MergeOp.java
+++ b/java/com/google/gerrit/server/submit/MergeOp.java
@@ -41,6 +41,7 @@
 import com.google.gerrit.entities.ChangeMessage;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
@@ -237,7 +238,7 @@
   private final Map<Change.Id, Change> updatedChanges;
 
   private Timestamp ts;
-  private RequestId submissionId;
+  private SubmissionId submissionId;
   private IdentifiedUser caller;
 
   private MergeOpRepoManager orm;
@@ -449,7 +450,7 @@
     this.dryrun = dryrun;
     this.caller = caller;
     this.ts = TimeUtil.nowTs();
-    this.submissionId = new RequestId(change.getId().toString());
+    this.submissionId = new SubmissionId(change);
 
     try (TraceContext traceContext =
         TraceContext.open().addTag(RequestId.Type.SUBMISSION_ID, submissionId)) {
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategy.java b/java/com/google/gerrit/server/submit/SubmitStrategy.java
index 4c68e1b..efb2f76 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategy.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategy.java
@@ -21,6 +21,7 @@
 import com.google.common.collect.Sets;
 import com.google.gerrit.entities.BranchNameKey;
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.extensions.config.FactoryModule;
@@ -42,7 +43,6 @@
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.git.TagCache;
 import com.google.gerrit.server.git.validators.OnSubmitValidators;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.project.ProjectConfig;
@@ -94,7 +94,7 @@
           RevFlag canMergeFlag,
           Set<RevCommit> alreadyAccepted,
           Set<CodeReviewCommit> incoming,
-          RequestId submissionId,
+          SubmissionId submissionId,
           SubmitInput submitInput,
           SubmoduleOp submoduleOp,
           boolean dryrun);
@@ -125,7 +125,7 @@
     final MergeTip mergeTip;
     final RevFlag canMergeFlag;
     final Set<RevCommit> alreadyAccepted;
-    final RequestId submissionId;
+    final SubmissionId submissionId;
     final SubmitType submitType;
     final SubmitInput submitInput;
     final SubmoduleOp submoduleOp;
@@ -164,7 +164,7 @@
         @Assisted RevFlag canMergeFlag,
         @Assisted Set<RevCommit> alreadyAccepted,
         @Assisted Set<CodeReviewCommit> incoming,
-        @Assisted RequestId submissionId,
+        @Assisted SubmissionId submissionId,
         @Assisted SubmitType submitType,
         @Assisted SubmitInput submitInput,
         @Assisted SubmoduleOp submoduleOp,
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java b/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java
index cba572bc..a015665 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyFactory.java
@@ -16,13 +16,13 @@
 
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.entities.BranchNameKey;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.extensions.api.changes.SubmitInput;
 import com.google.gerrit.extensions.client.SubmitType;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.CodeReviewCommit;
 import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk;
 import com.google.gerrit.server.git.MergeTip;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.submit.MergeOp.CommitStatus;
 import com.google.inject.Inject;
 import com.google.inject.Singleton;
@@ -52,7 +52,7 @@
       IdentifiedUser caller,
       MergeTip mergeTip,
       CommitStatus commitStatus,
-      RequestId submissionId,
+      SubmissionId submissionId,
       SubmitInput submitInput,
       SubmoduleOp submoduleOp,
       boolean dryrun)
diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
index b7bc58a..6be4a91 100644
--- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
+++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java
@@ -453,7 +453,7 @@
     Change c = ctx.getChange();
     logger.atFine().log("Setting change %s merged", c.getId());
     c.setStatus(Change.Status.MERGED);
-    c.setSubmissionId(args.submissionId.toStringForStorage());
+    c.setSubmissionId(args.submissionId.toString());
 
     // TODO(dborowitz): We need to be able to change the author of the message,
     // which is not the user from the update context. addMergedMessage was able
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index 4f9a67c..f3636c9 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -335,6 +335,12 @@
                 if (retryWithTraceOnFailure
                     && opts.retryWithTrace().isPresent()
                     && opts.retryWithTrace().get().test(t)) {
+                  // Exception hooks may identify exceptions for which retrying with trace should be
+                  // skipped.
+                  if (exceptionHooks.stream().anyMatch(h -> h.skipRetryWithTrace(t))) {
+                    return false;
+                  }
+
                   String caller = opts.caller().orElse("N/A");
                   String cause = formatCause(t);
                   if (!traceContext.isTracing()) {
diff --git a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
index 08719d3..ee5f117 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/ConsistencyCheckerIT.java
@@ -29,6 +29,7 @@
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.extensions.api.changes.FixInput;
 import com.google.gerrit.extensions.client.ChangeStatus;
 import com.google.gerrit.extensions.common.ChangeInfo;
@@ -39,7 +40,6 @@
 import com.google.gerrit.server.change.ConsistencyChecker;
 import com.google.gerrit.server.change.NotifyResolver;
 import com.google.gerrit.server.change.PatchSetInserter;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.notedb.ChangeNoteUtil;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.Sequences;
@@ -315,7 +315,7 @@
             public boolean updateChange(ChangeContext ctx) {
               ctx.getChange().setStatus(Change.Status.MERGED);
               ctx.getUpdate(ctx.getChange().currentPatchSetId())
-                  .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
+                  .fixStatusToMerged(new SubmissionId(ctx.getChange()));
               return true;
             }
           });
@@ -865,7 +865,7 @@
             public boolean updateChange(ChangeContext ctx) {
               ctx.getChange().setStatus(Change.Status.MERGED);
               ctx.getUpdate(ctx.getChange().currentPatchSetId())
-                  .fixStatusToMerged(new RequestId(ctx.getChange().getId().toString()));
+                  .fixStatusToMerged(new SubmissionId(ctx.getChange()));
               return true;
             }
           });
diff --git a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
index 9c4bdef4..8577c16 100644
--- a/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
+++ b/javatests/com/google/gerrit/integration/git/GitProtocolV2IT.java
@@ -35,6 +35,7 @@
 import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.extensions.api.GerritApi;
 import com.google.gerrit.extensions.common.ChangeInput;
+import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.inject.Inject;
@@ -47,12 +48,15 @@
 
 @UseSsh
 public class GitProtocolV2IT extends StandaloneSiteTest {
+  private static final String ADMIN_PASSWORD = "secret";
   private final String[] SSH_KEYGEN_CMD =
       new String[] {"ssh-keygen", "-t", "rsa", "-q", "-P", "", "-f"};
   private final String[] GIT_LS_REMOTE =
       new String[] {"git", "-c", "protocol.version=2", "ls-remote", "-o", "trace=12345"};
   private final String[] GIT_CLONE_MIRROR =
       new String[] {"git", "-c", "protocol.version=2", "clone", "--mirror"};
+  private final String[] GIT_FETCH = new String[] {"git", "-c", "protocol.version=2", "fetch"};
+  private final String[] GIT_INIT = new String[] {"git", "init"};
   private final String GIT_SSH_COMMAND =
       "ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -i";
 
@@ -61,6 +65,7 @@
   @Inject private ProjectOperations projectOperations;
   @Inject private @TestSshServerAddress InetSocketAddress sshAddress;
   @Inject private @GerritServerConfig Config config;
+  @Inject private AllProjectsName allProjectsName;
 
   @BeforeClass
   public static void assertGitClientVersion() throws Exception {
@@ -131,7 +136,7 @@
               .commit;
 
       // Prepare new change on secret branch
-      in = new ChangeInput(project.get(), "secret", "Test secret change");
+      in = new ChangeInput(project.get(), ADMIN_PASSWORD, "Test secret change");
       in.newBranch = true;
 
       // Create new change and retrieve SHA1 for the created patch set
@@ -257,9 +262,82 @@
     }
   }
 
+  @Test
+  public void testGitWireProtocolV2FetchIndividualRef() throws Exception {
+    try (ServerContext ctx = startServer()) {
+      ctx.getInjector().injectMembers(this);
+
+      // Setup admin password
+      gApi.accounts().id(admin.username()).setHttpPassword(ADMIN_PASSWORD);
+
+      // Get authenticated Git/HTTP URL
+      String urlWithCredentials =
+          config
+              .getString("gerrit", null, "canonicalweburl")
+              .replace("http://", "http://" + admin.username() + ":" + ADMIN_PASSWORD + "@");
+
+      // Create project
+      Project.NameKey privateProject = Project.nameKey("private-project");
+      gApi.projects().create(privateProject.get());
+
+      // Set protocol.version=2 in target repository
+      execute(
+          ImmutableList.of("git", "config", "protocol.version", "2"),
+          sitePaths
+              .site_path
+              .resolve("git")
+              .resolve(privateProject.get() + Constants.DOT_GIT)
+              .toFile());
+
+      // Disallow general read permissions for anonymous users
+      projectOperations
+          .project(allProjectsName)
+          .forUpdate()
+          .add(deny(Permission.READ).ref("refs/*").group(SystemGroupBackend.ANONYMOUS_USERS))
+          .add(
+              allow(Permission.READ)
+                  .ref("refs/heads/master")
+                  .group(SystemGroupBackend.REGISTERED_USERS))
+          .update();
+
+      // Set up project permission to allow registered users fetching changes/*
+      projectOperations
+          .project(privateProject)
+          .forUpdate()
+          .add(
+              allow(Permission.READ)
+                  .ref("refs/changes/*")
+                  .group(SystemGroupBackend.REGISTERED_USERS))
+          .update();
+
+      // Create new change and retrieve refs for the created patch set
+      ChangeInput visibleChangeIn =
+          new ChangeInput(privateProject.get(), "master", "Test private change");
+      visibleChangeIn.newBranch = true;
+      int visibleChangeNumber = gApi.changes().create(visibleChangeIn).info()._number;
+      Change.Id changeId = Change.id(visibleChangeNumber);
+      String visibleChangeNumberRef = RefNames.patchSetRef(PatchSet.id(changeId, 1));
+
+      // Fetch a single ref using git wire protocol v2 over HTTP with authentication
+      execute(GIT_INIT);
+
+      String outFetchRef =
+          execute(
+              ImmutableList.<String>builder()
+                  .add(GIT_FETCH)
+                  .add(urlWithCredentials + "/" + privateProject.get())
+                  .add(visibleChangeNumberRef)
+                  .build(),
+              ImmutableMap.of("GIT_TRACE_PACKET", "1"));
+
+      assertThat(outFetchRef).contains("git< version 2");
+      assertThat(outFetchRef).contains(visibleChangeNumberRef);
+    }
+  }
+
   private void setUpUserAuthentication(String username) throws Exception {
     // Assign HTTP password to user
-    gApi.accounts().id(username).setHttpPassword("secret");
+    gApi.accounts().id(username).setHttpPassword(ADMIN_PASSWORD);
 
     // Generate private/public key for user
     execute(
@@ -285,6 +363,10 @@
     assertThat(out).contains(commit);
   }
 
+  private String execute(String... cmds) throws Exception {
+    return execute(ImmutableList.<String>builder().add(cmds).build());
+  }
+
   private String execute(ImmutableList<String> cmd) throws Exception {
     return execute(cmd, sitePaths.data_dir.toFile(), ImmutableMap.of());
   }
diff --git a/javatests/com/google/gerrit/prettify/BUILD b/javatests/com/google/gerrit/prettify/BUILD
new file mode 100644
index 0000000..0eb7cee
--- /dev/null
+++ b/javatests/com/google/gerrit/prettify/BUILD
@@ -0,0 +1,13 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+
+junit_tests(
+    name = "prettify_tests",
+    srcs = glob(["**/*.java"]),
+    deps = [
+        "//java/com/google/gerrit/prettify:server",
+        "//java/com/google/gerrit/prettify/common/testing",
+        "//java/com/google/gerrit/testing:gerrit-test-util",
+        "//lib:guava",
+        "//lib/truth",
+    ],
+)
diff --git a/javatests/com/google/gerrit/prettify/common/SparseFileContentBuilderTest.java b/javatests/com/google/gerrit/prettify/common/SparseFileContentBuilderTest.java
new file mode 100644
index 0000000..a751d50
--- /dev/null
+++ b/javatests/com/google/gerrit/prettify/common/SparseFileContentBuilderTest.java
@@ -0,0 +1,170 @@
+// Copyright (C) 2019 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.prettify.common;
+
+import static com.google.gerrit.prettify.common.testing.SparseFileContentSubject.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableMap;
+import org.junit.Ignore;
+import org.junit.Test;
+
+public class SparseFileContentBuilderTest {
+
+  @Test
+  public void addLineWithNegativeNumber() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(10);
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(-1, "First line"));
+
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(-5, "First line"));
+  }
+
+  @Test
+  @Ignore
+  public void addLineNumberZeroFileSize() {
+    // Temporary ignore - see comments in SparseFileContentBuilder.build() method
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(0);
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(0, "First line"));
+  }
+
+  @Test
+  @Ignore
+  public void addLineNumberNonZeroFileSize() {
+    // Temporary ignore - see comments in SparseFileContentBuilder.build() method
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(5);
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(5, "First line"));
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(6, "First line"));
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(7, "First line"));
+  }
+
+  @Test
+  public void addLineIncorrectOrder() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(5);
+
+    builder.addLine(0, "First line");
+    builder.addLine(1, "Second line");
+    builder.addLine(3, "Third line");
+    builder.addLine(4, "Fourth line");
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(4, "Other Line"));
+
+    assertThrows(IllegalArgumentException.class, () -> builder.addLine(2, "Other Line"));
+  }
+
+  @Test
+  public void emptyContentZeroSize() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(0);
+
+    SparseFileContent content = builder.build();
+    assertThat(content).getSize().isEqualTo(0);
+    assertThat(content).getRangesCount().isEqualTo(0);
+    assertThat(content).lines().isEmpty();
+  }
+
+  @Test
+  public void emptyContentNonZeroSize() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(4);
+    SparseFileContent content = builder.build();
+    assertThat(content).getSize().isEqualTo(4);
+    assertThat(content).getRangesCount().isEqualTo(0);
+    assertThat(content).lines().isEmpty();
+  }
+
+  @Test
+  public void oneLineContentLineNumberZero() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(1);
+
+    builder.addLine(0, "First line");
+    SparseFileContent content = builder.build();
+    assertThat(content).getSize().isEqualTo(1);
+    assertThat(content).getRangesCount().isEqualTo(1);
+    assertThat(content).lines().containsExactlyEntriesIn(ImmutableMap.of(0, "First line"));
+  }
+
+  @Test
+  public void oneLineContentLineNumberNotZero() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(6);
+
+    builder.addLine(5, "First line");
+    SparseFileContent content = builder.build();
+    assertThat(content).getSize().isEqualTo(6);
+    assertThat(content).getRangesCount().isEqualTo(1);
+    assertThat(content).lines().containsExactlyEntriesIn(ImmutableMap.of(5, "First line"));
+  }
+
+  @Test
+  public void multiLineContinuousContentStartingFromZero() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(5);
+
+    builder.addLine(0, "First line");
+    builder.addLine(1, "Second line");
+    builder.addLine(2, "Third line");
+    SparseFileContent content = builder.build();
+    assertThat(content).getSize().isEqualTo(5);
+    assertThat(content).getRangesCount().isEqualTo(1);
+    assertThat(content)
+        .lines()
+        .containsExactlyEntriesIn(
+            ImmutableMap.of(
+                0, "First line",
+                1, "Second line",
+                2, "Third line"));
+  }
+
+  @Test
+  public void multiLineContentStartingFromNonZeroLine() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(8);
+
+    builder.addLine(5, "First line");
+    builder.addLine(6, "Second line");
+    builder.addLine(7, "Third line");
+    SparseFileContent content = builder.build();
+    assertThat(content).getSize().isEqualTo(8);
+    assertThat(content).getRangesCount().isEqualTo(1);
+    assertThat(content)
+        .lines()
+        .containsExactlyEntriesIn(
+            ImmutableMap.of(
+                5, "First line",
+                6, "Second line",
+                7, "Third line"));
+  }
+
+  @Test
+  public void multiLineContentWithGaps() {
+    SparseFileContentBuilder builder = new SparseFileContentBuilder(10000);
+    builder.addLine(0, "First line");
+    builder.addLine(1, "Second line");
+    builder.addLine(3, "Third line");
+    builder.addLine(4, "Fourth line");
+    builder.addLine(5, "Fifth line");
+    builder.addLine(6, "Sixth line");
+    builder.addLine(10, "Seventh line");
+    SparseFileContent content = builder.build();
+    assertThat(content).getSize().isEqualTo(10000);
+    assertThat(content).getRangesCount().isEqualTo(3);
+    assertThat(content)
+        .lines()
+        .containsExactlyEntriesIn(
+            ImmutableMap.builder()
+                .put(0, "First line")
+                .put(1, "Second line")
+                .put(3, "Third line")
+                .put(4, "Fourth line")
+                .put(5, "Fifth line")
+                .put(6, "Sixth line")
+                .put(10, "Seventh line")
+                .build());
+  }
+}
diff --git a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
index 145e914..db0dec8 100644
--- a/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
+++ b/javatests/com/google/gerrit/server/notedb/ChangeNotesTest.java
@@ -45,6 +45,7 @@
 import com.google.gerrit.entities.CommentRange;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.mail.Address;
 import com.google.gerrit.server.AssigneeStatusUpdate;
@@ -52,7 +53,6 @@
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.ReviewerSet;
 import com.google.gerrit.server.config.GerritServerId;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
 import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.TestChanges;
@@ -432,7 +432,7 @@
   @Test
   public void approvalsPostSubmit() throws Exception {
     Change c = newChange();
-    RequestId submissionId = submissionId(c);
+    SubmissionId submissionId = new SubmissionId(c);
     ChangeUpdate update = newUpdate(c, changeOwner);
     update.putApproval("Code-Review", (short) 1);
     update.putApproval("Verified", (short) 1);
@@ -467,7 +467,7 @@
   @Test
   public void approvalsDuringSubmit() throws Exception {
     Change c = newChange();
-    RequestId submissionId = submissionId(c);
+    SubmissionId submissionId = new SubmissionId(c);
     ChangeUpdate update = newUpdate(c, changeOwner);
     update.putApproval("Code-Review", (short) 1);
     update.putApproval("Verified", (short) 1);
@@ -604,7 +604,7 @@
   @Test
   public void submitRecords() throws Exception {
     Change c = newChange();
-    RequestId submissionId = submissionId(c);
+    SubmissionId submissionId = new SubmissionId(c);
     ChangeUpdate update = newUpdate(c, changeOwner);
     update.setSubjectForCommit("Submit patch set 1");
 
@@ -640,13 +640,13 @@
                 null,
                 submitLabel("Verified", "OK", changeOwner.getAccountId()),
                 submitLabel("Alternative-Code-Review", "NEED", null)));
-    assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toStringForStorage());
+    assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toString());
   }
 
   @Test
   public void latestSubmitRecordsOnly() throws Exception {
     Change c = newChange();
-    RequestId submissionId = submissionId(c);
+    SubmissionId submissionId = new SubmissionId(c);
     ChangeUpdate update = newUpdate(c, changeOwner);
     update.setSubjectForCommit("Submit patch set 1");
     update.merge(
@@ -669,7 +669,7 @@
     assertThat(notes.getSubmitRecords())
         .containsExactly(
             submitRecord("OK", null, submitLabel("Code-Review", "OK", changeOwner.getAccountId())));
-    assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toStringForStorage());
+    assertThat(notes.getChange().getSubmissionId()).isEqualTo(submissionId.toString());
   }
 
   @Test
@@ -977,7 +977,7 @@
     // Finish off by merging the change.
     update = newUpdate(c, changeOwner);
     update.merge(
-        submissionId(c),
+        new SubmissionId(c),
         ImmutableList.of(
             submitRecord(
                 "NOT_READY",
@@ -3140,8 +3140,4 @@
     update.commit();
     return tr.parseBody(commit);
   }
-
-  private RequestId submissionId(Change c) {
-    return new RequestId(c.getId().toString());
-  }
 }
diff --git a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
index 97781a4..5e25c13 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitMessageOutputTest.java
@@ -21,9 +21,9 @@
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.mail.Address;
 import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.util.time.TimeUtil;
 import com.google.gerrit.testing.ConfigSuite;
 import com.google.gerrit.testing.TestChanges;
@@ -151,7 +151,7 @@
     ChangeUpdate update = newUpdate(c, changeOwner);
     update.setSubjectForCommit("Submit patch set 1");
 
-    RequestId submissionId = submissionId(c);
+    SubmissionId submissionId = new SubmissionId(c);
     update.merge(
         submissionId,
         ImmutableList.of(
@@ -174,7 +174,7 @@
             + "Patch-set: 1\n"
             + "Status: merged\n"
             + "Submission-id: "
-            + submissionId.toStringForStorage()
+            + submissionId.toString()
             + "\n"
             + "Submitted-with: NOT_READY\n"
             + "Submitted-with: OK: Verified: Gerrit User 1 <1@gerrit>\n"
@@ -223,7 +223,7 @@
     ChangeUpdate update = newUpdate(c, changeOwner);
     update.setSubjectForCommit("Submit patch set 1");
 
-    RequestId submissionId = submissionId(c);
+    SubmissionId submissionId = new SubmissionId(c);
     update.merge(
         submissionId, ImmutableList.of(submitRecord("RULE_ERROR", "Problem with patch set:\n1")));
     update.commit();
@@ -234,7 +234,7 @@
             + "Patch-set: 1\n"
             + "Status: merged\n"
             + "Submission-id: "
-            + submissionId.toStringForStorage()
+            + submissionId.toString()
             + "\n"
             + "Submitted-with: RULE_ERROR Problem with patch set: 1\n",
         update.getResult());
@@ -427,8 +427,4 @@
     RevCommit commit = parseCommit(commitId);
     assertThat(commit.getFullMessage()).isEqualTo(expected);
   }
-
-  private RequestId submissionId(Change c) {
-    return new RequestId(c.getId().toString());
-  }
 }
diff --git a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
index 5860c48..c7ca887 100644
--- a/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
+++ b/javatests/com/google/gerrit/server/update/BatchUpdateTest.java
@@ -26,12 +26,12 @@
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.entities.SubmissionId;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.change.ChangeInserter;
 import com.google.gerrit.server.change.PatchSetInserter;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.logging.RequestId;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.notedb.Sequences;
@@ -332,7 +332,7 @@
       cr.label = "Code-Review";
       sr.labels = ImmutableList.of(cr);
       ChangeUpdate update = ctx.getUpdate(ctx.getChange().currentPatchSetId());
-      update.merge(new RequestId(), ImmutableList.of(sr));
+      update.merge(new SubmissionId(ctx.getChange()), ImmutableList.of(sr));
       update.setChangeMessage("Submitted");
       return true;
     }
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html
index b116171..d801638 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.html
@@ -182,6 +182,11 @@
       test('input text change triggers function', () => {
         sandbox.spy(element, '_getRecentChanges');
         element.$.parentInput.noDebounce = true;
+        MockInteractions.pressAndReleaseKeyOn(
+            element.$.parentInput.$.input,
+            13,
+            null,
+            'enter');
         element._text = '1';
         assert.isTrue(element._getRecentChanges.calledOnce);
         element._text = '12';
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index d169728..3f2f4bd 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -24,7 +24,6 @@
     CATEGORY_RPC: 'RPC Timing',
     // Reported events - alphabetize below.
     APP_STARTED: 'App Started',
-    PAGE_LOADED: 'Page Loaded',
   };
 
   // Plugin-related reporting constants.
@@ -298,38 +297,21 @@
         console.error('pageLoaded should be called after window.onload');
         this.async(this.pageLoaded, 100);
       } else {
-        const loadTime = this.performanceTiming.loadEventEnd -
+        const perfEvents = Object.keys(this.performanceTiming.toJSON());
+        perfEvents.forEach(
+            eventName => this._reportPerformanceTiming(eventName)
+        );
+      }
+    },
+
+    _reportPerformanceTiming(eventName) {
+      const eventTiming = this.performanceTiming[eventName];
+      if (eventTiming > 0) {
+        const elapsedTime = eventTiming -
             this.performanceTiming.navigationStart;
+        // NavResTime - Navigation and resource timings.
         this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
-            TIMING.PAGE_LOADED, loadTime, true);
-
-        const requestStart = this.performanceTiming.requestStart -
-            this.performanceTiming.navigationStart;
-        this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
-            'requestStart', requestStart, true);
-
-        const responseEnd = this.performanceTiming.responseEnd -
-            this.performanceTiming.navigationStart;
-        this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
-            'responseEnd', responseEnd, true);
-
-        const domLoading = this.performanceTiming.domLoading -
-          this.performanceTiming.navigationStart;
-        this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
-            'domLoading', domLoading, true);
-
-        const domContentLoadedEventStart =
-          this.performanceTiming.domContentLoadedEventStart -
-          this.performanceTiming.navigationStart;
-        this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
-            'domContentLoadedEventStart', domContentLoadedEventStart, true);
-
-        if (this.performanceTiming.redirectEnd > 0) {
-          const redirectEnd = this.performanceTiming.redirectEnd -
-              this.performanceTiming.navigationStart;
-          this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
-              'redirectEnd', redirectEnd, true);
-        }
+            `NavResTime - ${eventName}`, elapsedTime, true);
       }
     },
 
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
index c2c0297..430b41e 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
@@ -52,6 +52,7 @@
         navigationStart: 1,
         loadEventEnd: 2,
       };
+      fakePerformance.toJSON = () => fakePerformance;
       sinon.stub(element, 'performanceTiming',
           {get() { return fakePerformance; }});
       sandbox.stub(element, 'reporter');
@@ -83,7 +84,7 @@
       element.pageLoaded();
       assert.isTrue(
           element.reporter.calledWithExactly(
-              'timing-report', 'UI Latency', 'Page Loaded',
+              'timing-report', 'UI Latency', 'NavResTime - loadEventEnd',
               fakePerformance.loadEventEnd - fakePerformance.navigationStart,
               true)
       );
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
index 3a60255..be3c48f 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js
@@ -264,13 +264,23 @@
         return;
       }
 
+      // Reset _suggestions for every update
+      // This will also prevent from carrying over suggestions:
+      // @see Issue 12039
+      this._suggestions = [];
+
+      // TODO(taoalpha): Also skip if text has not changed
+
       if (this._disableSuggestions) { return; }
       if (text.length < threshold) {
-        this._suggestions = [];
         this.value = '';
         return;
       }
 
+      if (!this._focused) {
+        return;
+      }
+
       const update = () => {
         this.query(text).then(suggestions => {
           if (text !== this.text) {
diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
index 39329e5..9df3cfc 100644
--- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html
@@ -38,6 +38,10 @@
   suite('gr-autocomplete tests', () => {
     let element;
     let sandbox;
+    const focusOnInput = element => {
+      MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null,
+          'enter');
+    };
 
     setup(() => {
       element = fixture('basic');
@@ -63,6 +67,7 @@
       assert.isTrue(element.$.suggestions.isHidden);
       assert.equal(element.$.suggestions.$.cursor.index, -1);
 
+      focusOnInput(element);
       element.text = 'blah';
 
       assert.isTrue(queryStub.called);
@@ -186,6 +191,7 @@
         return promise = Promise.resolve([{name: 'suggestion', value: 0}]);
       });
       element.query = queryStub;
+      focusOnInput(element);
       element.text = 'blah';
 
       promise.then(() => {
@@ -207,6 +213,7 @@
         return promise = Promise.resolve([{name: 'suggestion', value: 0}]);
       });
       element.query = queryStub;
+      focusOnInput(element);
       element.text = 'blah';
       element.clearOnCommit = true;
 
@@ -228,15 +235,11 @@
         return Promise.resolve([]);
       });
       element.query = queryStub;
-
       element.threshold = 2;
-
+      focusOnInput(element);
       element.text = 'a';
-
       assert.isFalse(queryStub.called);
-
       element.text = 'ab';
-
       assert.isTrue(queryStub.called);
     });
 
@@ -249,6 +252,7 @@
           (name, cb) => { callback = cb; });
       element.query = queryStub;
       element.noDebounce = false;
+      focusOnInput(element);
       element.text = 'a';
       assert.isFalse(queryStub.called);
       assert.isTrue(debounceStub.called);
@@ -269,11 +273,60 @@
       assert.equal(element._suggestions.length, 0);
     });
 
+    test('when focused', done => {
+      let promise;
+      const queryStub = sandbox.stub()
+          .returns(promise = Promise.resolve([{name: 'suggestion', value: 0}]));
+      element.query = queryStub;
+      element.suggestOnlyWhenFocus = true;
+      focusOnInput(element);
+      element.text = 'bla';
+      assert.equal(element._focused, true);
+      flushAsynchronousOperations();
+      promise.then(() => {
+        assert.equal(element._suggestions.length, 1);
+        assert.equal(queryStub.notCalled, false);
+        done();
+      });
+    });
+
+    test('when not focused', done => {
+      let promise;
+      const queryStub = sandbox.stub()
+          .returns(promise = Promise.resolve([{name: 'suggestion', value: 0}]));
+      element.query = queryStub;
+      element.suggestOnlyWhenFocus = true;
+      element.text = 'bla';
+      assert.equal(element._focused, false);
+      flushAsynchronousOperations();
+      promise.then(() => {
+        assert.equal(element._suggestions.length, 0);
+        done();
+      });
+    });
+
+    test('suggestions should not carry over', done => {
+      let promise;
+      const queryStub = sandbox.stub()
+          .returns(promise = Promise.resolve([{name: 'suggestion', value: 0}]));
+      element.query = queryStub;
+      focusOnInput(element);
+      element.text = 'bla';
+      flushAsynchronousOperations();
+      promise.then(() => {
+        assert.equal(element._suggestions.length, 1);
+        element._updateSuggestions('', 0, false);
+        assert.equal(element._suggestions.length, 0);
+        done();
+      });
+    });
+
     test('multi completes only the last part of the query', done => {
       let promise;
       const queryStub = sandbox.stub()
           .returns(promise = Promise.resolve([{name: 'suggestion', value: 0}]));
       element.query = queryStub;
+      focusOnInput(element);
       element.text = 'blah blah';
       element.multi = true;