Merge changes from topic "plugin-get-change-fields"
* changes:
Move plugin field tests into respective packages
Split up plugin fields tests into extension/REST/SSH files
Bind ChangeAttributeFactory as a DynamicSet
Support plugin-defined options in extension API
Move DynamicMap<DynamicBean> binding into sys injector
PluginFieldsIT: Actually use extracted SSH_GSON constant
ChangeQueryProcessor: Don't implement PluginAttributesFactory
Support plugin fields in GetChange
Extract ChangeAttributeFactory from ChangeQueryProcessor
Add tests for plugin-provided fields controlled by options
CmdLineParser: Require plugin option names to start with '-'
Expand tests for plugin fields
dev-plugins.txt: Recommend ServletModule instead of HttpPluginModule
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 4dea01d..a994711 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2585,30 +2585,30 @@
)]}'
{
- "commit":{
- "parents":[
+ "commit": {
+ "parents": [
{
- "commit":"1eee2c9d8f352483781e772f35dc586a69ff5646",
+ "commit": "1eee2c9d8f352483781e772f35dc586a69ff5646",
}
],
- "author":{
- "name":"Shawn O. Pearce",
- "email":"sop@google.com",
- "date":"2012-04-24 18:08:08.000000000",
- "tz":-420
+ "author": {
+ "name": "Shawn O. Pearce",
+ "email": "sop@google.com",
+ "date": "2012-04-24 18:08:08.000000000",
+ "tz": -420
},
- "committer":{
- "name":"Shawn O. Pearce",
- "email":"sop@google.com",
- "date":"2012-04-24 18:08:08.000000000",
- "tz":-420
+ "committer": {
+ "name": "Shawn O. Pearce",
+ "email": "sop@google.com",
+ "date": "2012-04-24 18:08:08.000000000",
+ "tz": -420
},
- "subject":"Use an EventBus to manage star icons",
- "message":"Use an EventBus to manage star icons\n\nImage widgets that need to ..."
+ "subject": "Use an EventBus to manage star icons",
+ "message": "Use an EventBus to manage star icons\n\nImage widgets that need to ..."
},
- "base_patch_set_number":1,
- "base_revision":"c35558e0925e6985c91f3a16921537d5e572b7a3",
- "ref":"refs/users/01/1000001/edit-76482/1"
+ "base_patch_set_number": 1,
+ "base_revision": "c35558e0925e6985c91f3a16921537d5e572b7a3",
+ "ref": "refs/users/01/1000001/edit-76482/1"
}
----
@@ -2796,7 +2796,7 @@
)]}'
{
- "web_links":[
+ "web_links": [
{
"show_on_side_by_side_diff_view": true,
"name": "side-by-side preview diff",
@@ -4819,30 +4819,30 @@
)]}'
{
- "commit":{
- "parents":[
+ "commit": {
+ "parents": [
{
- "commit":"1eee2c9d8f352483781e772f35dc586a69ff5646",
+ "commit": "1eee2c9d8f352483781e772f35dc586a69ff5646",
}
],
- "author":{
- "name":"John Doe",
- "email":"john.doe@example.com",
- "date":"2013-05-07 15:21:27.000000000",
- "tz":120
+ "author": {
+ "name": "John Doe",
+ "email": "john.doe@example.com",
+ "date": "2013-05-07 15:21:27.000000000",
+ "tz": 120
},
- "committer":{
- "name":"Jane Doe",
- "email":"jane.doe@example.com",
- "date":"2013-05-07 15:35:43.000000000",
- "tz":120
+ "committer": {
+ "name": "Jane Doe",
+ "email": "jane.doe@example.com",
+ "date": "2013-05-07 15:35:43.000000000",
+ "tz": 120
},
- "subject":"Implement feature X",
- "message":"Implement feature X\n\nWith this feature ..."
+ "subject": "Implement feature X",
+ "message": "Implement feature X\n\nWith this feature ..."
},
- "base_patch_set_number":1,
- "base_revision":"674ac754f91e64a0efb8087e59a176484bd534d1"
- "ref":"refs/users/01/1000001/edit-42622/1"
+ "base_patch_set_number": 1,
+ "base_revision": "674ac754f91e64a0efb8087e59a176484bd534d1"
+ "ref": "refs/users/01/1000001/edit-42622/1"
}
----
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 9803c19..f69c4ae 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -288,11 +288,11 @@
},
"child-project": {
"id": "child-project",
- "parent":"parent-project"
+ "parent": "parent-project"
},
"parent-project": {
"id": "parent-project",
- "parent":"All-Projects"
+ "parent": "All-Projects"
}
}
----
@@ -1271,14 +1271,14 @@
Content-Type: application/json; charset=UTF-8
{
- "add":{
- "refs/heads/*":{
- "permissions":{
- "read":{
- "rules":{
+ "add": {
+ "refs/heads/*": {
+ "permissions": {
+ "read": {
+ "rules": {
"global:Anonymous-Users": {
- "action":"DENY",
- "force":false
+ "action": "DENY",
+ "force": false
}
}
}
diff --git a/java/com/google/gerrit/server/change/RevisionResource.java b/java/com/google/gerrit/server/change/RevisionResource.java
index deb5022..caafe24 100644
--- a/java/com/google/gerrit/server/change/RevisionResource.java
+++ b/java/com/google/gerrit/server/change/RevisionResource.java
@@ -34,7 +34,7 @@
public static final TypeLiteral<RestView<RevisionResource>> REVISION_KIND =
new TypeLiteral<RestView<RevisionResource>>() {};
- public static RevisionResource createNonCachable(ChangeResource change, PatchSet ps) {
+ public static RevisionResource createNonCacheable(ChangeResource change, PatchSet ps) {
return new RevisionResource(change, ps, Optional.empty(), false);
}
@@ -52,11 +52,11 @@
}
private RevisionResource(
- ChangeResource change, PatchSet ps, Optional<ChangeEdit> edit, boolean cachable) {
+ ChangeResource change, PatchSet ps, Optional<ChangeEdit> edit, boolean cacheable) {
this.change = change;
this.ps = ps;
this.edit = edit;
- this.cacheable = cachable;
+ this.cacheable = cacheable;
}
public boolean isCacheable() {
diff --git a/java/com/google/gerrit/server/config/ConfigResource.java b/java/com/google/gerrit/server/config/ConfigResource.java
index 4275dc4..f2b7c8e 100644
--- a/java/com/google/gerrit/server/config/ConfigResource.java
+++ b/java/com/google/gerrit/server/config/ConfigResource.java
@@ -26,7 +26,7 @@
/**
* Default cache control that gets set on the 'Cache-Control' header for responses on this
- * resource that are cachable.
+ * resource that are cacheable.
*
* <p>Not all resources are cacheable and in fact the vast majority might not be. Caching is a
* trade-off between the freshness of data and the number of QPS that the web UI sends.
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index 1f3f001..e0eb3f2 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -179,7 +179,7 @@
CREATE_REPLACE,
NORMAL,
AUTOCLOSE,
- };
+ }
@Singleton
private static class Metrics {
diff --git a/java/com/google/gerrit/server/restapi/change/Revisions.java b/java/com/google/gerrit/server/restapi/change/Revisions.java
index 4edd741..84257e9 100644
--- a/java/com/google/gerrit/server/restapi/change/Revisions.java
+++ b/java/com/google/gerrit/server/restapi/change/Revisions.java
@@ -82,7 +82,7 @@
if (id.get().equals("current")) {
PatchSet ps = psUtil.current(change.getNotes());
if (ps != null && visible(change)) {
- return RevisionResource.createNonCachable(change, ps);
+ return RevisionResource.createNonCacheable(change, ps);
}
throw new ResourceNotFoundException(id);
}
diff --git a/plugins/delete-project b/plugins/delete-project
index 1304fb0..53311c0 160000
--- a/plugins/delete-project
+++ b/plugins/delete-project
@@ -1 +1 @@
-Subproject commit 1304fb030ea990d63df42cab678cdf2b46722d1b
+Subproject commit 53311c006635e83faaa7a1b68039edebe0fe8e43
diff --git a/plugins/gitiles b/plugins/gitiles
index 623105f..dcd8d7f 160000
--- a/plugins/gitiles
+++ b/plugins/gitiles
@@ -1 +1 @@
-Subproject commit 623105f14dca02cb294ed94a952f5e8ce0e96683
+Subproject commit dcd8d7f93ebe21e60688e65fa1261a836a82eb32
diff --git a/plugins/webhooks b/plugins/webhooks
index b8fcfd5..064ae07 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit b8fcfd5d712abb599e49c3e40d266b77f05a398a
+Subproject commit 064ae07f55710034062ef2a36cd030abeee93265
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/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index cca5635..59af36d 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -234,10 +234,8 @@
* User-perceived app start time, should be reported when the app is ready.
*/
appStarted(hidden) {
- const startTime =
- new Date().getTime() - this.performanceTiming.navigationStart;
this.reporter(TIMING.TYPE, TIMING.CATEGORY_UI_LATENCY,
- TIMING.APP_STARTED, startTime);
+ TIMING.APP_STARTED, this.now());
if (hidden) {
this.reporter(PAGE_VISIBILITY.TYPE, PAGE_VISIBILITY.CATEGORY,
PAGE_VISIBILITY.STARTED_HIDDEN);
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
index 8b85074..2087790 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
@@ -61,11 +61,11 @@
});
test('appStarted', () => {
+ sandbox.stub(element, 'now').returns(42);
element.appStarted(true);
assert.isTrue(
element.reporter.calledWithExactly(
- 'timing-report', 'UI Latency', 'App Started',
- NOW_TIME - fakePerformance.navigationStart
+ 'timing-report', 'UI Latency', 'App Started', 42
));
assert.isTrue(
element.reporter.calledWithExactly(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 29cc3b9..9646b06 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -38,6 +38,7 @@
<gr-reporting id="reporting"></gr-reporting>
<gr-js-api-interface id="jsAPI"></gr-js-api-interface>
</template>
+ <script src="../../../scripts/util.js"></script>
<script src="../gr-diff/gr-diff-line.js"></script>
<script src="../gr-diff/gr-diff-group.js"></script>
<script src="../gr-diff-highlight/gr-annotation.js"></script>
@@ -108,6 +109,13 @@
type: Array,
value: () => [],
},
+ /**
+ * The promise last returned from `render()` while the asynchronous
+ * rendering is running - `null` otherwise. Provides a `cancel()`
+ * method that rejects it with `{isCancelled: true}`.
+ * @type {?Object}
+ */
+ _cancelableRenderPromise: Object,
},
get diffElement() {
@@ -146,25 +154,32 @@
reporting.time(TimingLabel.TOTAL);
reporting.time(TimingLabel.CONTENT);
this.dispatchEvent(new CustomEvent('render-start', {bubbles: true}));
- return this.$.processor.process(this.diff.content, isBinary)
- .then(() => {
- if (this.isImageDiff) {
- this._builder.renderDiff();
- }
- this.dispatchEvent(new CustomEvent('render-content',
- {bubbles: true}));
+ this._cancelableRenderPromise = util.makeCancelable(
+ this.$.processor.process(this.diff.content, isBinary)
+ .then(() => {
+ if (this.isImageDiff) {
+ this._builder.renderDiff();
+ }
+ this.dispatchEvent(new CustomEvent('render-content',
+ {bubbles: true}));
- if (this._diffTooLargeForSyntax()) {
- this.$.syntaxLayer.enabled = false;
- }
+ if (this._diffTooLargeForSyntax()) {
+ this.$.syntaxLayer.enabled = false;
+ }
- reporting.timeEnd(TimingLabel.CONTENT);
- reporting.time(TimingLabel.SYNTAX);
- return this.$.syntaxLayer.process().then(() => {
- reporting.timeEnd(TimingLabel.SYNTAX);
- reporting.timeEnd(TimingLabel.TOTAL);
- });
- });
+ reporting.timeEnd(TimingLabel.CONTENT);
+ reporting.time(TimingLabel.SYNTAX);
+ return this.$.syntaxLayer.process().then(() => {
+ reporting.timeEnd(TimingLabel.SYNTAX);
+ reporting.timeEnd(TimingLabel.TOTAL);
+ });
+ }));
+ return this._cancelableRenderPromise
+ .finally(() => { this._cancelableRenderPromise = null; })
+ // Mocca testing does not like uncaught rejections, so we catch
+ // the cancels which are expected and should not throw errors in
+ // tests.
+ .catch(e => { if (!e.isCanceled) return Promise.reject(e); });
},
_setupAnnotationLayers() {
@@ -259,6 +274,10 @@
cancel() {
this.$.processor.cancel();
this.$.syntaxLayer.cancel();
+ if (this._cancelableRenderPromise) {
+ this._cancelableRenderPromise.cancel();
+ this._cancelableRenderPromise = null;
+ }
},
_handlePreferenceError(pref) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.html b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.html
index baa8bba..663cf25 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.html
@@ -20,5 +20,7 @@
<dom-module id="gr-diff-processor">
<script src="../gr-diff/gr-diff-line.js"></script>
<script src="../gr-diff/gr-diff-group.js"></script>
+
+ <script src="../../../scripts/util.js"></script>
<script src="gr-diff-processor.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
index dead8d7..a9268a7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
@@ -80,8 +80,18 @@
value: 64,
},
- /** @type {number|undefined} */
+ /** @type {?number} */
_nextStepHandle: Number,
+ /**
+ * The promise last returned from `process()` while the asynchronous
+ * processing is running - `null` otherwise. Provides a `cancel()`
+ * method that rejects it with `{isCancelled: true}`.
+ * @type {?Object}
+ */
+ _processPromise: {
+ type: Object,
+ value: null,
+ },
_isScrolling: Boolean,
},
@@ -108,6 +118,10 @@
* processed.
*/
process(content, isBinary) {
+ // Cancel any still running process() calls, because they append to the
+ // same groups field.
+ this.cancel();
+
this.groups = [];
this.push('groups', this._makeFileComments());
@@ -115,57 +129,64 @@
// so finish processing.
if (isBinary) { return Promise.resolve(); }
- return new Promise(resolve => {
- const state = {
- lineNums: {left: 0, right: 0},
- sectionIndex: 0,
- };
- content = this._splitCommonGroupsWithComments(content);
+ this._processPromise = util.makeCancelable(
+ new Promise(resolve => {
+ const state = {
+ lineNums: {left: 0, right: 0},
+ sectionIndex: 0,
+ };
- let currentBatch = 0;
- const nextStep = () => {
- if (this._isScrolling) {
- this.async(nextStep, 100);
- return;
- }
- // If we are done, resolve the promise.
- if (state.sectionIndex >= content.length) {
- resolve(this.groups);
- this._nextStepHandle = undefined;
- return;
- }
+ content = this._splitCommonGroupsWithComments(content);
- // Process the next section and incorporate the result.
- const result = this._processNext(state, content);
- for (const group of result.groups) {
- this.push('groups', group);
- currentBatch += group.lines.length;
- }
- state.lineNums.left += result.lineDelta.left;
- state.lineNums.right += result.lineDelta.right;
+ let currentBatch = 0;
+ const nextStep = () => {
+ if (this._isScrolling) {
+ this.async(nextStep, 100);
+ return;
+ }
+ // If we are done, resolve the promise.
+ if (state.sectionIndex >= content.length) {
+ resolve(this.groups);
+ this._nextStepHandle = null;
+ return;
+ }
- // Increment the index and recurse.
- state.sectionIndex++;
- if (currentBatch >= this._asyncThreshold) {
- currentBatch = 0;
- this._nextStepHandle = this.async(nextStep, 1);
- } else {
+ // Process the next section and incorporate the result.
+ const result = this._processNext(state, content);
+ for (const group of result.groups) {
+ this.push('groups', group);
+ currentBatch += group.lines.length;
+ }
+ state.lineNums.left += result.lineDelta.left;
+ state.lineNums.right += result.lineDelta.right;
+
+ // Increment the index and recurse.
+ state.sectionIndex++;
+ if (currentBatch >= this._asyncThreshold) {
+ currentBatch = 0;
+ this._nextStepHandle = this.async(nextStep, 1);
+ } else {
+ nextStep.call(this);
+ }
+ };
+
nextStep.call(this);
- }
- };
-
- nextStep.call(this);
- });
+ }));
+ return this._processPromise
+ .finally(() => { this._processPromise = null; });
},
/**
* Cancel any jobs that are running.
*/
cancel() {
- if (this._nextStepHandle !== undefined) {
+ if (this._nextStepHandle != null) {
this.cancelAsync(this._nextStepHandle);
- this._nextStepHandle = undefined;
+ this._nextStepHandle = null;
+ }
+ if (this._processPromise) {
+ this._processPromise.cancel();
}
},
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();
});
});
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.html b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.html
index 017cd5d..67c32bb 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.html
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.html
@@ -21,6 +21,7 @@
<template>
<gr-lib-loader id="libLoader"></gr-lib-loader>
</template>
+ <script src="../../../scripts/util.js"></script>
<script src="../gr-diff/gr-diff-line.js"></script>
<script src="../gr-diff-highlight/gr-annotation.js"></script>
<script src="gr-syntax-layer.js"></script>
diff --git a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
index a7e7377..32eb969 100644
--- a/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
@@ -155,6 +155,16 @@
},
/** @type {?number} */
_processHandle: Number,
+ /**
+ * The promise last returned from `process()` while the asynchronous
+ * processing is running - `null` otherwise. Provides a `cancel()`
+ * method that rejects it with `{isCancelled: true}`.
+ * @type {?Object}
+ */
+ _processPromise: {
+ type: Object,
+ value: null,
+ },
_hljs: Object,
},
@@ -212,6 +222,10 @@
* @return {Promise}
*/
process() {
+ // Cancel any still running process() calls, because they append to the
+ // same _baseRanges and _revisionRanges fields.
+ this.cancel();
+
// Discard existing ranges.
this._baseRanges = [];
this._revisionRanges = [];
@@ -220,8 +234,6 @@
return Promise.resolve();
}
- this.cancel();
-
if (this.diff.meta_a) {
this._baseLanguage = this._getLanguage(this.diff.meta_a);
}
@@ -241,49 +253,55 @@
lastNotify: {left: 1, right: 1},
};
- return this._loadHLJS().then(() => {
- return new Promise(resolve => {
- const nextStep = () => {
- this._processHandle = null;
- this._processNextLine(state);
+ this._processPromise = util.makeCancelable(this._loadHLJS()
+ .then(() => {
+ return new Promise(resolve => {
+ const nextStep = () => {
+ this._processHandle = null;
+ this._processNextLine(state);
- // Move to the next line in the section.
- state.lineIndex++;
+ // Move to the next line in the section.
+ state.lineIndex++;
- // If the section has been exhausted, move to the next one.
- if (this._isSectionDone(state)) {
- state.lineIndex = 0;
- state.sectionIndex++;
- }
+ // If the section has been exhausted, move to the next one.
+ if (this._isSectionDone(state)) {
+ state.lineIndex = 0;
+ state.sectionIndex++;
+ }
- // If all sections have been exhausted, finish.
- if (state.sectionIndex >= this.diff.content.length) {
- resolve();
- this._notify(state);
- return;
- }
+ // If all sections have been exhausted, finish.
+ if (state.sectionIndex >= this.diff.content.length) {
+ resolve();
+ this._notify(state);
+ return;
+ }
- if (state.lineIndex % 100 === 0) {
- this._notify(state);
- this._processHandle = this.async(nextStep, ASYNC_DELAY);
- } else {
- nextStep.call(this);
- }
- };
+ if (state.lineIndex % 100 === 0) {
+ this._notify(state);
+ this._processHandle = this.async(nextStep, ASYNC_DELAY);
+ } else {
+ nextStep.call(this);
+ }
+ };
- this._processHandle = this.async(nextStep, 1);
- });
- });
+ this._processHandle = this.async(nextStep, 1);
+ });
+ }));
+ return this._processPromise
+ .finally(() => { this._processPromise = null; });
},
/**
* Cancel any asynchronous syntax processing jobs.
*/
cancel() {
- if (this._processHandle) {
+ if (this._processHandle != null) {
this.cancelAsync(this._processHandle);
this._processHandle = null;
}
+ if (this._processPromise) {
+ this._processPromise.cancel();
+ }
},
_diffChanged() {
diff --git a/polygerrit-ui/app/scripts/util.js b/polygerrit-ui/app/scripts/util.js
index b4ab21a..624992b 100644
--- a/polygerrit-ui/app/scripts/util.js
+++ b/polygerrit-ui/app/scripts/util.js
@@ -41,5 +41,39 @@
}
return '';
};
+
+ /**
+ * Make the promise cancelable.
+ *
+ * Returns a promise with a `cancel()` method wrapped around `promise`.
+ * Calling `cancel()` will reject the returned promise with
+ * {isCancelled: true} synchronously. If the inner promise for a cancelled
+ * promise resolves or rejects this is ignored.
+ */
+ util.makeCancelable = promise => {
+ // True if the promise is either resolved or reject (possibly cancelled)
+ let isDone = false;
+
+ let rejectPromise;
+
+ const wrappedPromise = new Promise((resolve, reject) => {
+ rejectPromise = reject;
+ promise.then(val => {
+ if (!isDone) resolve(val);
+ isDone = true;
+ }, error => {
+ if (!isDone) reject(error);
+ isDone = true;
+ });
+ });
+
+ wrappedPromise.cancel = () => {
+ if (isDone) return;
+ rejectPromise({isCanceled: true});
+ isDone = true;
+ };
+ return wrappedPromise;
+ };
+
window.util = util;
})(window);
