From d512905e6c5dcc285e8272735ce0d45d9120b778 Mon Sep 17 00:00:00 2001 From: Tao Zhou Date: Tue, 3 Dec 2019 17:29:16 +0100 Subject: [PATCH] Notify all coverage listeners when coverage data is available Since coverage data can be provided asynchronously and the diff rendering won't wait for it. So its neccessary to notify all listeners once data coming back. Change-Id: I19625d6c53ad15491c3ec6f508a4bc076334e015 --- .../diff/gr-diff-builder/gr-diff-builder.html | 1 - .../diff/gr-diff-host/gr-diff-host.js | 8 ++++++-- .../app/elements/diff/gr-diff/gr-diff.html | 2 +- .../gr-js-api-interface.js | 20 +++++++++++++++++-- 4 files changed, 25 insertions(+), 6 deletions(-) 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 2a57162eb6..01e1f1fa5a 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 @@ -77,7 +77,6 @@ limitations under the License. properties: { diff: Object, - diffPath: String, changeNum: String, patchNum: String, viewMode: String, diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js index f5dc438518..bdb914f1c6 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js @@ -301,7 +301,7 @@ const layers = [this.$.syntaxLayer]; // Get layers from plugins (if any). for (const pluginLayer of this.$.jsAPI.getDiffLayers( - this.diffPath, this.changeNum, this.patchNum)) { + this.path, this.changeNum, this.patchNum)) { layers.push(pluginLayer); } this._layers = layers; @@ -313,7 +313,9 @@ this._coverageRanges = []; const {changeNum, path, patchRange: {basePatchNum, patchNum}} = this; - this.$.jsAPI.getCoverageRanges(changeNum, path, basePatchNum, patchNum). + this.$.jsAPI.getCoverageRanges( + changeNum, path, basePatchNum, patchNum + ). then(coverageRanges => { if (changeNum !== this.changeNum || path !== this.path || @@ -344,6 +346,8 @@ return this._loadDiffAssets(diff); }); + // Not waiting for getCoverageRanges intentionally as + // plugin loading should not block the content rendering return Promise.all([diffRequest, assetRequest]) .then(results => { const diff = results[0]; diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html index 1446fd4bb1..3b64f5566f 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html @@ -373,7 +373,7 @@ limitations under the License. coverage-ranges="[[coverageRanges]]" project-name="[[projectName]]" diff="[[diff]]" - diff-path="[[path]]" + path="[[path]]" change-num="[[changeNum]]" patch-num="[[patchRange.patchNum]]" view-mode="[[viewMode]]" diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js index 1802a4a750..ae08007a13 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface.js @@ -270,10 +270,26 @@ // Only one coverage provider makes sense. If there are more, then we // simply ignore them. if (provider) { - return provider(changeNum, path, basePatchNum, patchNum); + return annotationApi; } } - return []; + return null; + }).then(annotationApi => { + if (!annotationApi) return []; + const provider = annotationApi.getCoverageProvider(); + return provider(changeNum, path, basePatchNum, patchNum) + .then(ranges => { + ranges = ranges || []; + // Notify with the coverage data. + ranges.forEach(range => { + annotationApi.notify( + path, + range.code_range.start_line, + range.code_range.end_line, + range.side); + }); + return ranges; + }); }); }