Migrate binary diff from legacy diff to lit diff

The support of binary diffs is extremely poor: We are just showing a
diff for the FILE line, so that you can comment on the file as a whole.
Otherwise we are just saying: "There is a difference in binary files."

So there was no point in migrating the diff pixel perfect. Here are
the changes: https://imgur.com/a/b7MxTtn

The pattern for how the image diff builder is implemented and called
is not great, but we use it anyway, because now is not the time to
think about it. And these two builders being consistent is of value.

The primary goal here is to remove the (last) dependency on the
legacy diff builder. Once this change is submitted, we can start
removing it. Yay!

Release-Notes: skip
Google-Bug-Id: b/280018838
Change-Id: I5961a3cd47696077baeffa1698bd34b23588bca6
(cherry picked from commit df87880519ab685d0496031dbd81fe73cf8f0cfc)

6 files changed, 71 insertions, 30 deletions

diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
index 5267f30795..502c6cff66 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-binary.ts
@@ -3,16 +3,17 @@
  * Copyright 2017 Google LLC
  * SPDX-License-Identifier: Apache-2.0
  */
-import {GrDiffBuilderUnified} from './gr-diff-builder-unified';
+import {GrDiffBuilderLit} from './gr-diff-builder-lit';
 import {DiffInfo, DiffPreferencesInfo} from '../../../types/diff';
 import {createElementDiff} from '../gr-diff/gr-diff-utils';
 import {GrDiffGroup} from '../gr-diff/gr-diff-group';
-import {GrDiffLine} from '../gr-diff/gr-diff-line';
-import {GrDiffLineType} from '../../../api/diff';
-import {queryAndAssert} from '../../../utils/common-util';
 import {html, render} from 'lit';
+import {BinaryDiffBuilder} from './gr-diff-builder';
 
