Cancel diff rendering when diffs are collapsed or detached

Formerly, if diff rendering is started, but then unloaded or navigated
away from, the rendering work would needlessly continue in the
background. Cancel rendering when a diff will not be used.

Bug: Issue 6061
Change-Id: I695281229335e84f2d5c802c1fa971842c588330

polygerrit-ui/app/behaviors/async-foreach-behavior/async-foreach-behavior.html

@@ -25,12 +25,23 @@ limitations under the License.
     /**
      * @template T
      * @param {!Array<T>} array
-     * @param {!Function} fn
+     * @param {!Function} fn An iteratee function to be passed each element of
+     *     the array in order. Must return a promise, and the following
+     *     iteration will not begin until resolution of the promise returned by
+     *     the previous iteration.
+     *
+     *     An optional second argument to fn is a callback that will halt the
+     *     loop if called.
      * @return {!Promise<undefined>}
      */
     asyncForeach(array, fn) {
       if (!array.length) { return Promise.resolve(); }
-      return fn(array[0]).then(() => this.asyncForeach(array.slice(1), fn));
+      let stop = false;
+      const stopCallback = () => { stop = true; };
+      return fn(array[0], stopCallback).then(exit => {
+        if (stop) { return Promise.resolve(); }
+        return this.asyncForeach(array.slice(1), fn);
+      });
     },
   };
 })(window);

polygerrit-ui/app/behaviors/async-foreach-behavior/async-foreach-behavior_test.html

@@ -35,5 +35,19 @@ limitations under the License.
             assert.equal(fn.getCall(2).args[0], 3);
           });
     });
+
+    test('halts on stop condition', () => {
+      const stub = sinon.stub();
+      const fn = (e, stop) => {
+        stub(e);
+        stop();
+        return Promise.resolve();
+      };
+      return Gerrit.AsyncForeachBehavior.asyncForeach([1, 2, 3], fn)
+          .then(() => {
+            assert.isTrue(stub.calledOnce);
+            assert.equal(stub.lastCall.args[0], 1);
+          });
+    });
   });
 </script>

polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js

@@ -137,6 +137,9 @@
         value: true,
         computed: '_computeShowSizeBars(_userPrefs)',
       },
+
+      /** @type {Function} */
+      _cancelForEachDiff: Function,
     },
 
     behaviors: [
@@ -174,6 +177,10 @@
       keydown: '_scopedKeydownHandler',
     },
 
+    detached() {
+      this._cancelDiffs();
+    },
+
     /**
      * Iron-a11y-keys-behavior catches keyboard events globally. Some keyboard
      * events must be scoped to a component level (e.g. `enter`) in order to not
@@ -849,6 +856,7 @@
 
     _clearCollapsedDiffs(collapsedDiffs) {
       for (const diff of collapsedDiffs) {
+        diff.cancel();
         diff.clearDiffContent();
       }
     },
@@ -871,7 +879,9 @@
       return (new Promise(resolve => {
         this.fire('reload-drafts', {resolve});
       })).then(() => {
-        return this.asyncForeach(paths, path => {
+        return this.asyncForeach(paths, (path, cancel) => {
+          this._cancelForEachDiff = cancel;
+
           iter++;
           console.log('Expanding diff', iter, 'of', initialCount, ':',
               path);
@@ -884,6 +894,7 @@
           }
           return Promise.all(promises);
         }).then(() => {
+          this._cancelForEachDiff = null;
           this._nextRenderParams = null;
           console.log('Finished expanding', initialCount, 'diff(s)');
           this.$.reporting.timeEnd(timerName);
@@ -892,6 +903,12 @@
       });
     },
 
+    /** Cancel the rendering work of every diff in the list */
+    _cancelDiffs() {
+      if (this._cancelForEachDiff) { this._cancelForEachDiff(); }
+      this._forEachDiff(d => d.cancel());
+    },
+
     /**
      * In the given NodeList of diff elements, find the diff for the given path.
      * @param  {string} path

polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html

@@ -851,6 +851,7 @@ limitations under the License.
         reload() {
           done();
         },
+        cancel() {},
       }];
       sinon.stub(element, 'diffs', {
         get() { return diffs; },
@@ -858,6 +859,16 @@ limitations under the License.
       element.push('_expandedFilePaths', path);
     });
 
+    test('_clearCollapsedDiffs', () => {
+      const diff = {
+        cancel: sinon.stub(),
+        clearDiffContent: sinon.stub(),
+      };
+      element._clearCollapsedDiffs([diff]);
+      assert.isTrue(diff.cancel.calledOnce);
+      assert.isTrue(diff.clearDiffContent.calledOnce);
+    });
+
     test('filesExpanded value updates to correct enum', () => {
       element._files = [{__path: 'foo.bar'}, {__path: 'baz.bar'}];
       flushAsynchronousOperations();

polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js

@@ -182,7 +182,7 @@
 
     /** @return {!Promise} */
     reload() {
-      this.$.diffBuilder.cancel();
+      this.cancel();
       this.clearBlame();
       this._safetyBypass = null;
       this._showWarning = false;
@@ -203,6 +203,11 @@
       });
     },
 
+    /** Cancel any remaining diff builder rendering work. */
+    cancel() {
+      this.$.diffBuilder.cancel();
+    },
+
     /** @return {!Array<!HTMLElement>} */
     getCursorStops() {
       if (this.hidden && this.noAutoRender) {

polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html

@@ -49,7 +49,7 @@ limitations under the License.
 
     test('reload cancels before network resolves', () => {
       element = fixture('basic');
-      const cancelStub = sandbox.stub(element.$.diffBuilder, 'cancel');
+      const cancelStub = sandbox.stub(element, 'cancel');
 
       // Stub the network calls into requests that never resolve.
       sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
@@ -58,13 +58,18 @@ limitations under the License.
       assert.isTrue(cancelStub.called);
     });
 
+    test('cancel', () => {
+      const cancelStub = sandbox.stub(element.$.diffBuilder, 'cancel');
+      element.cancel();
+      assert.isTrue(cancelStub.calledOnce);
+    });
+
     test('_diffLength', () => {
       element = fixture('basic');
       const mock = document.createElement('mock-diff-response');
       assert.equal(element._diffLength(mock.diffResponse), 52);
     });
 
-
     suite('_get{PatchNum|IsParentComment}ByLineAndContent', () => {
       let lineEl;
       let contentEl;