Skip to content

Commit

Permalink
Merge pull request #1150 from cramforce/localstorage
Browse files Browse the repository at this point in the history
Delegate to viewer for cid generation if AMP is embedded.
  • Loading branch information
cramforce committed Dec 14, 2015
2 parents 49de3bc + fdbc162 commit acbca0b
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 2 deletions.
8 changes: 8 additions & 0 deletions build-system/tasks/presubmit-checks.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,14 @@ var forbiddenTerms = {
'test/functional/test-cid.js',
],
},
'getBaseCid': {
message: requiresReviewPrivacy,
whitelist: [
'src/service/cid-impl.js',
'src/viewer.js',
'test/functional/test-cid.js',
],
},
'cookie\\W': {
message: requiresReviewPrivacy,
whitelist: [
Expand Down
25 changes: 23 additions & 2 deletions src/service/cid-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {getCookie} from '../cookies';
import {getService} from '../service';
import {parseUrl} from '../url';
import {timer} from '../timer';
import {viewerFor} from '../viewer';
import {sha384Base64} from
'../../third_party/closure-library/sha384-generated';

Expand Down Expand Up @@ -182,6 +183,13 @@ function getBaseCid(cid, persistenceConsent) {
cid.baseCid_ = stored.cid;
return Promise.resolve(stored.cid);
}
// 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()) {
return viewer.getBaseCid();
}

// We need to make a new one.
const seed = getEntropy(win);
const newVal = cid.sha384Base64_(seed);
Expand All @@ -208,7 +216,15 @@ function store(win, cidString) {
const item = {};
item['time'] = timer.now();
item['cid'] = cidString;
win.localStorage.setItem('amp-cid', JSON.stringify(item));
const data = JSON.stringify(item);
try {
win.localStorage.setItem('amp-cid', data);
} catch (ignore) {
// Setting localStorage may fail. In practice we don't expect that to
// happen a lot (since we don't go anywhere near the quote, but
// in particular in Safari private browsing mode it always fails.
// In that case we just don't store anything, which is just fine.
}
}

/**
Expand All @@ -218,7 +234,12 @@ function store(win, cidString) {
* @return {!BaseCidInfo|undefined}
*/
function read(win) {
const val = win.localStorage.getItem('amp-cid');
let val;
try {
val = win.localStorage.getItem('amp-cid');
} catch (ignore) {
// If reading from localStorage fails, we assume it is empty.
}
if (!val) {
return undefined;
}
Expand Down
8 changes: 8 additions & 0 deletions src/viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,14 @@ export class Viewer {
return this.sendMessage_('popHistory', {stackIndex: stackIndex}, true);
}

/**
* Retrieves the Base CID from the viewer
* @return {!Promise<string>}
*/
getBaseCid() {
return this.sendMessage_('cid', undefined, true);
}

/**
* Requests AMP document to receive a message from Viewer.
* @param {string} eventType
Expand Down
30 changes: 30 additions & 0 deletions test/functional/test-cid.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ import {installCidService, getSourceOrigin, isProxyOrigin} from
'../../src/service/cid-impl';
import {parseUrl} from '../../src/url';
import {timer} from '../../src/timer';
import {viewerFor} from '../../src/viewer';
import * as sinon from 'sinon';

describe('cid', () => {

let isEmbedded;
let sandbox;
let clock;
let fakeWin;
Expand All @@ -32,6 +34,7 @@ describe('cid', () => {

beforeEach(() => {
let call = 1;
isEmbedded = false;
sandbox = sinon.sandbox.create();
clock = sandbox.useFakeTimers();
storage = {};
Expand Down Expand Up @@ -63,6 +66,13 @@ describe('cid', () => {
'amp-analytics': true
}
};
const viewer = viewerFor(fakeWin);
sandbox.stub(viewer, 'isEmbedded', function() {
return isEmbedded;
});
sandbox.stub(viewer, 'getBaseCid', function() {
return Promise.resolve('from-viewer');
});
installCidService(fakeWin);
return cidFor(fakeWin).then(c => {
cid = c;
Expand Down Expand Up @@ -162,6 +172,25 @@ describe('cid', () => {
'sha384(YYYhttp://www.origin.come2)');
});

it('should retrieve cid from viewer if embedded', () => {
isEmbedded = true;
return compare(
'e2',
'sha384(from-viewerhttp://www.origin.come2)');
});

it('should prefer value in storage if present', () => {
isEmbedded = true;
storage['amp-cid'] = JSON.stringify({
cid: 'in-storage',
time: timer.now(),
});
return compare(
'e2',
'sha384(in-storagehttp://www.origin.come2)');
});


it('should work without mocking', () => {
const win = {
location: {
Expand All @@ -174,6 +203,7 @@ describe('cid', () => {
};
win.__proto__ = window;
expect(win.location.href).to.equal('https://cdn.ampproject.org/v/www.origin.com/');
viewerFor(win).isEmbedded = () => false;
installCidService(win);
return cidFor(win).then(cid => {
return cid.get('foo', hasConsent).then(c1 => {
Expand Down

0 comments on commit acbca0b

Please sign in to comment.