Skip to content

Commit

Permalink
Refector baseCid in viewer into cid-impl (#7111)
Browse files Browse the repository at this point in the history
  • Loading branch information
@muxin
muxin committed Jan 20, 2017
1 parent e88a6e0 commit 1b2453c
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 125 deletions.
2 changes: 2 additions & 0 deletions build-system/tasks/presubmit-checks.js
View file
Expand Up @@ -288,6 +288,7 @@ var forbiddenTerms = {
'src/service/viewer-impl.js',
'src/service/storage-impl.js',
'src/service/history-impl.js',
'extensions/amp-analytics/0.1/cid-impl.js',
'extensions/amp-access/0.1/login-dialog.js',
'extensions/amp-access/0.1/signin.js',
],
Expand Down Expand Up @@ -343,6 +344,7 @@ var forbiddenTerms = {
whitelist: [
'src/service/viewer-impl.js',
'src/inabox/inabox-viewer.js',
'extensions/amp-analytics/0.1/cid-impl.js',
],
},
'eval\\(': {
Expand Down
44 changes: 39 additions & 5 deletions extensions/amp-analytics/0.1/cid-impl.js
View file
Expand Up @@ -33,8 +33,11 @@ import {isIframed} from '../../../src/dom';
import {getCryptoRandomBytesArray} from '../../../src/utils/bytes';
import {viewerForDoc} from '../../../src/viewer';
import {cryptoFor} from '../../../src/crypto';
import {user} from '../../../src/log';
import {tryParseJson} from '../../../src/json';
import {timerFor} from '../../../src/timer';
import {user, dev} from '../../../src/log';

const TAG_ = 'Cid';

const ONE_DAY_MILLIS = 24 * 3600 * 1000;

Expand Down Expand Up @@ -278,12 +281,11 @@ function getBaseCid(cid, persistenceConsent) {
* @param {string} cidString Actual cid string to store.
*/
function store(win, persistenceConsent, cidString) {
const viewer = viewerForDoc(win.document);
// TODO(lannka, #4457): ideally, we should check if viewer has the capability
// of CID storage, rather than if it is iframed.
if (isIframed(win)) {
// If we are being embedded, try to save the base cid to the viewer.
viewer.baseCid(createCidData(cidString));
viewerBaseCid(win, createCidData(cidString));
} else {
// To use local storage, we need user's consent.
persistenceConsent.then(() => {
Expand All @@ -299,6 +301,39 @@ function store(win, persistenceConsent, cidString) {
}
}

/**
* Get/set the Base CID from/to the viewer.
* @param {string=} opt_data Stringified JSON object {cid, time}.
* @return {!Promise<string|undefined>}
*/
export function viewerBaseCid(win, opt_data) {
const viewer = viewerForDoc(win.document);
return viewer.isTrustedViewer().then(trusted => {
if (!trusted) {
return undefined;
}
const cidPromise = viewer.sendMessageAwaitResponse('cid', opt_data)
.then(data => {
// For backward compatibility: #4029
if (data && !tryParseJson(data)) {
return JSON.stringify({
time: Date.now(), // CID returned from old API is always fresh
cid: data,
});
}
return data;
});
// Getting the CID may take some time (waits for JS file to
// load, might hit GC), but we do not wait indefinitely. Typically
// it should resolve in milli seconds.
return timerFor(win).timeoutPromise(10000, cidPromise, 'base cid')
.catch(error => {
dev().error(TAG_, error);
return undefined;
});
});
}

/**
* Creates a JSON object that contains the given CID and the current time as
* a timestamp.
Expand Down Expand Up @@ -326,11 +361,10 @@ function read(win) {
} catch (ignore) {
// If reading from localStorage fails, we assume it is empty.
}
const viewer = viewerForDoc(win.document);
let dataPromise = Promise.resolve(data);
if (!data && isIframed(win)) {
// If we are being embedded, try to get the base cid from the viewer.
dataPromise = viewer.baseCid();
dataPromise = viewerBaseCid(win);
}
return dataPromise.then(data => {
if (!data) {
Expand Down
34 changes: 0 additions & 34 deletions src/service/viewer-impl.js
View file
Expand Up @@ -30,7 +30,6 @@ import {
import {timerFor} from '../timer';
import {reportError} from '../error';
import {VisibilityState} from '../visibility-state';
import {tryParseJson} from '../json';

const TAG_ = 'Viewer';
const SENTINEL_ = '__AMP__';
Expand Down Expand Up @@ -700,39 +699,6 @@ export class Viewer {
return observable.add(handler);
}

/**
* Get/set the Base CID from/to the viewer.
* @param {string=} opt_data Stringified JSON object {cid, time}.
* @return {!Promise<string|undefined>}
* TODO: move this to cid-impl
*/
baseCid(opt_data) {
return this.isTrustedViewer().then(trusted => {
if (!trusted) {
return undefined;
}
const cidPromise = this.sendMessageAwaitResponse('cid', opt_data)
.then(data => {
// For backward compatibility: #4029
if (data && !tryParseJson(data)) {
return JSON.stringify({
time: Date.now(), // CID returned from old API is always fresh
cid: data,
});
}
return data;
});
// Getting the CID may take some time (waits for JS file to
// load, might hit GC), but we do not wait indefinitely. Typically
// it should resolve in milli seconds.
return timerFor(this.win).timeoutPromise(10000, cidPromise, 'base cid')
.catch(error => {
dev().error(TAG_, error);
return undefined;
});
});
}

/**
* Requests AMP document to receive a message from Viewer.
* @param {string} eventType
Expand Down
84 changes: 70 additions & 14 deletions test/functional/test-cid.js
View file
Expand Up @@ -18,6 +18,7 @@ import {cidFor} from '../../src/cid';
import {
installCidService,
getProxySourceOrigin,
viewerBaseCid,
} from '../../extensions/amp-analytics/0.1/cid-impl';
import {installCryptoService, Crypto,}
from '../../extensions/amp-analytics/0.1/crypto-impl';
Expand All @@ -43,12 +44,15 @@ describe('cid', () => {
let clock;
let fakeWin;
let ampdoc;
let viewer;
let storage;
let viewerStorage;
let cid;
let crypto;
let viewerBaseCidStub;
let viewerSendMessageStub;
let whenFirstVisible;
let trustedViewer;
let shouldSendMessageTimeout;

const hasConsent = Promise.resolve();
const timer = timerFor(window);
Expand All @@ -58,6 +62,8 @@ describe('cid', () => {
sandbox = sinon.sandbox.create();
clock = sandbox.useFakeTimers();
whenFirstVisible = Promise.resolve();
trustedViewer = true;
shouldSendMessageTimeout = false;
storage = {};
viewerStorage = null;
fakeWin = {
Expand Down Expand Up @@ -104,16 +110,25 @@ describe('cid', () => {
return Promise.resolve();
});

const viewer = installViewerServiceForDoc(ampdoc);
viewer = installViewerServiceForDoc(ampdoc);
sandbox.stub(viewer, 'whenFirstVisible', function() {
return whenFirstVisible;
});
viewerBaseCidStub = sandbox.stub(viewer, 'baseCid', function(opt_data) {
if (opt_data) {
viewerStorage = opt_data;
}
return Promise.resolve(viewerStorage || undefined);
});
sandbox.stub(viewer, 'isTrustedViewer',
() => Promise.resolve(trustedViewer));
viewerSendMessageStub = sandbox.stub(viewer, 'sendMessageAwaitResponse',
(eventType, opt_data) => {
if (eventType != 'cid') {
return Promise.reject();
}
if (shouldSendMessageTimeout) {
return timer.promise(15000);
}
if (opt_data) {
viewerStorage = opt_data;
}
return Promise.resolve(viewerStorage || undefined);
});

return Promise
.all([installCidService(fakeWin), installCryptoService(fakeWin)])
Expand Down Expand Up @@ -226,24 +241,65 @@ describe('cid', () => {
compare('e1', `sha384(${expectedBaseCid}http://www.origin.come1)`),
compare('e2', `sha384(${expectedBaseCid}http://www.origin.come2)`),
]).then(() => {
expect(viewerBaseCidStub).to.be.calledOnce;
expect(viewerBaseCidStub).to.not.be.calledWith(sinon.match.string);
expect(viewerSendMessageStub).to.be.calledOnce;
expect(viewerSendMessageStub).to.be.calledWith('cid');

// Ensure it's called only once since we cache it in memory.
return compare('e3', `sha384(${expectedBaseCid}http://www.origin.come3)`);
}).then(() => {
expect(viewerBaseCidStub).to.be.calledOnce;
expect(viewerBaseCidStub).to.not.be.calledWith(sinon.match.string);
expect(viewerSendMessageStub).to.be.calledOnce;
return expect(cid.baseCid_).to.eventually.equal(expectedBaseCid);
});
});

it('should read from viewer storage if embedded and convert cid to ' +
'new format', () => {
fakeWin.parent = {};
const expectedBaseCid = 'from-viewer';
// baseCid returned by legacy API
viewerStorage = expectedBaseCid;
return Promise.all([
compare('e1', `sha384(${expectedBaseCid}http://www.origin.come1)`),
compare('e2', `sha384(${expectedBaseCid}http://www.origin.come2)`),
]).then(() => {
expect(viewerSendMessageStub).to.be.calledOnce;
expect(viewerSendMessageStub).to.be.calledWith('cid');
});
});

it('should not read from untrusted viewer', () => {
fakeWin.parent = {};
trustedViewer = false;
const viewerBaseCid = 'from-viewer';
viewerStorage = JSON.stringify({
time: 0,
cid: viewerBaseCid,
});
return Promise.all([
compare('e1', 'sha384(sha384([1,2,3,0,0,0,0,0,0,0,0,0,0,0,0,15])http://www.origin.come1)'),
compare('e2', 'sha384(sha384([1,2,3,0,0,0,0,0,0,0,0,0,0,0,0,15])http://www.origin.come2)'),
]).then(() => {
expect(viewerSendMessageStub).to.not.be.called;
});
});

it('should time out reading from viewer', () => {
shouldSendMessageTimeout = true;
const promise = viewerBaseCid(fakeWin);
return Promise.resolve().then(() => {
clock.tick(10001);
return promise;
}).then(cid => {
expect(cid).to.be.undefined;
});
});

it('should store to viewer storage if embedded', () => {
fakeWin.parent = {};
const expectedBaseCid = 'sha384([1,2,3,0,0,0,0,0,0,0,0,0,0,0,0,15])';
return compare('e2', `sha384(${expectedBaseCid}http://www.origin.come2)`)
.then(() => {
expect(viewerBaseCidStub).to.be.calledWith(JSON.stringify({
expect(viewerSendMessageStub).to.be.calledWith('cid', JSON.stringify({
time: 0,
cid: expectedBaseCid,
}));
Expand All @@ -252,7 +308,7 @@ describe('cid', () => {
return compare('e3', `sha384(${expectedBaseCid}http://www.origin.come3)`);
})
.then(() => {
expect(viewerBaseCidStub).to.be.calledWith(sinon.match.string);
expect(viewerSendMessageStub).to.be.calledWith(sinon.match.string);
return expect(cid.baseCid_).to.eventually.equal(expectedBaseCid);
});
});
Expand Down
72 changes: 0 additions & 72 deletions test/functional/test-viewer.js
View file
Expand Up @@ -18,7 +18,6 @@ import {Viewer} from '../../src/service/viewer-impl';
import {dev} from '../../src/log';
import {installDocService} from '../../src/service/ampdoc-impl';
import {installPlatformService} from '../../src/service/platform-impl';
import {timerFor} from '../../src/timer';
import {installTimerService} from '../../src/service/timer-impl';
import * as sinon from 'sinon';

Expand Down Expand Up @@ -387,77 +386,6 @@ describe('Viewer', () => {
});
});

describe('baseCid', () => {
const cidData = JSON.stringify({
time: 100,
cid: 'cid-123',
});
let trustedViewer;
let persistedCidData;
let shouldTimeout;

beforeEach(() => {
shouldTimeout = false;
clock.tick(100);
trustedViewer = true;
persistedCidData = cidData;
sandbox.stub(viewer, 'isTrustedViewer',
() => Promise.resolve(trustedViewer));
sandbox.stub(viewer, 'sendMessageAwaitResponse', (message, payload) => {
if (message != 'cid') {
return Promise.reject();
}
if (shouldTimeout) {
return timerFor(window).promise(15000);
}
if (payload) {
persistedCidData = payload;
}
return Promise.resolve(persistedCidData);
});
});

it('should return CID', () => {
const p = expect(viewer.baseCid()).to.eventually.equal(cidData);
p.then(() => {
// This should not trigger a timeout.
clock.tick(100000);
});
return p;
});

it('should not request cid for untrusted viewer', () => {
trustedViewer = false;
return expect(viewer.baseCid()).to.eventually.be.undefined;
});

it('should convert CID returned by legacy API to new format', () => {
persistedCidData = 'cid-123';
return expect(viewer.baseCid()).to.eventually.equal(cidData);
});

it('should send message to store cid', () => {
const newCidData = JSON.stringify({time: 101, cid: 'cid-456'});
return expect(viewer.baseCid(newCidData))
.to.eventually.equal(newCidData);
});

it('should time out', () => {
shouldTimeout = true;
const p = expect(viewer.baseCid()).to.eventually.be.undefined;
Promise.resolve().then(() => {
clock.tick(9999);
Promise.resolve().then(() => {
clock.tick(1);
});
});
return p.then(() => {
// Ticked 100 at start.
expect(Date.now()).to.equal(10100);
});
});
});

describe('Messaging not embedded', () => {

it('should not expect messaging', () => {
Expand Down

0 comments on commit 1b2453c

Please sign in to comment.