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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃殌 amp-subscriptions prerender #21643

Merged
merged 9 commits into from
Apr 3, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,18 @@ export class GoogleSubscriptionsPlatform {
'success');
}

/** @override */
isPrerenderSafe() {
/**
* If it's a google viewer then calling google for an
* entitlement at prerender time does not leak any private
* information. If it's not a google viewer then we wait
* for the page to be visible to avoid leaking that the
* page was prerendered
*/
return this.isGoogleViewer_;
}

/** @override */
getEntitlements() {
return this.runtime_.getEntitlements().then(swgEntitlements => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,11 @@ describes.realWin('amp-subscriptions-google', {amp: true}, env => {
expect(methods.linkAccount).to.not.be.called;
});

it('should not allow prerender for non-google viewer', () => {
platform.isGoogleViewer_ = false;
expect(platform.isPrerenderSafe()).to.be.false;
});

it('should reauthorize on complete linking', () => {
analyticsMock.expects('actionEvent')
.withExactArgs(PLATFORM_ID, 'link', 'success')
Expand Down Expand Up @@ -389,6 +394,12 @@ describes.realWin('amp-subscriptions-google', {amp: true}, env => {
});
});

it('should allow prerender if in a google viewer', () => {
viewer.params_['viewerUrl'] = 'https://www.google.com/other';
platform = new GoogleSubscriptionsPlatform(ampdoc, {}, serviceAdapter);
expect(platform.isPrerenderSafe()).to.be.true;
});

it('should attach button given to decorateUI', () => {
const elem = env.win.document.createElement('div');
const decorateStub = sandbox.stub(platform.runtime_.buttonApi_,
Expand Down
6 changes: 5 additions & 1 deletion extensions/amp-subscriptions/0.1/amp-subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,11 @@ export class SubscriptionService {
if (getMode().development || getMode().localDev) {
timeout = SERVICE_TIMEOUT * 2;
}
return this.viewer_.whenFirstVisible().then(() => {
// Prerender safe platforms don't have to wait for the
// page to become visible, all others wait for whenFirstVisible()
const visiblePromise = subscriptionPlatform.isPrerenderSafe() ?
Promise.resolve() : this.viewer_.whenFirstVisible();
return visiblePromise.then(() => {
return this.timer_.timeoutPromise(
timeout,
subscriptionPlatform.getEntitlements()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ export class LocalSubscriptionPlatform {
});
}

/** @override */
isPrerenderSafe() {
// Local platform can never be allowed to prerender in a viewer
return false;
}

/** @override */
getEntitlements() {
return this.urlBuilder_.buildUrl(this.authorizationUrl_,
Expand Down
7 changes: 7 additions & 0 deletions extensions/amp-subscriptions/0.1/subscription-platform.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ export class SubscriptionPlatform {
*/
reset() {}

/**
* True if this platform can fetch entitlement safely in pre-render
* without leaking information to the publisher or a 3rd party
* @return {boolean}
*/
isPrerenderSafe() {}

/**
* Returns if pingback is enabled for this platform.
* @return {boolean}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ describes.fakeWin('LocalSubscriptionsPlatform', {amp: true}, env => {
expect(localSubscriptionPlatform.getBaseScore()).to.be.equal(99);
});

it('Should not allow prerender', () => {
expect(localSubscriptionPlatform.isPrerenderSafe()).to.be.false;
});

it('should fetch the entitlements on getEntitlements', () => {
const fetchStub = sandbox.stub(localSubscriptionPlatform.xhr_, 'fetchJson')
.callsFake(() => Promise.resolve({json: () => Promise.resolve(json)}));
Expand Down
4 changes: 4 additions & 0 deletions extensions/amp-subscriptions/0.1/test/test-viewer-platform.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ describes.fakeWin('ViewerSubscriptionPlatform', {amp: true}, env => {
});
});

it('Should allow prerender', () => {
expect(viewerPlatform.isPrerenderSafe()).to.be.true;
});

describe('verifyAuthToken_', () => {
const entitlement = Entitlement.parseFromJson(entitlementData);
entitlement.service = 'local';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ export class ViewerSubscriptionPlatform {
this.origin_ = origin;
}

/** @override */
isPrerenderSafe() {
return true;
}

/** @override */
getEntitlements() {
devAssert(this.currentProductId_, 'Current product is not set');
Expand Down