From 342f40c9b7547f4afd6b238382a6d5557c92e858 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Fri, 22 Nov 2019 16:44:56 -0800 Subject: [PATCH] Deflake FIE and amp-iframe tests executed on the running runtime (#25727) * Testing master branch tests * deflake * Remove async errors * lints --- .../amp-iframe/0.1/test/test-amp-iframe.js | 64 ++++++++++++------- src/service/resource.js | 11 +++- 2 files changed, 52 insertions(+), 23 deletions(-) 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 1afa33553cc1..498571c1e209 100644 --- a/extensions/amp-iframe/0.1/test/test-amp-iframe.js +++ b/extensions/amp-iframe/0.1/test/test-amp-iframe.js @@ -80,6 +80,25 @@ describes.realWin( setTrackingIframeTimeoutForTesting(20); }); + function stubUserAsserts() { + const errors = []; + env.sandbox + .stub(user(), 'assert') + .callsFake((shouldBeTrueish, message) => { + if (!shouldBeTrueish) { + errors.push(message); + } + return shouldBeTrueish; + }); + const replay = function() { + if (errors.length > 0) { + throw errors[0]; + } + }; + replay.errors = errors; + return replay; + } + function waitForJsInIframe(opt_ranJs = 1, opt_timeout = 300) { return poll( 'waiting for JS to run', @@ -171,13 +190,13 @@ describes.realWin( return ampIframe; } - it('should render iframe', function*() { + it('should render iframe', async () => { const ampIframe = createAmpIframe(env, { src: iframeSrc, width: 100, height: 100, }); - yield waitForAmpIframeLayoutPromise(doc, ampIframe); + await waitForAmpIframeLayoutPromise(doc, ampIframe); const impl = ampIframe.implementation_; const iframe = ampIframe.querySelector('iframe'); expect(iframe.src).to.equal(iframeSrc + '#amp=1'); @@ -188,7 +207,7 @@ describes.realWin( expect(iframe.parentNode).to.equal(scrollWrapper); expect(impl.looksLikeTrackingIframe_()).to.be.false; expect(impl.getLayoutPriority()).to.equal(LayoutPriority.CONTENT); - yield timer.promise(IFRAME_MESSAGE_TIMEOUT); + await timer.promise(IFRAME_MESSAGE_TIMEOUT); expect(ranJs).to.equal(0); }); @@ -268,7 +287,7 @@ describes.realWin( }); }); - it('should not render at the top', function*() { + it('should not render at the top', async () => { expectAsyncConsoleError(/position/); const ampIframe = createAmpIframe( env, @@ -281,8 +300,8 @@ describes.realWin( 599, 1000 ); - yield whenUpgradedToCustomElement(ampIframe); - yield ampIframe.signals().whenSignal(CommonSignals.LOAD_START); + await whenUpgradedToCustomElement(ampIframe); + await ampIframe.signals().whenSignal(CommonSignals.LOAD_START); }); it('should respect translations', function*() { @@ -319,15 +338,16 @@ describes.realWin( expect(ampIframe.querySelector('iframe')).to.not.be.null; }); - it('should deny http', function*() { + it('should deny http', async () => { + const asserts = stubUserAsserts(); const ampIframe = createAmpIframe(env, { src: 'http://google.com/fpp', sandbox: 'allow-scripts', width: 100, height: 100, }); - yield waitForAmpIframeLayoutPromise(doc, ampIframe); - expect(ampIframe.querySelector('iframe')).to.be.null; + await waitForAmpIframeLayoutPromise(doc, ampIframe); + expect(asserts).to.throw(/Must start with https/); }); it('should allow data-uri', function*() { @@ -380,19 +400,20 @@ describes.realWin( }); }); - it('should deny srcdoc with allow-same-origin', function*() { + it('should deny srcdoc with allow-same-origin', async () => { + const asserts = stubUserAsserts(); const ampIframe = createAmpIframe(env, { width: 100, height: 100, sandbox: 'allow-same-origin', - srcdoc: '', + srcdoc: 'test', }); - yield waitForAmpIframeLayoutPromise(doc, ampIframe); - const iframe = ampIframe.querySelector('iframe'); - expect(iframe).to.be.null; + await waitForAmpIframeLayoutPromise(doc, ampIframe); + expect(asserts).to.throw(/allow-same-origin.*srcdoc/); }); - it('should deny data uri with allow-same-origin', function*() { + it('should deny data uri with allow-same-origin', async () => { + const asserts = stubUserAsserts(); const ampIframe = createAmpIframe(env, { width: 100, height: 100, @@ -402,12 +423,12 @@ describes.realWin( 'PHNjcmlwdD5kb2N1bWVudC53cml0ZSgnUiAnICsgZG9jdW1lbnQucmVmZXJyZXIgK' + 'yAnLCAnICsgbG9jYXRpb24uaHJlZik8L3NjcmlwdD4=', }); - yield waitForAmpIframeLayoutPromise(doc, ampIframe); - const iframe = ampIframe.querySelector('iframe'); - expect(iframe).to.be.null; + await waitForAmpIframeLayoutPromise(doc, ampIframe); + expect(asserts).to.throw(/amp-iframe-origin-policy.md/); }); - it('should deny DATA uri with allow-same-origin', function*() { + it('should deny DATA uri with allow-same-origin', async () => { + const asserts = stubUserAsserts(); const ampIframe = createAmpIframe(env, { width: 100, height: 100, @@ -417,9 +438,8 @@ describes.realWin( 'PHNjcmlwdD5kb2N1bWVudC53cml0ZSgnUiAnICsgZG9jdW1lbnQucmVmZXJyZXIgK' + 'yAnLCAnICsgbG9jYXRpb24uaHJlZik8L3NjcmlwdD4=', }); - yield waitForAmpIframeLayoutPromise(doc, ampIframe); - const iframe = ampIframe.querySelector('iframe'); - expect(iframe).to.be.null; + await waitForAmpIframeLayoutPromise(doc, ampIframe); + expect(asserts).to.throw(/amp-iframe-origin-policy.md/); }); it('should deny same origin', () => { diff --git a/src/service/resource.js b/src/service/resource.js index 4fb935810683..8619aefb0919 100644 --- a/src/service/resource.js +++ b/src/service/resource.js @@ -435,10 +435,19 @@ export class Resource { ) { return; } + if ( + !this.element.ownerDocument || + !this.element.ownerDocument.defaultView + ) { + // Most likely this is an element who's window has just been destroyed. + // This is an issue with FIE embeds destruction. Such elements will be + // considered "not displayable" until they are GC'ed. + this.state_ = ResourceState.NOT_LAID_OUT; + return; + } this.isMeasureRequested_ = false; - // TODO const oldBox = this.layoutBox_; this.measureViaResources_(); const box = this.layoutBox_;