Merge "Fix diff-builder leaks"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js
index c9d5d78..bb617f0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.js
@@ -113,6 +113,14 @@
};
}
+ /** @override */
+ detached() {
+ super.detached();
+ if (this._builder) {
+ this._builder.clear();
+ }
+ }
+
get diffElement() {
return this.queryEffectiveChildren('#diffTable');
}
@@ -144,6 +152,9 @@
// Stop the processor if it's running.
this.cancel();
+ if (this._builder) {
+ this._builder.clear();
+ }
this._builder = this._getDiffBuilder(this.diff, prefs);
this.$.processor.context = prefs.context;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 1cabd82..04f1548 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -62,13 +62,22 @@
throw Error('Invalid line length from preferences.');
}
+ this._layerUpdateListener = this._handleLayerUpdate.bind(this);
for (const layer of this.layers) {
if (layer.addListener) {
- layer.addListener(this._handleLayerUpdate.bind(this));
+ layer.addListener(this._layerUpdateListener);
}
}
}
+GrDiffBuilder.prototype.clear = function() {
+ for (const layer of this.layers) {
+ if (layer.removeListener) {
+ layer.removeListener(this._layerUpdateListener);
+ }
+ }
+};
+
GrDiffBuilder.GroupType = {
ADDED: 'b',
BOTH: 'ab',
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 cb299de..abcdc73 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
@@ -314,12 +314,19 @@
});
}
+ /** @override */
+ detached() {
+ super.detached();
+ this.clear();
+ }
+
/**
* @param {boolean=} shouldReportMetric indicate a new Diff Page. This is a
* signal to report metrics event that started on location change.
* @return {!Promise}
**/
reload(shouldReportMetric) {
+ this.clear();
this._loading = true;
this._errorMessage = null;
const whitespaceLevel = this._getIgnoreWhitespace();
@@ -398,6 +405,11 @@
.then(() => { this._loading = false; });
}
+ clear() {
+ this.$.jsAPI.disposeDiffLayers(this.path);
+ this._layers = [];
+ }
+
_getCoverageData() {
const {changeNum, path, patchRange: {basePatchNum, patchNum}} = this;
this.$.jsAPI.getCoverageAnnotationApi().
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
index 9c9997c..c774f1a 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
@@ -107,6 +107,10 @@
this._listeners.push(fn);
}
+ removeListener(fn) {
+ this._listeners = this._listeners.filter(f => f != fn);
+ }
+
/**
* Notify Layer listeners of changes to annotations.
*
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.js
index 3b24404..6f73c36 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.js
@@ -144,7 +144,6 @@
// path.
if (annotationLayer._path === path) {
annotationLayer.notifyListeners(startRange, endRange, side);
- break;
}
}
};
@@ -153,6 +152,8 @@
* Should be called to register annotation layers by the framework. Not
* intended to be called by plugins.
*
+ * Don't forget to dispose layer.
+ *
* @param {string} path The file path (eg: /COMMIT_MSG').
* @param {string} changeNum The Gerrit change number.
* @param {string} patchNum The Gerrit patch number.
@@ -165,6 +166,11 @@
return annotationLayer;
};
+GrAnnotationActionsInterface.prototype.disposeLayer = function(path) {
+ this._annotationLayers = this._annotationLayers
+ .filter(annotationLayer => annotationLayer._path !== path);
+};
+
/**
* Used to create an instance of the Annotation Layer interface.
*
@@ -186,6 +192,7 @@
/**
* Register a listener for layer updates.
+ * Don't forget to removeListener when you stop using layer.
*
* @param {Function} fn The update handler function.
* Should accept as arguments the line numbers for the start and end of
@@ -195,6 +202,10 @@
this._listeners.push(fn);
};
+AnnotationLayer.prototype.removeListener = function(fn) {
+ this._listeners = this._listeners.filter(f => f != fn);
+};
+
/**
* Layer method to add annotations to a line.
*
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.js
index a98936f..43d7378 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.js
@@ -277,6 +277,17 @@
return layers;
}
+ disposeDiffLayers(path) {
+ for (const annotationApi of
+ this._getEventCallbacks(EventType.ANNOTATE_DIFF)) {
+ try {
+ annotationApi.disposeLayer(path);
+ } catch (err) {
+ console.error(err);
+ }
+ }
+ }
+
/**
* Retrieves coverage data possibly provided by a plugin.
*
