Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
May 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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