Cancel all debouncers when the element is detached
The long living (60 seconds) debouncer from the error manager was most
likely causing major headaches by making the CI test suite extremely
flaky. Canceling on detach will hopefully make frontend developers much
happier again. :-)
All the other debouncers are not likely not cause major problems,
beacuse they are typically short lived, but it should be common best
practice to always cancel all debouncers on detach. So this change
fixes the entire code base.
There is a call to `flushDebouncers()` is the common-test-setup.ts,
which was thought to be a safeguard against the problem, but in fact
it is not. Added a commment explaining that.
Change-Id: I18f7a8e0304d3f75cf4317ea567b9f76b585398b
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
index 034081d..9fafcfa 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.ts
@@ -64,6 +64,8 @@
*/
const MAX_GROUP_SIZE = 120;
+const DEBOUNCER_RESET_IS_SCROLLING = 'resetIsScrolling';
+
/**
* Converts the API's `DiffContent`s to `GrDiffGroup`s for rendering.
*
@@ -123,6 +125,7 @@
/** @override */
detached() {
super.detached();
+ this.cancelDebouncer(DEBOUNCER_RESET_IS_SCROLLING);
this.cancel();
this.unlisten(window, 'scroll', '_handleWindowScroll');
}
@@ -130,7 +133,7 @@
_handleWindowScroll() {
this._isScrolling = true;
this.debounce(
- 'resetIsScrolling',
+ DEBOUNCER_RESET_IS_SCROLLING,
() => {
this._isScrolling = false;
},
