Skip to content

Commit

Permalink
Separate out implementaton of document-info and remove dependency of … (
Browse files Browse the repository at this point in the history
ampproject#5864)

Together this improves code size for `3p-frame` based and some other extensions.

Part of ampproject#5792
  • Loading branch information
cramforce authored and Vanessa Pasque committed Dec 22, 2016
1 parent 6e3748d commit da1e506
Show file tree
Hide file tree
Showing 13 changed files with 202 additions and 143 deletions.
6 changes: 0 additions & 6 deletions extensions/amp-ad/0.1/test/test-amp-ad-3p-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,6 @@ describe('amp-ad-3p-impl', () => {
});
});

it('should reject if no canonical URL provided', () => {
win.document.head.innerHTML = '';
return expect(ad3p.layoutCallback())
.to.eventually.be.rejectedWith('canonical');
});

it('should reject if no type attribute provided', () => {
ad3p.element.removeAttribute('type');
return expect(ad3p.layoutCallback())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ describe('amp-install-serviceworker', () => {
};
resetServiceForTesting(window, 'documentInfo');
getServiceForDoc(document, 'documentInfo', () => {
return documentInfo;
return {
get: () => documentInfo,
};
});
whenVisible = Promise.resolve();
getService(win, 'viewer', () => {
Expand Down
17 changes: 12 additions & 5 deletions src/3p-frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import {dev, user} from './log';
import {documentInfoForDoc} from './document-info';
import {getLengthNumeral} from '../src/layout';
import {getService} from './service';
import {tryParseJson} from './json';
import {getMode} from './mode';
import {getModeObject} from './mode-object';
Expand Down Expand Up @@ -192,16 +191,24 @@ export function preloadBootstrap(window, preconnect) {
* @visibleForTesting
*/
export function getBootstrapBaseUrl(parentWindow, opt_strictForUnitTest) {
return getService(self, 'bootstrapBaseUrl', () => {
return getCustomBootstrapBaseUrl(parentWindow, opt_strictForUnitTest) ||
getDefaultBootstrapBaseUrl(parentWindow);
});
// The value is cached in a global variable called `bootstrapBaseUrl`;
const bootstrapBaseUrl = parentWindow.bootstrapBaseUrl;
if (bootstrapBaseUrl) {
return bootstrapBaseUrl;
}
return parentWindow.bootstrapBaseUrl =
getCustomBootstrapBaseUrl(parentWindow, opt_strictForUnitTest)
|| getDefaultBootstrapBaseUrl(parentWindow);
}

export function setDefaultBootstrapBaseUrlForTesting(url) {
overrideBootstrapBaseUrl = url;
}

export function resetBootstrapBaseUrlForTesting(win) {
win.bootstrapBaseUrl = undefined;
}

/**
* Returns the default base URL for 3p bootstrap iframes.
* @param {!Window} parentWindow
Expand Down
60 changes: 4 additions & 56 deletions src/document-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,66 +14,14 @@
* limitations under the License.
*/

import {getServiceForDoc} from './service';
import {parseUrl, getSourceUrl} from './url';
import {user} from './log';


/**
* Properties:
* - url: The doc's url.
* - sourceUrl: the source url of an amp document.
* - canonicalUrl: The doc's canonical.
* - pageViewId: Id for this page view. Low entropy but should be unique
* for concurrent page views of a user().
*
* @typedef {{
* url: string,
* sourceUrl: string,
* canonicalUrl: string,
* pageViewId: string,
* }}
*/
export let DocumentInfoDef;
import {getExistingServiceForDoc} from './service';


/**
* @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc
* @return {!DocumentInfoDef} Info about the doc
* @return {!./service/document-info-impl.DocumentInfoDef} Info about the doc
*/
export function documentInfoForDoc(nodeOrDoc) {
return /** @type {!DocumentInfoDef} */ (getServiceForDoc(nodeOrDoc,
'documentInfo', ampdoc => {
const rootNode = ampdoc.getRootNode();
let canonicalUrl = rootNode && rootNode.AMP
&& rootNode.AMP.canonicalUrl;
if (!canonicalUrl) {
const canonicalTag = user().assert(
rootNode.querySelector('link[rel=canonical]'),
'AMP files are required to have a <link rel=canonical> tag.');
canonicalUrl = parseUrl(canonicalTag.href).href;
}
const pageViewId = getPageViewId(ampdoc.win);
const res = {
get sourceUrl() {
return getSourceUrl(ampdoc.getUrl());
},
canonicalUrl,
pageViewId,
};
return res;
}));
}



/**
* Returns a relatively low entropy random string.
* This should be called once per window and then cached for subsequent
* access to the same value to be persistent per page.
* @param {!Window} win
* @return {string}
*/
function getPageViewId(win) {
return String(Math.floor(win.Math.random() * 10000));
return /** @type {!./service/document-info-impl.DocInfo} */ (
getExistingServiceForDoc(nodeOrDoc, 'documentInfo')).get();
}
2 changes: 2 additions & 0 deletions src/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
} from './shadow-embed';
import {getMode} from './mode';
import {installActionServiceForDoc} from './service/action-impl';
import {installDocumentInfoServiceForDoc} from './service/document-info-impl';
import {installGlobalSubmitListenerForDoc} from './document-submit';
import {extensionsFor} from './extensions';
import {installHistoryServiceForDoc} from './service/history-impl';
Expand Down Expand Up @@ -107,6 +108,7 @@ export function installRuntimeServices(global) {
* @param {!Object<string, string>=} opt_initParams
*/
export function installAmpdocServices(ampdoc, opt_initParams) {
installDocumentInfoServiceForDoc(ampdoc);
installViewerServiceForDoc(ampdoc, opt_initParams);
installViewportServiceForDoc(ampdoc);
installHistoryServiceForDoc(ampdoc);
Expand Down
97 changes: 97 additions & 0 deletions src/service/document-info-impl.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/**
* Copyright 2015 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import {fromClassForDoc} from '../service';
import {parseUrl, getSourceUrl} from '../url';


/**
* Properties:
* - url: The doc's url.
* - sourceUrl: the source url of an amp document.
* - canonicalUrl: The doc's canonical.
* - pageViewId: Id for this page view. Low entropy but should be unique
* for concurrent page views of a user().
*
* @typedef {{
* sourceUrl: string,
* canonicalUrl: string,
* pageViewId: string,
* }}
*/
export let DocumentInfoDef;


/**
* @param {!Node|!./ampdoc-impl.AmpDoc} nodeOrDoc
* @return {!DocInfo} Info about the doc
*/
export function installDocumentInfoServiceForDoc(nodeOrDoc) {
return fromClassForDoc(nodeOrDoc, 'documentInfo', DocInfo);
}


export class DocInfo {
/**
* @param {!./ampdoc-impl.AmpDoc} ampdoc
*/
constructor(ampdoc) {
/** @private @const */
this.ampdoc_ = ampdoc;
/** @private {?DocumentInfoDef} */
this.info_ = null;
}

/** @return {!DocumentInfoDef} */
get() {
if (this.info_) {
return this.info_;
}
const ampdoc = this.ampdoc_;
const url = ampdoc.getUrl();
const sourceUrl = getSourceUrl(url);
const rootNode = ampdoc.getRootNode();
let canonicalUrl = rootNode && rootNode.AMP
&& rootNode.AMP.canonicalUrl;
if (!canonicalUrl) {
const canonicalTag = rootNode.querySelector('link[rel=canonical]');
canonicalUrl = canonicalTag
? parseUrl(canonicalTag.href).href
: sourceUrl;
}
const pageViewId = getPageViewId(ampdoc.win);
return this.info_ = {
/** @return {string} */
get sourceUrl() {
return getSourceUrl(ampdoc.getUrl());
},
canonicalUrl,
pageViewId,
};
}
}


/**
* Returns a relatively low entropy random string.
* This should be called once per window and then cached for subsequent
* access to the same value to be persistent per page.
* @param {!Window} win
* @return {string}
*/
function getPageViewId(win) {
return String(Math.floor(win.Math.random() * 10000));
}
2 changes: 1 addition & 1 deletion src/service/url-replacements-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ export class UrlReplacements {

/**
* Resolves the value via document info.
* @param {function(!../document-info.DocumentInfoDef):T} getter
* @param {function(!./document-info-impl.DocumentInfoDef):T} getter
* @return {T}
* @template T
*/
Expand Down
6 changes: 3 additions & 3 deletions test/functional/test-3p-frame.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ import {
getSubDomain,
preloadBootstrap,
resetCountForTesting,
resetBootstrapBaseUrlForTesting,
} from '../../src/3p-frame';
import {documentInfoForDoc} from '../../src/document-info';
import {loadPromise} from '../../src/event-helper';
import {preconnectForElement} from '../../src/preconnect';
import {resetServiceForTesting} from '../../src/service';
import {validateData} from '../../3p/3p';
import {viewerForDoc} from '../../src/viewer';
import * as sinon from 'sinon';
Expand All @@ -46,8 +46,8 @@ describe('3p-frame', () => {
});

afterEach(() => {
resetBootstrapBaseUrlForTesting(window);
sandbox.restore();
resetServiceForTesting(window, 'bootstrapBaseUrl');
resetCountForTesting();
const m = document.querySelector(
'[name="amp-3p-iframe-src"]');
Expand Down Expand Up @@ -321,7 +321,7 @@ describe('3p-frame', () => {

container.appendChild(div);
const name = getIframe(window, div).name;
resetServiceForTesting(window, 'bootstrapBaseUrl');
resetBootstrapBaseUrlForTesting(window);
resetCountForTesting();
const newName = getIframe(window, div).name;
expect(name).to.match(/d-\d+.ampproject.net__ping__0/);
Expand Down
3 changes: 3 additions & 0 deletions test/functional/test-document-click.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import {installTimerService} from '../../src/service/timer-impl';
import {
installUrlReplacementsServiceForDoc,
} from '../../src/service/url-replacements-impl';
import {installDocumentInfoServiceForDoc,} from
'../../src/service/document-info-impl';
import * as sinon from 'sinon';
import {toggleExperiment} from '../../src/experiments';

Expand Down Expand Up @@ -95,6 +97,7 @@ describe('test-document-click onDocumentElementClick_', () => {
push: () => {},
};
installTimerService(win);
installDocumentInfoServiceForDoc(ampdoc);
installUrlReplacementsServiceForDoc(ampdoc);
});

Expand Down
18 changes: 8 additions & 10 deletions test/functional/test-document-info.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

import {createIframePromise} from '../../testing/iframe';
import {documentInfoForDoc} from '../../src/document-info';
import {installDocumentInfoServiceForDoc,} from
'../../src/service/document-info-impl';
import {installDocService} from '../../src/service/ampdoc-impl';
import * as sinon from 'sinon';

Expand All @@ -38,7 +40,10 @@ describe('document-info', () => {
link.setAttribute('rel', 'canonical');
iframe.doc.head.appendChild(link);
}
installDocService(iframe.win, true);
const win = iframe.win;
installDocService(win, true);
sandbox.stub(win.Math, 'random', () => 0.123456789);
installDocumentInfoServiceForDoc(win.document);
return iframe.win;
});
}
Expand All @@ -63,6 +68,7 @@ describe('document-info', () => {
};
win.document.defaultView = win;
installDocService(win, true);
installDocumentInfoServiceForDoc(win.document);
expect(documentInfoForDoc(win.document).sourceUrl).to.equal(
'http://www.origin.com/foo/?f=0');
});
Expand All @@ -80,6 +86,7 @@ describe('document-info', () => {
};
win.document.defaultView = win;
installDocService(win, true);
installDocumentInfoServiceForDoc(win.document);
expect(documentInfoForDoc(win.document).sourceUrl).to.equal(
'http://www.origin.com/foo/?f=0');
win.location.href = 'https://cdn.ampproject.org/v/www.origin.com/foo/?f=1';
Expand All @@ -89,7 +96,6 @@ describe('document-info', () => {

it('should provide the pageViewId', () => {
return getWin('https://twitter.com/').then(win => {
sandbox.stub(win.Math, 'random', () => 0.123456789);
expect(documentInfoForDoc(win.document).pageViewId).to.equal('1234');
expect(documentInfoForDoc(win.document).pageViewId).to.equal('1234');
});
Expand All @@ -101,12 +107,4 @@ describe('document-info', () => {
'http://localhost:' + location.port + '/foo.html');
});
});

it('should throw if no canonical is available.', () => {
return getWin(null).then(win => {
expect(() => {
documentInfoForDoc(win.document).canonicalUrl;
}).to.throw(/AMP files are required to have a/);
});
});
});
10 changes: 7 additions & 3 deletions test/functional/test-performance.js
Original file line number Diff line number Diff line change
Expand Up @@ -534,9 +534,13 @@ describe('performance', () => {
perf.coreServicesAvailable();
resetServiceForTesting(window, 'documentInfo');
const info = {
canonicalUrl: 'https://foo.bar/baz',
pageViewId: 12345,
sourceUrl: 'https://hello.world/baz/#development',
get: () => {
return {
canonicalUrl: 'https://foo.bar/baz',
pageViewId: 12345,
sourceUrl: 'https://hello.world/baz/#development',
};
},
};
getService(window, 'documentInfo', () => info);

Expand Down
Loading

0 comments on commit da1e506

Please sign in to comment.