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

Backup CID in Storage for non-proxy origin #30375

Merged
merged 8 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 5 additions & 1 deletion extensions/amp-access/0.1/amp-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,11 @@ export class AccessService {
const consent = Promise.resolve();
this.readerIdPromise_ = this.cid_.then((cid) => {
return cid.get(
{scope: 'amp-access', createCookieIfNotPresent: true},
{
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
},
consent
);
});
Expand Down
36 changes: 30 additions & 6 deletions extensions/amp-access/0.1/test/test-amp-access.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,11 @@ describes.fakeWin(
cidMock
.expects('get')
.withExactArgs(
{scope: 'amp-access', createCookieIfNotPresent: true},
{
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
},
env.sandbox.match(() => true)
)
.returns(Promise.resolve(result))
Expand Down Expand Up @@ -799,7 +803,11 @@ describes.fakeWin(
cidMock
.expects('get')
.withExactArgs(
{scope: 'amp-access', createCookieIfNotPresent: true},
{
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
},
env.sandbox.match(() => true)
)
.returns(Promise.resolve(result))
Expand Down Expand Up @@ -1293,7 +1301,11 @@ describes.fakeWin(
cidMock
.expects('get')
.withExactArgs(
{scope: 'amp-access', createCookieIfNotPresent: true},
{
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
},
env.sandbox.match(() => true)
)
.returns(Promise.resolve('reader1'))
Expand All @@ -1314,7 +1326,11 @@ describes.fakeWin(
cidMock
.expects('get')
.withExactArgs(
{scope: 'amp-access', createCookieIfNotPresent: true},
{
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
},
env.sandbox.match(() => true)
)
.returns(Promise.resolve('reader1'))
Expand Down Expand Up @@ -1353,7 +1369,11 @@ describes.fakeWin(
cidMock
.expects('get')
.withExactArgs(
{scope: 'amp-access', createCookieIfNotPresent: true},
{
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
},
env.sandbox.match(() => true)
)
.returns(Promise.resolve('reader1'))
Expand Down Expand Up @@ -1871,7 +1891,11 @@ describes.fakeWin(
cidMock
.expects('get')
.withExactArgs(
{scope: 'amp-access', createCookieIfNotPresent: true},
{
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
},
env.sandbox.match(() => true)
)
.returns(Promise.resolve(result))
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-consent/0.1/consent-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ export function expandConsentEndpointUrl(element, url) {
export function getConsentCID(node) {
return Services.cidForDoc(node).then((cid) => {
return cid.get(
{scope: CID_SCOPE, createCookieIfNotPresent: true},
{scope: CID_SCOPE, createCookieIfNotPresent: true, backupToStorage: true},
/** consent */ Promise.resolve()
);
});
Expand Down
4 changes: 4 additions & 0 deletions extensions/amp-experiment/0.1/test/test-variant.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'amp-experiment',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('name:123abc').returns(Promise.resolve(0.4));
Expand All @@ -279,6 +280,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'custom-scope',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('name:123abc').returns(Promise.resolve(0.4));
Expand All @@ -298,6 +300,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'amp-experiment',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('custom-group:123abc').returns(Promise.resolve(0.4));
Expand Down Expand Up @@ -325,6 +328,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'amp-experiment',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('name:123abc').returns(Promise.resolve(0.4));
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-experiment/0.1/variant.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ function getBucketTicket(ampdoc, group, opt_cidScope) {
{
scope: dev().assertString(opt_cidScope),
createCookieIfNotPresent: true,
backupToStorage: true,
},
Promise.resolve()
)
Expand Down
4 changes: 4 additions & 0 deletions extensions/amp-experiment/1.0/test/test-variant.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'amp-experiment',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('name:123abc').returns(Promise.resolve(0.4));
Expand All @@ -390,6 +391,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'custom-scope',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('name:123abc').returns(Promise.resolve(0.4));
Expand All @@ -415,6 +417,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'amp-experiment',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('custom-group:123abc').returns(Promise.resolve(0.4));
Expand Down Expand Up @@ -448,6 +451,7 @@ describes.sandboxed('allocateVariant', {}, (env) => {
.withArgs({
scope: 'amp-experiment',
createCookieIfNotPresent: true,
backupToStorage: true,
})
.returns(Promise.resolve('123abc'));
uniformStub.withArgs('name:123abc').returns(Promise.resolve(0.4));
Expand Down
1 change: 1 addition & 0 deletions extensions/amp-experiment/1.0/variant.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ function getBucketTicket(ampdoc, group, opt_cidScope) {
{
scope: dev().assertString(opt_cidScope),
createCookieIfNotPresent: true,
backupToStorage: true,
},
Promise.resolve()
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,11 @@ export class AmpStoryInteractive extends AMP.BaseElement {
if (!this.clientIdPromise_) {
this.clientIdPromise_ = this.clientIdService_.then((data) => {
return data.get(
{scope: 'amp-story', createCookieIfNotPresent: true},
{
scope: 'amp-story',
createCookieIfNotPresent: true,
backupToStorage: true,
},
/* consent */ Promise.resolve()
);
});
Expand Down
5 changes: 4 additions & 1 deletion extensions/amp-subscriptions/0.1/amp-subscriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,10 @@ export class SubscriptionService {
const scope =
'amp-access' + (serviceId == 'local' ? '' : '-' + serviceId);
readerIdPromise = this.cid_.then((cid) =>
cid.get({scope, createCookieIfNotPresent: true}, consent)
cid.get(
{scope, createCookieIfNotPresent: true, backupToStorage: true},
consent
)
);
this.serviceIdToReaderIdPromiseMap_[serviceId] = readerIdPromise;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, (env) => {
// Local service is default to "amp-access" scope.
scope: 'amp-access',
createCookieIfNotPresent: true,
backupToStorage: true,
});
});

Expand All @@ -272,6 +273,7 @@ describes.fakeWin('AmpSubscriptions', {amp: true}, (env) => {
expect(cidGet).to.be.calledOnce.calledWith({
scope: 'amp-access-service1',
createCookieIfNotPresent: true,
backupToStorage: true,
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ export class AmpUserNotification extends AMP.BaseElement {
// the notification or have the nagging notification sitting there
// (to never resolve).
return cid.get(
{scope: TAG, createCookieIfNotPresent: true},
{scope: TAG, createCookieIfNotPresent: true, backupToStorage: true},
Promise.resolve(),
this.dialogPromise_
);
Expand Down
1 change: 1 addition & 0 deletions spec/amp-var-substitutions.md
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ You can pass the following arguments into the Client ID variable like a function
- `cid scope` (Required): The namespace for the Client ID.
- `amp-user-notification id` (Optional): Use this argument to make the Client ID substitution dependent on the dismissal of a user notification shown to the visitor of the page. In amp-analytics, this is the same as using the [`data-consent-notification-id`](../extensions/amp-analytics/amp-analytics.md) attribute -- you may choose to use either one for the amp-analytics component.
- `cookie name` (Optional): The name of the fallback cookie when the document is not served by an AMP proxy. If not provided, `cid scope` will be used as the cookie name.
- `storage` (Optional): Whether to stored the AMP generated CID in Storage as backup, when the document is not served by an AMP proxy. Default value is `true`.
micajuine-ho marked this conversation as resolved.
Show resolved Hide resolved

#### Content Load Time

Expand Down
1 change: 1 addition & 0 deletions src/ad-cid.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export function getOrCreateAdCid(
scope: dev().assertString(clientIdScope),
createCookieIfNotPresent: true,
cookieName: opt_clientIdCookieName,
backupToStorage: true,
},
Promise.resolve(undefined)
)
Expand Down
Loading