Merge "Disable syntax computation on very long diffs"

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

@@ -76,6 +76,9 @@ limitations under the License.
       // syntax highlighting for the entire file.
       const SYNTAX_MAX_LINE_LENGTH = 500;
 
+      // Disable syntax highlighting if the overall diff is too large.
+      const SYNTAX_MAX_DIFF_LENGTH = 20000;
+
       const TRAILING_WHITESPACE_PATTERN = /\s+$/;
 
       Polymer({
@@ -190,7 +193,7 @@ limitations under the License.
                 this.dispatchEvent(new CustomEvent('render-content',
                     {bubbles: true}));
 
-                if (this._anyLineTooLong()) {
+                if (this._diffTooLargeForSyntax()) {
                   this.$.syntaxLayer.enabled = false;
                 }
 
@@ -473,10 +476,32 @@ limitations under the License.
           }, false);
         },
 
+        _diffTooLargeForSyntax() {
+          return this._anyLineTooLong() ||
+              this.getDiffLength() > SYNTAX_MAX_DIFF_LENGTH;
+        },
+
         setBlame(blame) {
           if (!this._builder || !blame) { return; }
           this._builder.setBlame(blame);
         },
+
+        /**
+         * Get the approximate length of the diff as the sum of the maximum
+         * length of the chunks.
+         * @return {number}
+         */
+        getDiffLength() {
+          return this.diff.content.reduce((sum, sec) => {
+            if (sec.hasOwnProperty('ab')) {
+              return sum + sec.ab.length;
+            } else {
+              return sum + Math.max(
+                  sec.hasOwnProperty('a') ? sec.a.length : 0,
+                  sec.hasOwnProperty('b') ? sec.b.length : 0);
+            }
+          }, 0);
+        },
       });
     })();
   </script>

polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html

@@ -1052,6 +1052,10 @@ limitations under the License.
         });
       });
 
+      test('getDiffLength', () => {
+        assert.equal(element.getDiffLength(diff), 52);
+      });
+
       test('getContentByLine', () => {
         let actual;
 

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

@@ -310,7 +310,7 @@ limitations under the License.
     <div id="sizeWarning" class$="[[_computeWarningClass(_showWarning)]]">
       <p>
         Prevented render because "Whole file" is enabled and this diff is very
-        large (about [[_diffLength(diff)]] lines).
+        large (about [[_diffLength]] lines).
       </p>
       <gr-button on-tap="_handleLimitedBypass">
         Render with limited context

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

@@ -195,6 +195,8 @@
           return this._createCommentThreadGroup.bind(this);
         },
       },
+
+      _diffLength: Number,
     },
 
     behaviors: [
@@ -624,6 +626,7 @@
 
     _diffChanged(newValue) {
       if (newValue) {
+        this._diffLength = this.$.diffBuilder.getDiffLength();
         this._renderDiffTable();
       }
     },
@@ -634,7 +637,7 @@
         return;
       }
       if (this.prefs.context === -1 &&
-          this._diffLength(this.diff) >= LARGE_DIFF_THRESHOLD_LINES &&
+          this._diffLength >= LARGE_DIFF_THRESHOLD_LINES &&
           this._safetyBypass === null) {
         this._showWarning = true;
         this.dispatchEvent(new CustomEvent('render', {bubbles: true}));
@@ -684,25 +687,6 @@
       return items.length === 0;
     },
 
-    /**
-     * The number of lines in the diff. For delta chunks that are different
-     * sizes on the left and the right, the longer side is used.
-     * @param {!Object} diff
-     * @return {number}
-     */
-    _diffLength(diff) {
-      return diff.content.reduce((sum, sec) => {
-        if (sec.hasOwnProperty('ab')) {
-          return sum + sec.ab.length;
-        } else {
-          return sum + Math.max(
-              sec.hasOwnProperty('a') ? sec.a.length : 0,
-              sec.hasOwnProperty('b') ? sec.b.length : 0
-          );
-        }
-      }, 0);
-    },
-
     _handleFullBypass() {
       this._safetyBypass = FULL_CONTEXT;
       this._renderDiffTable();

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

@@ -55,12 +55,6 @@ limitations under the License.
       assert.isTrue(cancelStub.calledOnce);
     });
 
-    test('_diffLength', () => {
-      element = fixture('basic');
-      const mock = document.createElement('mock-diff-response');
-      assert.equal(element._diffLength(mock.diffResponse), 52);
-    });
-
     test('line limit with line_wrapping', () => {
       element = fixture('basic');
       element.prefs = {line_wrapping: true, line_length: 80, tab_size: 2};
@@ -971,6 +965,7 @@ limitations under the License.
                   new CustomEvent('render', {bubbles: true}));
             });
         const mock = document.createElement('mock-diff-response');
+        sandbox.stub(element.$.diffBuilder, 'getDiffLength').returns(10000);
         element.diff = mock.diffResponse;
         element.comments = {left: [], right: []};
         element.noRenderOnPrefsChange = true;
@@ -978,7 +973,6 @@ limitations under the License.
 
       test('large render w/ context = 10', done => {
         element.prefs = {context: 10};
-        sandbox.stub(element, '_diffLength', () => 10000);
         element.addEventListener('render', () => {
           assert.isTrue(renderStub.called);
           assert.isFalse(element._showWarning);
@@ -990,7 +984,6 @@ limitations under the License.
       test('large render w/ whole file and bypass', done => {
         element.prefs = {context: -1};
         element._safetyBypass = 10;
-        sandbox.stub(element, '_diffLength', () => 10000);
         element.addEventListener('render', () => {
           assert.isTrue(renderStub.called);
           assert.isFalse(element._showWarning);
@@ -1001,7 +994,6 @@ limitations under the License.
 
       test('large render w/ whole file and no bypass', done => {
         element.prefs = {context: -1};
-        sandbox.stub(element, '_diffLength', () => 10000);
         element.addEventListener('render', () => {
           assert.isFalse(renderStub.called);
           assert.isTrue(element._showWarning);