Skip to content

Commit

Permalink
Amp Ad Fast Fetch allow amp-image prefetch (#11391)
Browse files Browse the repository at this point in the history 
* Amp Ad Fast Fetch allow amp-image prefetch

* type check fix

* feedback from @dvoytenko
  • Loading branch information
@keithwrightbos
keithwrightbos committed Sep 25, 2017
1 parent dca760f commit bb6cc2a
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 57 deletions.
14 changes: 11 additions & 3 deletions extensions/amp-a4a/0.1/amp-a4a.js
View file
Expand Up @@ -40,7 +40,7 @@ import {isArray, isObject, isEnumValue} from '../../../src/types';
import {utf8Decode} from '../../../src/utils/bytes';
import {getBinaryType, isExperimentOn} from '../../../src/experiments';
import {setStyle} from '../../../src/style';
import {assertHttpsUrl} from '../../../src/url';
import {assertHttpsUrl, isSecureUrl} from '../../../src/url';
import {parseJson} from '../../../src/json';
import {handleClick} from '../../../ads/alp/handler';
import {
Expand Down Expand Up @@ -106,7 +106,8 @@ export let SizeInfoDef;
/** @typedef {{
minifiedCreative: string,
customElementExtensions: !Array<string>,
customStylesheets: !Array<{href: string}>
customStylesheets: !Array<{href: string}>,
images: (Array<string>|undefined),
}} */
let CreativeMetaDataDef;

Expand Down Expand Up @@ -733,6 +734,9 @@ export class AmpA4A extends AMP.BaseElement {
// Preload any fonts.
(creativeMetaDataDef.customStylesheets || []).forEach(font =>
this.preconnect.preload(font.href));
// Preload any AMP images.
(creativeMetaDataDef.images || []).forEach(image =>
isSecureUrl(image) && this.preconnect.preload(image));
return creativeMetaDataDef;
})
.catch(error => {
Expand Down Expand Up @@ -1472,11 +1476,15 @@ export class AmpA4A extends AMP.BaseElement {
metaData.customStylesheets.forEach(stylesheet => {
if (!isObject(stylesheet) || !stylesheet['href'] ||
typeof stylesheet['href'] !== 'string' ||
!/^https:\/\//i.test(stylesheet['href'])) {
!isSecureUrl(stylesheet['href'])) {
throw new Error(errorMsg);
}
});
}
if (isArray(metaDataObj['images'])) {
// Load maximum of 5 images.
metaData.images = metaDataObj['images'].splice(0, 5);
}
// TODO(keithwrightbos): OK to assume ampRuntimeUtf16CharOffsets is before
// metadata as its in the head?
metaData.minifiedCreative =
Expand Down
136 changes: 82 additions & 54 deletions extensions/amp-a4a/0.1/test/test-amp-a4a.js
View file
Expand Up @@ -152,7 +152,7 @@ describe('amp-a4a', () => {

function buildCreativeString(opt_additionalInfo) {
const baseTestDoc = testFragments.minimalDocOneStyle;
const offsets = opt_additionalInfo || {};
const offsets = Object.assign({}, opt_additionalInfo || {});
offsets.ampRuntimeUtf16CharOffsets = [
baseTestDoc.indexOf('<style amp4ads-boilerplate'),
baseTestDoc.lastIndexOf('</script>') + '</script>'.length,
Expand Down Expand Up @@ -941,6 +941,35 @@ describe('amp-a4a', () => {
});
});
});
// TODO (keithwrightbos) - move into above e2e once signed creative with
// image within creative can be regenerated.
it('should prefetch amp images', () => {
return createIframePromise().then(fixture => {
setupForAdTesting(fixture);
fetchMock.getOnce(
TEST_URL + '&__amp_source_origin=about%3Asrcdoc', () => adResponse,
{name: 'ad'});
const doc = fixture.doc;
const a4aElement = createA4aElement(doc);
const a4a = new MockA4AImpl(a4aElement);
sandbox.stub(a4a, 'getAmpAdMetadata_', creative => {
const metaData = AmpA4A.prototype.getAmpAdMetadata_(creative);
metaData.images = ['https://prefetch.me.com?a=b', 'http://do.not.prefetch.me.com?c=d',
'https://prefetch.metoo.com?e=f'];
return metaData;
});
a4a.buildCallback();
a4a.onLayoutMeasure();
return a4a.layoutCallback().then(() => {
expect(doc.querySelector('link[rel=preload]' +
'[href="https://prefetch.me.com?a=b"]')).to.be.ok;
expect(doc.querySelector('link[rel=preload]' +
'[href="https://prefetch.metoo.com?e=f"]')).to.be.ok;
expect(doc.querySelector('link[rel=preload]' +
'[href="http://do.not.prefetch.me.com?c=d"]')).to.not.be.ok;
});
});
});
it('must not be position:fixed', () => {
return createIframePromise().then(fixture => {
setupForAdTesting(fixture);
Expand Down Expand Up @@ -1417,61 +1446,44 @@ describe('amp-a4a', () => {

describe('#getAmpAdMetadata_', () => {
let a4a;
let metaData;
beforeEach(() => {
metaData = {
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{href: 'https://fonts.com/css?helloworld'},
],
images: ['https://some.image.com/a=b', 'https://other.image.com'],
};
return createIframePromise().then(fixture => {
setupForAdTesting(fixture);
a4a = new MockA4AImpl(createA4aElement(fixture.doc));
return fixture;
});
});
it('should parse metadata', () => {
const actual = a4a.getAmpAdMetadata_(buildCreativeString({
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{href: 'https://fonts.com/css?helloworld'},
],
}));
const expected = {
const actual = a4a.getAmpAdMetadata_(buildCreativeString(metaData));
const expected = Object.assign(metaData, {
minifiedCreative: testFragments.minimalDocOneStyleSrcDoc,
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{href: 'https://fonts.com/css?helloworld'},
],
};
});
expect(actual).to.deep.equal(expected);
});
// TODO(levitzky) remove the following two tests after metadata bug is
// fixed.
it('should parse metadata with wrong opening tag', () => {
const creative = buildCreativeString({
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{href: 'https://fonts.com/css?helloworld'},
],
}).replace('<script type="application/json" amp-ad-metadata>',
const creative = buildCreativeString(metaData).replace(
'<script type="application/json" amp-ad-metadata>',
'<script type=application/json amp-ad-metadata>');
const actual = a4a.getAmpAdMetadata_(creative);
const expected = {
const expected = Object.assign({
minifiedCreative: testFragments.minimalDocOneStyleSrcDoc,
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{href: 'https://fonts.com/css?helloworld'},
],
};
}, metaData);
expect(actual).to.deep.equal(expected);
});
it('should return null if metadata opening tag is (truly) wrong', () => {
const creative = buildCreativeString({
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{href: 'https://fonts.com/css?helloworld'},
],
}).replace('<script type="application/json" amp-ad-metadata>',
const creative = buildCreativeString(metaData).replace(
'<script type="application/json" amp-ad-metadata>',
'<script type=application/json" amp-ad-metadata>');
expect(a4a.getAmpAdMetadata_(creative)).to.be.null;
});
Expand All @@ -1485,28 +1497,44 @@ describe('amp-a4a', () => {
baseTestDoc.slice(splicePoint))).to.be.null;
});
it('should return null if invalid extensions', () => {
expect(a4a.getAmpAdMetadata_(buildCreativeString({
customElementExtensions: 'amp-vine',
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{href: 'https://fonts.com/css?helloworld'},
],
}))).to.be.null;
metaData.customElementExtensions = 'amp-vine';
expect(a4a.getAmpAdMetadata_(buildCreativeString(metaData))).to.be.null;
});
it('should return null if non-array stylesheets', () => {
expect(a4a.getAmpAdMetadata_(buildCreativeString({
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: 'https://fonts.googleapis.com/css?foobar',
}))).to.be.null;
metaData.customStylesheets = 'https://fonts.googleapis.com/css?foobar';
expect(a4a.getAmpAdMetadata_(buildCreativeString(metaData))).to.be.null;
});
it('should return null if invalid stylesheet object', () => {
expect(a4a.getAmpAdMetadata_(buildCreativeString({
customElementExtensions: ['amp-vine', 'amp-vine', 'amp-vine'],
customStylesheets: [
{href: 'https://fonts.googleapis.com/css?foobar'},
{foo: 'https://fonts.com/css?helloworld'},
],
}))).to.be.null;
metaData.customStylesheets = [
{href: 'https://fonts.googleapis.com/css?foobar'},
{foo: 'https://fonts.com/css?helloworld'},
];
expect(a4a.getAmpAdMetadata_(buildCreativeString(metaData))).to.be.null;
});
it('should not include amp images if not an array', () => {
metaData.images = 'https://foo.com';
const actual = a4a.getAmpAdMetadata_(buildCreativeString(metaData));
const expected = Object.assign({
minifiedCreative: testFragments.minimalDocOneStyleSrcDoc,
}, metaData);
delete expected.images;
expect(actual).to.deep.equal(expected);
});
it('should tolerate missing images', () => {
delete metaData.images;
const actual = a4a.getAmpAdMetadata_(buildCreativeString(metaData));
const expected = Object.assign({
minifiedCreative: testFragments.minimalDocOneStyleSrcDoc,
}, metaData);
delete expected.images;
expect(actual).to.deep.equal(expected);
});
it('should limit to 5 images', () => {
while (metaData.images.length < 10) {
metaData.images.push('https://another.image.com?abc=def');
}
expect(a4a.getAmpAdMetadata_(buildCreativeString(metaData)).images.length)
.to.equal(5);
});
// FAILURE cases here
});
Expand Down

0 comments on commit bb6cc2a

Please sign in to comment.