IntersectionObserver needs to be properly cleaned up

[cramforce]

Currently we just null it. Instead we should call destroy or similar. In this we need to e.g. cancel the timeout if we have one scheduled. Otherwise we might post messages to windows which no longer exist.

Similarly, we fail to deregister on detachedCallback. Because amp-sticky-ad removes the ad, we never learn about this.

We should consider calling unlayoutCallback from detachedCallback by default. @dvoytenko @jridgewell

@erwinmombay I think this might cause the postMessage errors.

[cramforce]

Ah, yes, and because we never get a viewportCallback that unregisters the scroll handler, we create an error on every scroll.

[zhouyx]

Oh I only remove the the amp-sticky-ad component from dom, and doesn't take care of the ad. To confirm, the fix for now would be to deregister amp-ad element first before remove amp-sticky-ad?

[cramforce]

Nope, the right fix would be to either implement detachedCallback in amp-ad (or amp-ad-api-handler) or for us to decide to always call unlayoutCallback and viewportCallback from detachCallback in custom-element.js.

[cramforce]

@zhouyx I think the smaller race here is still around, but lower priority
https://github.com/ampproject/amphtml/blob/master/src/intersection-observer.js#L214

Basically we need to check in the callback whether the iframe still exists.