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

Conversation

jpettitt
Copy link
Contributor

@jpettitt jpettitt commented Mar 29, 2019

The PR allows amp-subscriptions to prerender pages in cases where the entitlements can safely be obtained without leaking privacy.

Adds a new isPrerenderSafe() method to the SubscriptionPlatform class.

If isPrerenderSafe() is true then getEntitlements() is allowed to progress even if the page is in prerender mode.

There are currently two platforms that are prerender safe.

  1. ViewerSubscriptionPlatform this platform is by definition prerender safe in that the viewer already knows what page is loaded so asking it for an entitlement does not leak additional information to the publisher or a third party.

  2. GoogleSubscriptionsPlatformService is pre-render safe when running under the Google viewer. In that instance google already knows which page was loaded so again asking for entitlements does not present a privacy leak. However on non-google platforms prerender is disabled to prevent a privacy leak.

@ghost
Copy link

ghost commented Mar 29, 2019

This pull request introduces 2 alerts when merging 17cddd8 into 0eecb1e - view on LGTM.com

new alerts:

  • 2 for Expression has no effect

Comment posted by LGTM.com

@jpettitt jpettitt requested a review from dvoytenko March 29, 2019 22:26
@jpettitt jpettitt marked this pull request as ready for review March 29, 2019 22:40
Copy link
Contributor

@dvoytenko dvoytenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Just needs tests.

@jpettitt jpettitt requested a review from dvoytenko April 1, 2019 22:38
@jpettitt jpettitt merged commit bde46f7 into ampproject:master Apr 3, 2019
@jpettitt jpettitt deleted the swg-prerender branch April 3, 2019 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants