diff --git a/src/3p-frame.js b/src/3p-frame.js index 2a5e06b540b0..51ab5b3c9f69 100644 --- a/src/3p-frame.js +++ b/src/3p-frame.js @@ -30,7 +30,7 @@ import {viewerFor} from './viewer'; /** @type {!Object} Number of 3p frames on the for that type. */ -const count = {}; +let count = {}; /** @@ -103,13 +103,15 @@ export function getIframe(parentWindow, element, opt_type) { if (!count[attributes.type]) { count[attributes.type] = 0; } - iframe.name = 'frame_' + attributes.type + '_' + count[attributes.type]++; + const baseUrl = getBootstrapBaseUrl(parentWindow); + const host = parseUrl(baseUrl).hostname; // Pass ad attributes to iframe via the fragment. - const src = getBootstrapBaseUrl(parentWindow) + '#' + - JSON.stringify(attributes); + const src = baseUrl + '#' + JSON.stringify(attributes); + const name = host + '_' + attributes.type + '_' + count[attributes.type]++; iframe.src = src; + iframe.name = name; iframe.ampLocation = parseUrl(src); iframe.width = attributes.width; iframe.height = attributes.height; @@ -190,17 +192,15 @@ export function getBootstrapBaseUrl(parentWindow, opt_strictForUnitTest) { * @return {string} */ function getDefaultBootstrapBaseUrl(parentWindow) { - let url = - 'https://' + getSubDomain(parentWindow) + - '.ampproject.net/$internalRuntimeVersion$/frame.html'; if (getMode().localDev) { - url = 'http://ads.localhost:' + + return 'http://ads.localhost:' + (parentWindow.location.port || parentWindow.parent.location.port) + '/dist.3p/current' + (getMode().minified ? '-min/frame' : '/frame.max') + '.html'; } - return url; + return 'https://' + getSubDomain(parentWindow) + + '.ampproject.net/$internalRuntimeVersion$/frame.html'; } /** @@ -250,6 +250,14 @@ function getCustomBootstrapBaseUrl(parentWindow, opt_strictForUnitTest) { '3p iframe url must not be on the same origin as the current doc' + 'ument %s (%s) in element %s. See https://github.com/ampproject/amphtml' + '/blob/master/spec/amp-iframe-origin-policy.md for details.', url, - parseUrl(url).origin, meta); + parsed.origin, meta); return url + '?$internalRuntimeVersion$'; } + +/** + * Resets the count of each 3p frame type + * @visibleForTesting + */ +export function resetCountForTesting() { + count = {}; +} diff --git a/test/functional/test-3p-frame.js b/test/functional/test-3p-frame.js index 1e2ef60a20f7..575508fed729 100644 --- a/test/functional/test-3p-frame.js +++ b/test/functional/test-3p-frame.js @@ -14,8 +14,14 @@ * limitations under the License. */ -import {addDataAndJsonAttributes_, getIframe, getBootstrapBaseUrl, - getSubDomain, prefetchBootstrap} from '../../src/3p-frame'; +import { + addDataAndJsonAttributes_, + getIframe, + getBootstrapBaseUrl, + getSubDomain, + prefetchBootstrap, + resetCountForTesting, +} from '../../src/3p-frame'; import {validateData} from '../../src/3p'; import {documentInfoFor} from '../../src/document-info'; import {loadPromise} from '../../src/event-helper'; @@ -36,6 +42,7 @@ describe('3p-frame', () => { afterEach(() => { sandbox.restore(); resetServiceForTesting(window, 'bootstrapBaseUrl'); + resetCountForTesting(); setModeForTesting(null); const m = document.querySelector( '[name="amp-3p-iframe-src"]'); @@ -238,4 +245,33 @@ describe('3p-frame', () => { }; expect(getSubDomain(fakeWin)).to.equal('d-5670'); }); + + it('uses a unique name based on domain', () => { + const viewerMock = sandbox.mock(viewerFor(window)); + viewerMock.expects('getUnconfirmedReferrerUrl') + .returns('http://acme.org/').twice(); + + setModeForTesting({}); + const link = document.createElement('link'); + link.setAttribute('rel', 'canonical'); + link.setAttribute('href', 'https://foo.bar/baz'); + document.head.appendChild(link); + + const div = document.createElement('div'); + div.setAttribute('type', '_ping_'); + div.getLayoutBox = function() { + return { + width: 100, + height: 200, + }; + }; + + const name = getIframe(window, div).name; + resetServiceForTesting(window, 'bootstrapBaseUrl'); + resetCountForTesting(); + const newName = getIframe(window, div).name; + expect(name).to.match(/d-\d+.ampproject.net__ping__0/); + expect(newName).to.match(/d-\d+.ampproject.net__ping__0/); + expect(newName).not.to.equal(name); + }); });