ampproject / ampproject/amphtml
Clearing instrumented intervals in 3p environmented frames cancels timers
- Dominant language
- JavaScript
- Stars
- 14.9k
- Forks
- 4.1k
- PR merge metrics
- No merged PRs in 30d
Description
Developing an amp-ad integration I faced a problem with some of my timeouts not firing up correctly (at all actually), after some extensive investigation I found the [native `clearInterval` call](https://github.com/ampproject/amphtml/blob/4007122a07a2c6b3f4310c0d3967039c4d83d678/3p/environment.js#L202) in the instrumented method to be the culprit. In was added in [this commit](https://github.com/ampproject/amphtml/commit/c2fb9cf5a9ae6365937f3c8589fad678f0ce94b5).
In native timers/intervals browser [shares the same ids pool](https://developer.mozilla.org/en-US/docs/Web/API/Window/clearTimeout) between those, so in any arbitrary point in time a pair of a timer and an interval with the same id value can not exist. Thus native `clearTimeout` and `clearInterval` native methods do the same - cancel the delayed call by the `timerId` passed regardless of the timer/interval type, and either a timer or an interval can be cancelled with either of those (the difference of the methods is only in names).
In the AMP lib the instrumented version of intervals uses own iterator to get/set ids for the created intervals and this numbering intersects with the browser built in used for timers. So the introduced call to the native `clearInterval` method for an interval id for which a native timer was previously created will cancel this timer as a unintended consequence.
Here is a test code I used to demonstrate this (to test locally, I`ve created a fake adm-ad provider and added it to the ads.amp.html page)
```javascript
/**
* @param {!Window} global
* @param {!Object} data
*/
export function timerstest(global, data) {
let tmr1 = setTimeout(function() { console.log("-- timer 1 fired ("+tmr1+")"); }, 3000);
console.log("-- timer 1 created ("+tmr1+")");
let tmr2 = setTimeout(function() { console.log("-- timer 2 fired ("+tmr2+")"); }, 3000);
console.log("-- timer 2 created ("+tmr2+")");
let int1Cnt = 1;
let int1 = setInterval(function() {
console.log("interval 1 fired ("+int1+", "+(int1Cnt++)+")");
if (int1Cnt>4) {
console.log("clearing interval 1 ("+int1+", "+(int1Cnt++)+")");
global.clearInterval(int1);
}
}, 500);
console.log("interval 1 created ("+int1+")");
}
```
The console output (the timer is actually never called as it is cancelled by the `clearInterval` call):

I guess either the timers must share the "instrumented" ids and be cleared the same way as intervals, or the native `clearInterval` call must be removed from the instrumented method (direct cancelling of intervals from outside of the frame seems to be "strange" practice imo).
Contributor guide
Assessment
This issue has not been assessed yet.