Implement viewer CID opt-in #10519
Implement viewer CID opt-in #10519
Conversation
src/service/viewer-cid-api.js
Outdated
| 'googleanalytics': 'AMP_ECID_GOOGLE', | ||
| }; | ||
|
|
||
| export class ViewerCidApi { |
src/service/viewer-cid-api.js
Outdated
| return Promise.resolve(false); | ||
| } | ||
|
|
||
| return this.viewer_.getViewerOrigin().then(origin => { |
There was a problem hiding this comment.
Can't this viewer.isTrustedViewer
There was a problem hiding this comment.
I see you want it Google specific, right?
There was a problem hiding this comment.
right. impl is the same right now. not sure how soon it can diverge.
depending on how likely we'll be changing the impl of viewer.isTrustedViewer?
for simplicity, i can change to isTrustedViewer for now if you like
src/service/viewer-cid-api.js
Outdated
| /** | ||
| * @return {!Promise<boolean>} | ||
| */ | ||
| shouldGetScopedCid(scope) { |
There was a problem hiding this comment.
I think we should always get a scoped CID when the viewer supports it (and is trusted). The sScopeOptedInForCidApi_ should determine whether we request usage of the API.
|
PTAL |
| }; | ||
|
|
||
| /** | ||
| * Exposes CID API if provided by the Viewer. |
There was a problem hiding this comment.
Also rename file to GoogleViewerCidApi
There was a problem hiding this comment.
shall we leave it for other Viewer integration?
src/service/viewer-cid-api.js
Outdated
| getScopedCid(scope) { | ||
| return this.shouldUseCidApi_(scope).then(clientIdApi => { | ||
| return this.viewer_.sendMessageAwaitResponse('cid', dict({ | ||
| scope, |
There was a problem hiding this comment.
See lint error. Must be 'scope': scope
src/service/viewer-cid-api.js
Outdated
| * @returns {boolean} | ||
| */ | ||
| isSupported() { | ||
| return this.viewer_.hasCapability('cid'); |
There was a problem hiding this comment.
This also needs to be guarded by isTrustedViewer.
shouldUseCidApi_ can then just be isScopeOptedInForCidApi_
Closes #10518