-export class GrDiffBuilderBinary extends GrDiffBuilderUnified {
+export class GrDiffBuilderBinary
+  extends GrDiffBuilderLit
+  implements BinaryDiffBuilder
+{
   constructor(
     diff: DiffInfo,
     prefs: DiffPreferencesInfo,
@@ -25,17 +26,21 @@ export class GrDiffBuilderBinary extends GrDiffBuilderUnified {
     const section = createElementDiff('tbody', 'binary-diff');
     // Do not create a diff row for 'LOST'.
     if (group.lines[0].beforeNumber !== 'FILE') return section;
+    return super.buildSectionElement(group);
+  }
 
-    const line = new GrDiffLine(GrDiffLineType.BOTH, 'FILE', 'FILE');
-    const fileRow = this.createRow(line);
-    const contentTd = queryAndAssert<HTMLTableCellElement>(
-      fileRow,
-      'td.both.file'
-    )!;
-    const div = document.createElement('div');
-    render(html`<span>Difference in binary files</span>`, div);
-    contentTd.insertBefore(div, contentTd.firstChild);
-    section.appendChild(fileRow);
-    return section;
+  public renderBinaryDiff() {
+    render(
+      html`
+        <tbody class="gr-diff binary-diff">
+          <tr class="gr-diff">
+            <td colspan="5" class="gr-diff">
+              <span>Difference in binary files</span>
+            </td>
+          </tr>
+        </tbody>
+      `,
+      this.outputEl
+    );
   }
 }
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
index 7fa6d72073..8149d0ed5d 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-element.ts
@@ -12,6 +12,7 @@ import {
   ImageDiffBuilder,
   DiffContextExpandedEventDetail,
   isImageDiffBuilder,
+  isBinaryDiffBuilder,
 } from './gr-diff-builder';
 import {GrDiffBuilderImage} from './gr-diff-builder-image';
 import {GrDiffBuilderBinary} from './gr-diff-builder-binary';
@@ -211,6 +212,8 @@ export class GrDiffBuilderElement implements GroupConsumer {
         .then(async () => {
           if (isImageDiffBuilder(this.builder)) {
             this.builder.renderImageDiff();
+          } else if (isBinaryDiffBuilder(this.builder)) {
+            this.builder.renderBinaryDiff();
           }
           await this.untilGroupsRendered();
           fire(this.diffElement, 'render-content', {});
@@ -437,7 +440,6 @@ export class GrDiffBuilderElement implements GroupConsumer {
         this.useNewImageDiffUi
       );
     } else if (this.diff.binary) {
-      // If the diff is binary, but not an image.
       return new GrDiffBuilderBinary(this.diff, localPrefs, this.diffElement);
     } else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
       this.renderPrefs = {
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
index 2a61ef27ea..c834afc9fb 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder-lit.ts
@@ -136,7 +136,7 @@ export class GrDiffBuilderLit extends GrDiffBuilder {
     };
   }
 
-  protected override buildSectionElement(group: GrDiffGroup) {
+  protected override buildSectionElement(group: GrDiffGroup): HTMLElement {
     const leftCl = `left-${group.startLine(Side.LEFT)}`;
     const rightCl = `right-${group.startLine(Side.RIGHT)}`;
     const section = html`
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
index 5ca51976ad..2581c03a0c 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-builder.ts
@@ -74,6 +74,16 @@ export function isImageDiffBuilder(
   return !!x && !!(x as ImageDiffBuilder).renderImageDiff;
 }
 
+export interface BinaryDiffBuilder extends DiffBuilder {
+  renderBinaryDiff(): void;
+}
+
+export function isBinaryDiffBuilder(
+  x: DiffBuilder | BinaryDiffBuilder | undefined
+): x is BinaryDiffBuilder {
+  return !!x && !!(x as BinaryDiffBuilder).renderBinaryDiff;
+}
+
 /**
  * Base class for different diff builders, like side-by-side, unified etc.
  *
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 1c5d594a67..3d54690585 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -413,11 +413,16 @@ export class GrDiff extends LitElement implements GrDiffApi {
         .image-diff .right.lineNumButton {
           border-left: 1px solid var(--border-color);
         }
-        .image-diff label,
-        .binary-diff label {
+        .image-diff label {
           font-family: var(--font-family);
           font-style: italic;
         }
+        tbody.binary-diff td {
+          font-family: var(--font-family);
+          font-style: italic;
+          text-align: center;
+          padding: var(--spacing-s) 0;
+        }
         .diff-row {
           outline: none;
           user-select: none;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index 227ac2dc5a..f0826ad925 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -3142,18 +3142,25 @@ suite('gr-diff tests', () => {
           element,
           /* HTML */ `
             <div class="diffContainer sideBySide">
+              <gr-diff-section class="left-FILE right-FILE"> </gr-diff-section>
+              <gr-diff-row class="left-FILE right-FILE"> </gr-diff-row>
               <table class="selected-right" id="diffTable">
                 <colgroup>
                   <col class="blame gr-diff" />
-                  <col class="gr-diff" width="48" />
-                  <col class="gr-diff" width="48" />
-                  <col class="gr-diff" />
+                  <col class="gr-diff left" width="48" />
+                  <col class="gr-diff left sign" />
+                  <col class="gr-diff left" />
+                  <col class="gr-diff right" width="48" />
+                  <col class="gr-diff right sign" />
+                  <col class="gr-diff right" />
                 </colgroup>
                 <tbody class="binary-diff gr-diff"></tbody>
-                <tbody class="binary-diff gr-diff">
+                <tbody class="both gr-diff section">
                   <tr
-                    aria-labelledby="left-button-FILE right-button-FILE right-content-FILE"
-                    class="both diff-row gr-diff unified"
+                    aria-labelledby="left-button-FILE left-content-FILE right-button-FILE right-content-FILE"
+                    class="diff-row gr-diff side-by-side"
+                    left-type="both"
+                    right-type="both"
                     tabindex="-1"
                   >
                     <td class="blame gr-diff" data-line-number="FILE"></td>
@@ -3168,6 +3175,14 @@ suite('gr-diff tests', () => {
                         File
                       </button>
                     </td>
+                    <td class="gr-diff left no-intraline-info sign"></td>
+                    <td
+                      class="both content file gr-diff left no-intraline-info"
+                    >
+                      <div class="thread-group" data-side="left">
+                        <slot name="left-FILE"> </slot>
+                      </div>
+                    </td>
                     <td class="gr-diff lineNum right" data-value="FILE">
                       <button
                         aria-label="Add file comment"
@@ -3179,19 +3194,23 @@ suite('gr-diff tests', () => {
                         File
                       </button>
                     </td>
+                    <td class="gr-diff no-intraline-info right sign"></td>
                     <td
                       class="both content file gr-diff no-intraline-info right"
                     >
-                      <div>
-                        <span> Difference in binary files </span>
-                      </div>
                       <div class="thread-group" data-side="right">
                         <slot name="right-FILE"> </slot>
-                        <slot name="left-FILE"> </slot>
                       </div>
                     </td>
                   </tr>
                 </tbody>
+                <tbody class="binary-diff gr-diff">
+                  <tr class="gr-diff">
+                    <td class="gr-diff" colspan="5">
+                      <span> Difference in binary files </span>
+                    </td>
+                  </tr>
+                </tbody>
               </table>
             </div>
           `