Skip to content

Commit

Permalink
Viewer: cleanup definitions of embedded and iframed. Enable web view …
Browse files Browse the repository at this point in the history
…handshake (#3162)

* Viewer: cleanup definitions of embedded and iframed. Enable web view handshake

* Use webview param

* Tests for webview=1
  • Loading branch information
dvoytenko committed May 10, 2016
1 parent 98129cf commit 3545bd4
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 16 deletions.
2 changes: 1 addition & 1 deletion extensions/amp-analytics/0.1/cid-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion src/document-click.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
21 changes: 18 additions & 3 deletions src/service/viewer-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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_;
}
Expand Down
2 changes: 1 addition & 1 deletion src/service/viewport-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
14 changes: 7 additions & 7 deletions test/functional/test-cid.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import * as sinon from 'sinon';

describe('cid', () => {

let isEmbedded;
let isIframed;
let sandbox;
let clock;
let fakeWin;
Expand All @@ -36,7 +36,7 @@ describe('cid', () => {

beforeEach(() => {
let call = 1;
isEmbedded = false;
isIframed = false;
sandbox = sinon.sandbox.create();
clock = sandbox.useFakeTimers();
storage = {};
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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(),
Expand All @@ -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 => {
Expand Down
18 changes: 18 additions & 0 deletions test/functional/test-viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions test/functional/test-viewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ describe('Viewport META', () => {
getScrollTop: () => 0,
getPaddingTop: () => 0,
onViewportEvent: () => {},
isEmbedded: () => false,
isIframed: () => false,
};
viewerMock = sandbox.mock(viewer);

Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);
});
Expand Down

0 comments on commit 3545bd4

Please sign in to comment.