From 3545bd4b50a5edb40845df83e7f2d75e08bb5055 Mon Sep 17 00:00:00 2001 From: Dima Voytenko Date: Mon, 9 May 2016 17:56:05 -0700 Subject: [PATCH] Viewer: cleanup definitions of embedded and iframed. Enable web view handshake (#3162) * Viewer: cleanup definitions of embedded and iframed. Enable web view handshake * Use webview param * Tests for webview=1 --- extensions/amp-analytics/0.1/cid-impl.js | 2 +- src/document-click.js | 2 +- src/service/viewer-impl.js | 21 ++++++++++++++++++--- src/service/viewport-impl.js | 2 +- test/functional/test-cid.js | 14 +++++++------- test/functional/test-viewer.js | 18 ++++++++++++++++++ test/functional/test-viewport.js | 6 +++--- 7 files changed, 49 insertions(+), 16 deletions(-) diff --git a/extensions/amp-analytics/0.1/cid-impl.js b/extensions/amp-analytics/0.1/cid-impl.js index c6b6a1976d23..66f8d305ada6 100644 --- a/extensions/amp-analytics/0.1/cid-impl.js +++ b/extensions/amp-analytics/0.1/cid-impl.js @@ -251,7 +251,7 @@ function getBaseCid(cid, persistenceConsent) { // If we are being embedded, try to get the base cid from the viewer. // Note, that we never try to persist to localStorage in this case. const viewer = viewerFor(win); - if (viewer.isEmbedded()) { + if (viewer.isIframed()) { return viewer.getBaseCid().then(cid => { if (!cid) { throw new Error('No CID'); diff --git a/src/document-click.js b/src/document-click.js index 894752157afd..228d490068c1 100644 --- a/src/document-click.js +++ b/src/document-click.js @@ -70,7 +70,7 @@ export class ClickHandler { this.history_ = historyFor(this.win); // Only intercept clicks when iframed. - if (this.viewer_.isEmbedded() && this.viewer_.isOvertakeHistory()) { + if (this.viewer_.isIframed() && this.viewer_.isOvertakeHistory()) { /** @private @const {!function(!Event)|undefined} */ this.boundHandle_ = this.handle_.bind(this); this.win.document.documentElement.addEventListener( diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index 42eccfb73885..df5bf3f4e39a 100644 --- a/src/service/viewer-impl.js +++ b/src/service/viewer-impl.js @@ -141,7 +141,7 @@ export class Viewer { this.win = win; /** @private @const {boolean} */ - this.isEmbedded_ = (this.win.parent && this.win.parent != this.win); + this.isIframed_ = (this.win.parent && this.win.parent != this.win); /** @const {!DocumentState} */ this.docState_ = documentStateFor(this.win); @@ -242,8 +242,8 @@ export class Viewer { dev.fine(TAG_, '- prerenderSize:', this.prerenderSize_); this.viewportType_ = this.params_['viewportType'] || this.viewportType_; - // Configure scrolling parameters when AMP is embeded in a viewer on iOS. - if (this.viewportType_ == ViewportType.NATURAL && this.isEmbedded_ && + // Configure scrolling parameters when AMP is iframed on iOS. + if (this.viewportType_ == ViewportType.NATURAL && this.isIframed_ && platform.isIos()) { this.viewportType_ = ViewportType.NATURAL_IOS_EMBED; } @@ -276,6 +276,13 @@ export class Viewer { this.performanceTracking_ = this.params_['csi'] === '1'; dev.fine(TAG_, '- performanceTracking:', this.performanceTracking_); + /** + * Whether the AMP document is embedded in a viewer, such as an iframe or + * a web view. + * @private @const {boolean} + */ + this.isEmbedded_ = this.isIframed_ || this.params_['webview'] === '1'; + /** @private {boolean} */ this.hasBeenVisible_ = this.isVisible(); @@ -455,6 +462,14 @@ export class Viewer { * Whether the document is embedded in a iframe. * @return {boolean} */ + isIframed() { + return this.isIframed_; + } + + /** + * Whether the document is embedded in a viewer. + * @return {boolean} + */ isEmbedded() { return this.isEmbedded_; } diff --git a/src/service/viewport-impl.js b/src/service/viewport-impl.js index e6e2ab60f3b8..dd5a51a0a258 100644 --- a/src/service/viewport-impl.js +++ b/src/service/viewport-impl.js @@ -398,7 +398,7 @@ export class Viewport { * @private */ getViewportMeta_() { - if (this.viewer_.isEmbedded()) { + if (this.viewer_.isIframed()) { // An embedded document does not control its viewport meta tag. return null; } diff --git a/test/functional/test-cid.js b/test/functional/test-cid.js index 3d9992ec646e..180b3028ef84 100644 --- a/test/functional/test-cid.js +++ b/test/functional/test-cid.js @@ -26,7 +26,7 @@ import * as sinon from 'sinon'; describe('cid', () => { - let isEmbedded; + let isIframed; let sandbox; let clock; let fakeWin; @@ -36,7 +36,7 @@ describe('cid', () => { beforeEach(() => { let call = 1; - isEmbedded = false; + isIframed = false; sandbox = sinon.sandbox.create(); clock = sandbox.useFakeTimers(); storage = {}; @@ -69,8 +69,8 @@ describe('cid', () => { }, }; const viewer = installViewerService(fakeWin); - sandbox.stub(viewer, 'isEmbedded', function() { - return isEmbedded; + sandbox.stub(viewer, 'isIframed', function() { + return isIframed; }); sandbox.stub(viewer, 'getBaseCid', function() { return Promise.resolve('from-viewer'); @@ -175,14 +175,14 @@ describe('cid', () => { }); it('should retrieve cid from viewer if embedded', () => { - isEmbedded = true; + isIframed = true; return compare( 'e2', 'sha384(from-viewerhttp://www.origin.come2)'); }); it('should prefer value in storage if present', () => { - isEmbedded = true; + isIframed = true; storage['amp-cid'] = JSON.stringify({ cid: 'in-storage', time: timer.now(), @@ -205,7 +205,7 @@ describe('cid', () => { }; win.__proto__ = window; expect(win.location.href).to.equal('https://cdn.ampproject.org/v/www.origin.com/'); - installViewerService(win).isEmbedded = () => false; + installViewerService(win).isIframed = () => false; installCidService(win); return cidFor(win).then(cid => { return cid.get('foo', hasConsent).then(c1 => { diff --git a/test/functional/test-viewer.js b/test/functional/test-viewer.js index 05667860ee19..76152ff79492 100644 --- a/test/functional/test-viewer.js +++ b/test/functional/test-viewer.js @@ -573,6 +573,24 @@ describe('Viewer', () => { }); }); + describe('isEmbedded', () => { + it('should NOT be embedded when not iframed or w/o "origin"', () => { + windowApi.parent = windowApi; + expect(new Viewer(windowApi).isEmbedded()).to.be.false; + }); + + it('should be embedded when iframed', () => { + windowApi.parent = {}; + expect(new Viewer(windowApi).isEmbedded()).to.be.true; + }); + + it('should be embedded with "origin" param', () => { + windowApi.parent = windowApi; + windowApi.location.hash = '#webview=1'; + expect(new Viewer(windowApi).isEmbedded()).to.be.true; + }); + }); + describe('isTrustedViewer', () => { function test(origin, toBeTrusted) { diff --git a/test/functional/test-viewport.js b/test/functional/test-viewport.js index 7af368bb5e4d..9ac7394f21b9 100644 --- a/test/functional/test-viewport.js +++ b/test/functional/test-viewport.js @@ -379,7 +379,7 @@ describe('Viewport META', () => { getScrollTop: () => 0, getPaddingTop: () => 0, onViewportEvent: () => {}, - isEmbedded: () => false, + isIframed: () => false, }; viewerMock = sandbox.mock(viewer); @@ -436,7 +436,7 @@ describe('Viewport META', () => { }); it('should ignore disable TouchZoom if embedded', () => { - viewerMock.expects('isEmbedded').returns(true).atLeast(1); + viewerMock.expects('isIframed').returns(true).atLeast(1); viewport.disableTouchZoom(); expect(viewportMetaSetter.callCount).to.equal(0); }); @@ -478,7 +478,7 @@ describe('Viewport META', () => { }); it('should ignore reset TouchZoom if embedded', () => { - viewerMock.expects('isEmbedded').returns(true).atLeast(1); + viewerMock.expects('isIframed').returns(true).atLeast(1); viewport.resetTouchZoom(); expect(viewportMetaSetter.callCount).to.equal(0); });