Skip to content

Commit 54afc8c

Browse files
calebcordrycalebcordry
authored
♻️Move VariableService to doc level (#22137)
1 parent d6ecb48 commit 54afc8c

13 files changed

Lines changed: 80 additions & 68 deletions

‎extensions/amp-analytics/0.1/amp-analytics.js‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import {AnalyticsEventType} from './events';
2020
import {CookieWriter} from './cookie-writer';
2121
import {
2222
ExpansionOptions,
23-
installVariableService,
24-
variableServiceFor,
23+
installVariableServiceForDoc,
24+
variableServiceForDoc,
2525
} from './variables';
2626
import {
2727
InstrumentationService,
@@ -88,8 +88,8 @@ export class AmpAnalytics extends AMP.BaseElement {
8888
/** @private {?./analytics-group.AnalyticsGroup} */
8989
this.analyticsGroup_ = null;
9090

91-
/** @private {!./variables.VariableService} */
92-
this.variableService_ = variableServiceFor(this.win);
91+
/** @private {?./variables.VariableService} */
92+
this.variableService_ = null;
9393

9494
/** @private {!../../../src/service/crypto-impl.Crypto} */
9595
this.cryptoService_ = Services.cryptoFor(this.win);
@@ -125,6 +125,8 @@ export class AmpAnalytics extends AMP.BaseElement {
125125

126126
/** @override */
127127
buildCallback() {
128+
this.variableService_ = variableServiceForDoc(this.element);
129+
128130
this.isSandbox_ = this.element.hasAttribute('sandbox');
129131

130132
this.element.setAttribute('aria-hidden', 'true');
@@ -711,7 +713,7 @@ AMP.extension(TAG, '0.1', AMP => {
711713
AMP.registerServiceForDoc(
712714
'amp-analytics-instrumentation', InstrumentationService);
713715
AMP.registerServiceForDoc('activity', Activity);
714-
installVariableService(AMP.win);
716+
installVariableServiceForDoc(AMP.ampdoc);
715717
installLinkerReaderService(AMP.win);
716718
// Register the element.
717719
AMP.registerElement(TAG, AmpAnalytics);

‎extensions/amp-analytics/0.1/config.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {dev, user, userAssert} from '../../../src/log';
2222
import {getChildJsonConfig} from '../../../src/json';
2323
import {getMode} from '../../../src/mode';
2424
import {isArray, isObject, toWin} from '../../../src/types';
25-
import {variableServiceFor} from './variables';
25+
import {variableServiceForDoc} from './variables';
2626

2727
const TAG = 'amp-analytics/config';
2828

@@ -355,12 +355,12 @@ export class AnalyticsConfig {
355355
const keys = [];
356356
const expansionPromises = [];
357357

358+
const urlReplacements = Services.urlReplacementsForDoc(element);
359+
const bindings = variableServiceForDoc(element).getMacros();
360+
358361
Object.keys(obj).forEach(key => {
359362
keys.push(key);
360-
const expanded = Services.urlReplacementsForDoc(element)
361-
.expandStringAsync(obj[key],
362-
variableServiceFor(/** @type {!Window} */
363-
(this.win_)).getMacros());
363+
const expanded = urlReplacements.expandStringAsync(obj[key], bindings);
364364
expansionPromises.push(expanded);
365365
});
366366

‎extensions/amp-analytics/0.1/cookie-writer.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
import {BASE_CID_MAX_AGE_MILLIS} from '../../../src/service/cid-impl';
1818
import {Services} from '../../../src/services';
1919
import {getMode} from '../../../src/mode';
20-
import {getNameArgs, variableServiceFor} from './variables';
20+
import {getNameArgs, variableServiceForDoc} from './variables';
2121
import {hasOwn} from '../../../src/utils/object';
2222
import {isInFie} from '../../../src/friendly-iframe-embed';
2323
import {isObject} from '../../../src/types';
@@ -65,7 +65,7 @@ export class CookieWriter {
6565
this.config_ = config;
6666

6767
/** @const @private {!JsonObject} */
68-
this.bindings_ = variableServiceFor(this.win_).getMacros();
68+
this.bindings_ = variableServiceForDoc(element).getMacros();
6969
}
7070

7171
/**

‎extensions/amp-analytics/0.1/linker-manager.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import {ExpansionOptions, variableServiceFor} from './variables';
17+
import {ExpansionOptions, variableServiceForDoc} from './variables';
1818
import {Priority} from '../../../src/service/navigation';
1919
import {Services} from '../../../src/services';
2020
import {WindowInterface} from '../../../src/window-interface';
@@ -71,7 +71,7 @@ export class LinkerManager {
7171
this.formSubmitUnlistener_ = null;
7272

7373
/** @const @private {!./variables.VariableService} */
74-
this.variableService_ = variableServiceFor(this.ampdoc_.win);
74+
this.variableService_ = variableServiceForDoc(this.ampdoc_);
7575
}
7676

7777

‎extensions/amp-analytics/0.1/requests.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {BatchSegmentDef, defaultSerializer} from './transport-serializer';
1818
import {
1919
ExpansionOptions,
2020
getConsentStateStr,
21-
variableServiceFor,
21+
variableServiceForDoc,
2222
} from './variables';
2323
import {SANDBOX_AVAILABLE_VARS} from './sandbox-vars-whitelist';
2424
import {Services} from '../../../src/services';
@@ -61,7 +61,7 @@ export class RequestHandler {
6161
this.batchIntervalPointer_ = null;
6262

6363
/** @private {!./variables.VariableService} */
64-
this.variableService_ = variableServiceFor(this.win);
64+
this.variableService_ = variableServiceForDoc(element);
6565

6666
/** @private {!../../../src/service/url-replacements-impl.UrlReplacements} */
6767
this.urlReplacementService_ = Services.urlReplacementsForDoc(element);
@@ -124,7 +124,7 @@ export class RequestHandler {
124124
this.lastTrigger_ = trigger;
125125
const bindings = this.variableService_.getMacros();
126126
bindings['RESOURCE_TIMING'] = getResourceTiming(
127-
this.win, trigger['resourceTimingSpec'], this.startTime_);
127+
this.ampdoc_, trigger['resourceTimingSpec'], this.startTime_);
128128
// TODO: (@zhouyx) Move to variable service once that becomes
129129
// a doc level services
130130
bindings['CONSENT_STATE'] = getConsentStateStr(this.element_);
@@ -313,7 +313,7 @@ export class RequestHandler {
313313
export function expandPostMessage(
314314
ampdoc, msg, configParams, trigger, expansionOption, element)
315315
{
316-
const variableService = variableServiceFor(ampdoc.win);
316+
const variableService = variableServiceForDoc(ampdoc);
317317
const urlReplacementService = Services.urlReplacementsForDoc(element);
318318

319319
const bindings = variableService.getMacros();

‎extensions/amp-analytics/0.1/resource-timing.js‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* limitations under the License.
1515
*/
1616

17-
import {ExpansionOptions, variableServiceFor} from './variables';
17+
import {ExpansionOptions, variableServiceForDoc} from './variables';
1818
import {findIndex} from '../../../src/utils/array';
1919
import {isObject} from '../../../src/types';
2020
import {parseUrlDeprecated} from '../../../src/url';
@@ -233,14 +233,14 @@ function filterEntries(entries, resourceDefs) {
233233
* single string.
234234
* @param {!Array<!PerformanceResourceTiming>} entries
235235
* @param {!JsonObject} resourceTimingSpec
236-
* @param {!Window} win
236+
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
237237
* @return {!Promise<string>}
238238
*/
239-
function serialize(entries, resourceTimingSpec, win) {
239+
function serialize(entries, resourceTimingSpec, ampdoc) {
240240
const resources = resourceTimingSpec['resources'];
241241
const encoding = resourceTimingSpec['encoding'];
242242

243-
const variableService = variableServiceFor(win);
243+
const variableService = variableServiceForDoc(ampdoc);
244244
const format = (val, relativeTo = 0) =>
245245
Math.round(val - relativeTo).toString(encoding['base'] || 10);
246246

@@ -255,11 +255,12 @@ function serialize(entries, resourceTimingSpec, win) {
255255

256256
/**
257257
* Serializes resource timing entries according to the resource timing spec.
258-
* @param {!Window} win
258+
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
259259
* @param {!JsonObject} resourceTimingSpec
260260
* @return {!Promise<string>}
261261
*/
262-
function serializeResourceTiming(win, resourceTimingSpec) {
262+
function serializeResourceTiming(ampdoc, resourceTimingSpec) {
263+
const {win} = ampdoc;
263264
// Check that the performance timing API exists before and that the spec is
264265
// valid before proceeding. If not, we simply return an empty string.
265266
if (resourceTimingSpec['done'] || !win.performance || !win.performance.now ||
@@ -287,19 +288,19 @@ function serializeResourceTiming(win, resourceTimingSpec) {
287288
return Promise.resolve('');
288289
}
289290
// Yield the thread in case iterating over all resources takes a long time.
290-
return yieldThread(() => serialize(entries, resourceTimingSpec, win));
291+
return yieldThread(() => serialize(entries, resourceTimingSpec, ampdoc));
291292
}
292293

293294
/**
294-
* @param {!Window} win resource timing spec.
295+
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
295296
* @param {!JsonObject|undefined} spec resource timing spec.
296297
* @param {number} startTime start timestamp.
297298
* @return {!Promise<string>}
298299
*/
299-
export function getResourceTiming(win, spec, startTime) {
300+
export function getResourceTiming(ampdoc, spec, startTime) {
300301
// Only allow collecting timing within 1s
301302
if (spec && (Date.now() < (startTime + 60 * 1000))) {
302-
return serializeResourceTiming(win, spec);
303+
return serializeResourceTiming(ampdoc, spec);
303304
} else {
304305
return Promise.resolve('');
305306
}

‎extensions/amp-analytics/0.1/test/test-amp-analytics.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,7 @@ describes.realWin('amp-analytics', {
217217
const whenFirstVisibleStub = sandbox.stub(
218218
viewer,
219219
'whenFirstVisible').callsFake(() => new Promise(function() {}));
220+
doc.body.appendChild(el);
220221
const analytics = new AmpAnalytics(el);
221222
el.getAmpDoc = () => ampdoc;
222223
analytics.buildCallback();

‎extensions/amp-analytics/0.1/test/test-cookie-writer.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ import * as cookie from '../../../../src/cookies';
1818
import {CookieWriter} from '../cookie-writer';
1919
import {dict} from '../../../../src/utils/object';
2020
import {installLinkerReaderService} from '../linker-reader';
21-
import {installVariableService, variableServiceFor} from '../variables';
21+
import {
22+
installVariableServiceForDoc,
23+
variableServiceForDoc,
24+
} from '../variables';
2225

2326

2427

@@ -46,7 +49,7 @@ describes.realWin('amp-analytics.cookie-writer', {
4649
});
4750
element = doc.createElement('div');
4851
doc.body.appendChild(element);
49-
installVariableService(win);
52+
installVariableServiceForDoc(doc);
5053
installLinkerReaderService(win);
5154
});
5255

@@ -105,7 +108,7 @@ describes.realWin('amp-analytics.cookie-writer', {
105108
location: 'https://www-example-com.cdn.ampproject.org',
106109
};
107110
installLinkerReaderService(mockWin);
108-
installVariableService(mockWin);
111+
installVariableServiceForDoc(doc);
109112
const cookieWriter = new CookieWriter(mockWin, element, config);
110113
expandAndWriteSpy = sandbox.spy(cookieWriter, 'expandAndWrite_');
111114
return cookieWriter.write().then(() => {
@@ -265,7 +268,7 @@ describes.realWin('amp-analytics.cookie-writer', {
265268
},
266269
});
267270
const cookieWriter = new CookieWriter(win, element, config);
268-
const variableService = variableServiceFor(win);
271+
const variableService = variableServiceForDoc(doc);
269272
sandbox.stub(variableService.linkerReader_,
270273
'get').callsFake((name, id) => {
271274
return `${name}-${id}`;

‎extensions/amp-analytics/0.1/test/test-linker-manager.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
installLinkerReaderService,
2727
linkerReaderServiceFor,
2828
} from '../linker-reader';
29-
import {installVariableService} from '../variables';
29+
import {installVariableServiceForDoc} from '../variables';
3030
import {mockWindowInterface} from '../../../../testing/test-helper';
3131
import {toggleExperiment} from '../../../../src/experiments';
3232

@@ -79,7 +79,7 @@ describes.realWin('Linker Manager', {amp: true}, env => {
7979
origin: 'https://amp-source-com.cdn.ampproject.org',
8080
});
8181
windowInterface.getHostname.returns('amp-source-com.cdn.ampproject.org');
82-
installVariableService(win);
82+
installVariableServiceForDoc(env.ampdoc);
8383
installLinkerReaderService(win);
8484
});
8585

‎extensions/amp-analytics/0.1/test/test-requests.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
import * as ResourceTiming from '../resource-timing';
1818
import * as lolex from 'lolex';
19-
import {ExpansionOptions, installVariableService} from '../variables';
19+
import {ExpansionOptions, installVariableServiceForDoc} from '../variables';
2020
import {RequestHandler, expandPostMessage} from '../requests';
2121
import {installLinkerReaderService} from '../linker-reader';
2222
import {macroTask} from '../../../../testing/yield';
@@ -29,9 +29,9 @@ describes.realWin('Requests', {amp: 1}, env => {
2929
let analyticsElement;
3030

3131
beforeEach(() => {
32-
installVariableService(env.win);
33-
installLinkerReaderService(env.win);
3432
ampdoc = env.ampdoc;
33+
installLinkerReaderService(env.win);
34+
installVariableServiceForDoc(ampdoc);
3535
ampdoc.defaultView = env.win;
3636
clock = lolex.install({target: ampdoc.win});
3737
preconnectSpy = sandbox.spy();

0 commit comments

Comments
 (0)