Asynchronous diff rendering

Building on existing support for asynchronous diff processing, the
rendering stage is now also asynchronous. The `emitGroup` method of
gr-diff-builder (which constructs a DOM fragment and attaches it to the
document) is now called whenever the processor emits a new group, rather
than waiting for all groups to be available and looping over them.

Adds support for canceling the processor, and removes a behavior where
the diff was being re-rendered needlessly, causing visible flicker.
Updates a test that broke when rendering became asynchronous.

Change-Id: I37fcd65efc8c901b85fc93e91611c022edc10dc4
This commit is contained in:
Wyatt Allen
2016-06-28 16:40:46 -07:00
parent 8e20db3119
commit db881fa0ad
5 changed files with 99 additions and 49 deletions

View File

@@ -21,7 +21,9 @@ limitations under the License.
<div class="contentWrapper"> <div class="contentWrapper">
<content></content> <content></content>
</div> </div>
<gr-diff-processor id="processor"></gr-diff-processor> <gr-diff-processor
id="processor"
groups="{{_groups}}"></gr-diff-processor>
</template> </template>
<script src="../gr-diff/gr-diff-line.js"></script> <script src="../gr-diff/gr-diff-line.js"></script>
<script src="../gr-diff/gr-diff-group.js"></script> <script src="../gr-diff/gr-diff-group.js"></script>
@@ -53,19 +55,34 @@ limitations under the License.
baseImage: Object, baseImage: Object,
revisionImage: Object, revisionImage: Object,
_builder: Object, _builder: Object,
_groups: Array,
}, },
get diffElement() { get diffElement() {
return this.queryEffectiveChildren('#diffTable'); return this.queryEffectiveChildren('#diffTable');
}, },
observers: [
'_groupsChanged(_groups.splices)',
],
render: function(diff, comments, prefs) { render: function(diff, comments, prefs) {
// Stop the processor (if it's running).
this.$.processor.cancel();
this._builder = this._getDiffBuilder(diff, comments, prefs); this._builder = this._getDiffBuilder(diff, comments, prefs);
this.$.processor.context = prefs.context; this.$.processor.context = prefs.context;
this.$.processor.keyLocations = this._getCommentLocations(comments); this.$.processor.keyLocations = this._getCommentLocations(comments);
this.$.processor.process(diff.content)
.then(this._renderDiff.bind(this)); this._clearDiffContent();
this.$.processor.process(diff.content).then(function() {
if (this.isImageDiff) {
this._builder.renderDiffImages();
}
this.fire('render');
}.bind(this));
}, },
getLineElByChild: function(node) { getLineElByChild: function(node) {
@@ -177,10 +194,6 @@ limitations under the License.
this._builder.emitGroup(group, sectionEl); this._builder.emitGroup(group, sectionEl);
}, },
emitDiff: function() {
this._builder.emitDiff();
},
showContext: function(newGroups, sectionEl) { showContext: function(newGroups, sectionEl) {
var groups = this._builder.groups; var groups = this._builder.groups;
// TODO(viktard): Polyfill findIndex for IE10. // TODO(viktard): Polyfill findIndex for IE10.
@@ -213,19 +226,6 @@ limitations under the License.
throw Error('Unsupported diff view mode: ' + this.viewMode); throw Error('Unsupported diff view mode: ' + this.viewMode);
}, },
_renderDiff: function(groups) {
this._builder.groups = groups;
this._clearDiffContent();
this.emitDiff();
if (this.isImageDiff) {
this._builder.renderDiffImages();
}
this.async(function() {
this.fire('render');
}, 1);
},
_clearDiffContent: function() { _clearDiffContent: function() {
this.diffElement.innerHTML = null; this.diffElement.innerHTML = null;
}, },
@@ -246,6 +246,18 @@ limitations under the License.
} }
return result; return result;
}, },
_groupsChanged: function(changeRecord) {
if (!changeRecord) { return; }
changeRecord.indexSplices.forEach(function(splice) {
var group;
for (var i = 0; i < splice.addedCount; i++) {
group = splice.object[splice.index + i];
this._builder.groups.push(group);
this._builder.emitGroup(group);
}
}, this);
},
}); });
})(); })();
</script> </script>

View File

@@ -59,12 +59,6 @@
var PARTIAL_CONTEXT_AMOUNT = 10; var PARTIAL_CONTEXT_AMOUNT = 10;
GrDiffBuilder.prototype.emitDiff = function() {
for (var i = 0; i < this.groups.length; i++) {
this.emitGroup(this.groups[i]);
}
};
GrDiffBuilder.prototype.buildSectionElement = function(group) { GrDiffBuilder.prototype.buildSectionElement = function(group) {
throw Error('Subclasses must implement buildGroupElement'); throw Error('Subclasses must implement buildGroupElement');
}; };

