Debounce rerendering after inputs change

The current implementation starts a render synchronously whenever one of the
inputs change. Often, multiple inputs change together, and the current
behavior causes unncessary rerenders (and cancels half way through).

This change instead schedules the rerendering asynchronously on the next
microtask, and debounces them so that multiple renders collapse into
one.

Change-Id: Id5e823a53ecfa9c542d81835bf67556dcab14d4a
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 8428da3..83e5216 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -1410,6 +1410,7 @@
         ignore_whitespace: 'IGNORE_NONE',
       };
       diff._diff = mock.diffResponse;
+      diff.$.diff.flushDebouncer('renderDiffTable');
     };
 
     const renderAndGetNewDiffs = function(index) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 5ecc2fc3..1c018de 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -102,6 +102,8 @@
    */
   const COMMIT_MSG_LINE_LENGTH = 72;
 
+  const RENDER_DIFF_TABLE_DEBOUNCE_NAME = 'renderDiffTable';
+
   Polymer({
     is: 'gr-diff',
 
@@ -392,6 +394,7 @@
     /** Cancel any remaining diff builder rendering work. */
     cancel() {
       this.$.diffBuilder.cancel();
+      this.cancelDebouncer(RENDER_DIFF_TABLE_DEBOUNCE_NAME);
     },
 
     /** @return {!Array<!HTMLElement>} */
@@ -679,17 +682,32 @@
       this.updateStyles(stylesToUpdate);
 
       if (this.diff && !this.noRenderOnPrefsChange) {
-        this._renderDiffTable();
+        this._debounceRenderDiffTable();
       }
     },
 
     _diffChanged(newValue) {
       if (newValue) {
         this._diffLength = this.$.diffBuilder.getDiffLength();
-        this._renderDiffTable();
+        this._debounceRenderDiffTable();
       }
     },
 
+    /**
+     * When called multiple times from the same microtask, will call
+     * _renderDiffTable only once, in the next microtask, unless it is cancelled
+     * before that microtask runs.
+     *
+     * This should be used instead of calling _renderDiffTable directly to
+     * render the diff in response to an input change, because there may be
+     * multiple inputs changing in the same microtask, but we only want to
+     * render once.
+     */
+    _debounceRenderDiffTable() {
+      this.debounce(
+          RENDER_DIFF_TABLE_DEBOUNCE_NAME, () => this._renderDiffTable());
+    },
+
     _renderDiffTable() {
       this._unobserveIncrementalNodes();
       if (!this.prefs) {
@@ -791,12 +809,12 @@
 
     _handleFullBypass() {
       this._safetyBypass = FULL_CONTEXT;
-      this._renderDiffTable();
+      this._debounceRenderDiffTable();
     },
 
     _handleLimitedBypass() {
       this._safetyBypass = LIMITED_CONTEXT;
-      this._renderDiffTable();
+      this._debounceRenderDiffTable();
     },
 
     /** @return {string} */
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 849ad0d..42f098b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -40,6 +40,8 @@
     let element;
     let sandbox;
 
+    const MINIMAL_PREFS = {tab_size: 2, line_length: 80};
+
     setup(() => {
       sandbox = sinon.sandbox.create();
     });
@@ -81,14 +83,14 @@
 
     test('line limit with line_wrapping', () => {
       element = fixture('basic');
-      element.prefs = {line_wrapping: true, line_length: 80, tab_size: 2};
+      element.prefs = Object.assign({}, MINIMAL_PREFS, {line_wrapping: true});
       flushAsynchronousOperations();
       assert.equal(element.customStyle['--line-limit'], '80ch');
     });
 
     test('line limit without line_wrapping', () => {
       element = fixture('basic');
-      element.prefs = {line_wrapping: false, line_length: 80, tab_size: 2};
+      element.prefs = Object.assign({}, MINIMAL_PREFS, {line_wrapping: false});
       flushAsynchronousOperations();
       assert.isNotOk(element.customStyle['--line-limit']);
     });
@@ -225,7 +227,7 @@
 
         const mock = document.createElement('mock-diff-response');
         element.$.diffBuilder._builder = element.$.diffBuilder._getDiffBuilder(
-            mock.diffResponse, {tab_size: 2, line_length: 80});
+            mock.diffResponse, Object.assign({}, MINIMAL_PREFS));
 
         // No thread groups.
         assert.isNotOk(element._getThreadGroupForLine(contentEl));
@@ -448,7 +450,7 @@
             binary: true,
           };
 
-          element.addEventListener('render', () => {
+          function rendered() {
             // Recognizes that it should be an image diff.
             assert.isTrue(element.isImageDiff);
             assert.instanceOf(
@@ -460,7 +462,9 @@
             assert.isNotOk(leftImage);
             assert.isOk(rightImage);
             done();
-          });
+            element.removeEventListener('render', rendered);
+          }
+          element.addEventListener('render', rendered);
 
           element.revisionImage = mockFile2;
           element.diff = mockDiff;
@@ -483,7 +487,7 @@
             binary: true,
           };
 
-          element.addEventListener('render', () => {
+          function rendered() {
             // Recognizes that it should be an image diff.
             assert.isTrue(element.isImageDiff);
             assert.instanceOf(
@@ -495,7 +499,9 @@
             assert.isOk(leftImage);
             assert.isNotOk(rightImage);
             done();
-          });
+            element.removeEventListener('render', rendered);
+          }
+          element.addEventListener('render', rendered);
 
           element.baseImage = mockFile1;
           element.diff = mockDiff;
@@ -519,7 +525,7 @@
           };
           mockFile1.type = 'image/jpeg-evil';
 
-          element.addEventListener('render', () => {
+          function rendered() {
             // Recognizes that it should be an image diff.
             assert.isTrue(element.isImageDiff);
             assert.instanceOf(
@@ -527,7 +533,9 @@
             const leftImage = element.$.diffTable.querySelector('td.left img');
             assert.isNotOk(leftImage);
             done();
-          });
+            element.removeEventListener('render', rendered);
+          }
+          element.addEventListener('render', rendered);
 
           element.baseImage = mockFile1;
           element.diff = mockDiff;
@@ -679,12 +687,14 @@
             change_type: 'MODIFIED',
             content: [{skip: 66}],
           };
+          element.flushDebouncer('renderDiffTable');
         });
 
         test('change in preferences re-renders diff', () => {
           sandbox.stub(element, '_renderDiffTable');
-          element.prefs = {};
-          element.prefs = {time_format: 'HHMM_12'};
+          element.prefs = Object.assign(
+              {}, MINIMAL_PREFS, {time_format: 'HHMM_12'});
+          element.flushDebouncer('renderDiffTable');
           assert.isTrue(element._renderDiffTable.called);
         });
 
