From f7e5fa88e6f8cf34a1c4324d247cfd34560ab51d Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Tue, 9 Feb 2021 00:56:27 -0800 Subject: [PATCH 1/2] Revert "fix overly eager iframe unload() call (#32459)" This reverts commit 784e808a15649f1b3c417c4998c58bdac9aebcc6. --- extensions/amp-iframe/0.1/amp-iframe.js | 3 +-- .../amp-iframe/0.1/test/test-amp-iframe.js | 20 ------------------- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/extensions/amp-iframe/0.1/amp-iframe.js b/extensions/amp-iframe/0.1/amp-iframe.js index 61e71e289e05..4db34bf7b2ae 100644 --- a/extensions/amp-iframe/0.1/amp-iframe.js +++ b/extensions/amp-iframe/0.1/amp-iframe.js @@ -645,8 +645,7 @@ export class AmpIframe extends AMP.BaseElement { return; } this.isDisplayed_ = isDisplayed; - const hasOwner = !!this.element.getOwner(); - if (!isDisplayed && !hasOwner && this.iframe_) { + if (!isDisplayed && this.iframe_) { this.getVsync().mutate(() => this.unload()); } } diff --git a/extensions/amp-iframe/0.1/test/test-amp-iframe.js b/extensions/amp-iframe/0.1/test/test-amp-iframe.js index d082758223cd..4a62f65ed239 100644 --- a/extensions/amp-iframe/0.1/test/test-amp-iframe.js +++ b/extensions/amp-iframe/0.1/test/test-amp-iframe.js @@ -1101,26 +1101,6 @@ describes.realWin( expect(impl.unload).to.be.calledOnce; }); - it('should not unlayout on pause if has owner', async () => { - const ampIframe = createAmpIframe(env, { - src: iframeSrc, - width: 100, - height: 100, - }); - const impl = await ampIframe.getImpl(false); - impl.element.getOwner = () => ({}); - await waitForAmpIframeLayoutPromise(doc, ampIframe); - expect(ampIframe.querySelector('iframe')).to.exist; - expect(ampIframe.unlayoutOnPause()).to.be.true; - - setDisplay(ampIframe, true); - setDisplay(ampIframe, false); - env.sandbox.stub(impl, 'unload'); - - await new Promise(setTimeout); - expect(impl.unload).not.called; - }); - it('should now need pausing before displayed', async () => { const ampIframe = createAmpIframe(env, { src: iframeSrc, From 3acd10ad4aeb41cbb7fb07a03c5c8be1e07ac8c1 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Tue, 9 Feb 2021 00:57:16 -0800 Subject: [PATCH 2/2] Revert "amp-iframe: unlayoutOnPause (#32466)" This reverts commit 32ccec8ff1c35cccd936268f32bac98a00bada9b. --- extensions/amp-iframe/0.1/amp-iframe.js | 5 ----- extensions/amp-iframe/0.1/test/test-amp-iframe.js | 6 +++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/extensions/amp-iframe/0.1/amp-iframe.js b/extensions/amp-iframe/0.1/amp-iframe.js index 4db34bf7b2ae..1ad6c3077de7 100644 --- a/extensions/amp-iframe/0.1/amp-iframe.js +++ b/extensions/amp-iframe/0.1/amp-iframe.js @@ -631,11 +631,6 @@ export class AmpIframe extends AMP.BaseElement { } } - /** @override */ - unlayoutOnPause() { - return true; - } - /** * @param {boolean} isDisplayed * @private diff --git a/extensions/amp-iframe/0.1/test/test-amp-iframe.js b/extensions/amp-iframe/0.1/test/test-amp-iframe.js index 4a62f65ed239..c1193f451f15 100644 --- a/extensions/amp-iframe/0.1/test/test-amp-iframe.js +++ b/extensions/amp-iframe/0.1/test/test-amp-iframe.js @@ -1089,7 +1089,7 @@ describes.realWin( const impl = await ampIframe.getImpl(false); await waitForAmpIframeLayoutPromise(doc, ampIframe); expect(ampIframe.querySelector('iframe')).to.exist; - expect(ampIframe.unlayoutOnPause()).to.be.true; + expect(ampIframe.unlayoutOnPause()).to.be.false; // The element should become visible before pause is necessary. setDisplay(ampIframe, true); @@ -1110,7 +1110,7 @@ describes.realWin( const impl = await ampIframe.getImpl(false); await waitForAmpIframeLayoutPromise(doc, ampIframe); expect(ampIframe.querySelector('iframe')).to.exist; - expect(ampIframe.unlayoutOnPause()).to.be.true; + expect(ampIframe.unlayoutOnPause()).to.be.false; // The element was never visible. env.sandbox./*OK*/ stub(impl, 'unload'); @@ -1128,7 +1128,7 @@ describes.realWin( }); const impl = await ampIframe.getImpl(false); expect(ampIframe.querySelector('iframe')).to.not.exist; - expect(ampIframe.unlayoutOnPause()).to.be.true; + expect(ampIframe.unlayoutOnPause()).to.be.false; env.sandbox./*OK*/ stub(impl, 'unload'); setDisplay(ampIframe, true);