View File

@@ -98,10 +98,20 @@ limitations under the License.
assert.equal(cursorElement.diffRow, firstDeltaRow); assert.equal(cursorElement.diffRow, firstDeltaRow);
}); });
test('diff cursor functionality (unified)', function() { suite('unified diff', function() {
diffElement.viewMode = 'UNIFIED_DIFF';
cursorElement.reInitCursor();
setup(function(done) {
// We must allow the diff to re-render after setting the viewMode.
var renderHandler = function() {
diffElement.removeEventListener('render', renderHandler);
cursorElement.reInitCursor();
done();
};
diffElement.addEventListener('render', renderHandler);
diffElement.viewMode = 'UNIFIED_DIFF';
});
test('diff cursor functionality (unified)', function() {
// The cursor has been initialized to the first delta. // The cursor has been initialized to the first delta.
assert.isOk(cursorElement.diffRow); assert.isOk(cursorElement.diffRow);
@@ -121,6 +131,7 @@ limitations under the License.
assert.notEqual(cursorElement.diffRow, firstDeltaRow.nextSibling); assert.notEqual(cursorElement.diffRow, firstDeltaRow.nextSibling);
assert.equal(cursorElement.diffRow, firstDeltaRow); assert.equal(cursorElement.diffRow, firstDeltaRow);
}); });
});
test('cursor side functionality', function() { test('cursor side functionality', function() {
// The side only applies to side-by-side mode, which should be the default // The side only applies to side-by-side mode, which should be the default

View File

@@ -58,6 +58,8 @@
type: Object, type: Object,
value: function() { return {left: {}, right: {}}; }, value: function() { return {left: {}, right: {}}; },
}, },
_nextStepHandle: Number,
}, },
/** /**
@@ -82,6 +84,7 @@
// If we are done, resolve the promise. // If we are done, resolve the promise.
if (state.sectionIndex >= content.length) { if (state.sectionIndex >= content.length) {
resolve(this.groups); resolve(this.groups);
this._nextStepHandle = undefined;
return; return;
} }
@@ -95,13 +98,23 @@
// Increment the index and recurse. // Increment the index and recurse.
state.sectionIndex++; state.sectionIndex++;
this.async(nextStep, 1); this._nextStepHandle = this.async(nextStep, 1);
}; };
nextStep.call(this); nextStep.call(this);
}.bind(this)); }.bind(this));
}, },
/**
* Cancel any jobs that are running.
*/
cancel: function() {
if (this._nextStepHandle !== undefined) {
this.cancelAsync(this._nextStepHandle);
this._nextStepHandle = undefined;
}
},
/** /**
* Process the next section of the diff. * Process the next section of the diff.
*/ */

View File

@@ -36,7 +36,10 @@
changeNum: String, changeNum: String,
patchRange: Object, patchRange: Object,
path: String, path: String,
prefs: Object, prefs: {
type: Object,
observer: '_prefsObserver',
},
projectConfig: { projectConfig: {
type: Object, type: Object,
observer: '_projectConfigChanged', observer: '_projectConfigChanged',
@@ -57,6 +60,7 @@
viewMode: { viewMode: {
type: String, type: String,
value: DiffViewMode.SIDE_BY_SIDE, value: DiffViewMode.SIDE_BY_SIDE,
observer: '_viewModeObserver',
}, },
_diff: Object, _diff: Object,
_comments: Object, _comments: Object,
@@ -64,10 +68,6 @@
_revisionImage: Object, _revisionImage: Object,
}, },
observers: [
'_prefsChanged(prefs.*, viewMode)',
],
listeners: { listeners: {
'thread-discard': '_handleThreadDiscard', 'thread-discard': '_handleThreadDiscard',
'comment-discard': '_handleCommentDiscard', 'comment-discard': '_handleCommentDiscard',
@@ -303,8 +303,28 @@
}); });
}, },
_prefsChanged: function(prefsChangeRecord) { _prefsObserver: function(newPrefs, oldPrefs) {
var prefs = prefsChangeRecord.base; // Scan the preference objects one level deep to see if they differ.
var differ = !oldPrefs;
if (newPrefs && oldPrefs) {
for (var key in newPrefs) {
if (newPrefs[key] !== oldPrefs[key]) {
differ = true;
}
}
}
if (differ) {
this._prefsChanged(newPrefs);
}
},
_viewModeObserver: function() {
this._prefsChanged(this.prefs);
},
_prefsChanged: function(prefs) {
if (!prefs) { return; }
this.customStyle['--content-width'] = prefs.line_length + 'ch'; this.customStyle['--content-width'] = prefs.line_length + 'ch';
this.updateStyles(); this.updateStyles();