@@ -692,8 +702,9 @@
             'noRenderOnPrefsChange', () => {
           sandbox.stub(element, '_renderDiffTable');
           element.noRenderOnPrefsChange = true;
-          element.prefs = {};
-          element.prefs = {time_format: 'HHMM_12'};
+          element.prefs = Object.assign(
+              {}, MINIMAL_PREFS, {time_format: 'HHMM_12'});
+          element.flushDebouncer('renderDiffTable');
           assert.isFalse(element._renderDiffTable.called);
         });
       });
@@ -760,33 +771,39 @@
       });
 
       test('large render w/ context = 10', done => {
-        element.prefs = {context: 10};
-        element.addEventListener('render', () => {
+        element.prefs = Object.assign({}, MINIMAL_PREFS, {context: 10});
+        function rendered() {
           assert.isTrue(renderStub.called);
           assert.isFalse(element._showWarning);
           done();
-        });
+          element.removeEventListener('render', rendered);
+        }
+        element.addEventListener('render', rendered);
         element._renderDiffTable();
       });
 
       test('large render w/ whole file and bypass', done => {
-        element.prefs = {context: -1};
+        element.prefs = Object.assign({}, MINIMAL_PREFS, {context: -1});
         element._safetyBypass = 10;
-        element.addEventListener('render', () => {
+        function rendered() {
           assert.isTrue(renderStub.called);
           assert.isFalse(element._showWarning);
           done();
-        });
+          element.removeEventListener('render', rendered);
+        }
+        element.addEventListener('render', rendered);
         element._renderDiffTable();
       });
 
       test('large render w/ whole file and no bypass', done => {
-        element.prefs = {context: -1};
-        element.addEventListener('render', () => {
+        element.prefs = Object.assign({}, MINIMAL_PREFS, {context: -1});
+        function rendered() {
           assert.isFalse(renderStub.called);
           assert.isTrue(element._showWarning);
           done();
-        });
+          element.removeEventListener('render', rendered);
+        }
+        element.addEventListener('render', rendered);
         element._renderDiffTable();
       });
     });