Lazy load AMP cache URL and make it optional.

ampproject · May 27, 2021 · e104900 · e104900
1 parent 5dc1502
commit e104900
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 29 deletions.
diff --git a/build-system/test-configs/dep-check-config.js b/build-system/test-configs/dep-check-config.js
@@ -331,8 +331,6 @@ exports.rules = [
  'src/service/extension-script.js',
  'extensions/amp-video/0.1/amp-video.js->' +
  'src/service/video-manager-impl.js',
- 'extensions/amp-video/0.1/video-cache.js->' +
- 'src/service/extension-script.js',
  'extensions/amp-video-iframe/0.1/amp-video-iframe.js->' +
  'src/service/video-manager-impl.js',
  'extensions/amp-ooyala-player/0.1/amp-ooyala-player.js->' +

diff --git a/extensions/amp-video/0.1/amp-video.js b/extensions/amp-video/0.1/amp-video.js
@@ -271,7 +271,7 @@ export class AmpVideo extends AMP.BaseElement {
 
  // Fetch and add cached sources URLs if opted-in, and if the sources don't already contained cached URLs from the AMP Cache.
  if (this.element.hasAttribute('cache') && !this.hasAnyCachedSources_()) {
- return fetchCachedSources(this.element, this.win);
+ return fetchCachedSources(this.element, this.getAmpDoc());
  }
  }
 

diff --git a/extensions/amp-video/0.1/test/test-video-cache.js b/extensions/amp-video/0.1/test/test-video-cache.js
@@ -17,25 +17,27 @@
 import {AmpCacheUrlService} from '../../../amp-cache-url/0.1/amp-cache-url';
 import {Services} from '../../../../src/services';
 import {createElementWithAttributes} from '../../../../src/dom';
-import {createExtensionScript} from '../../../../src/service/extension-script';
 import {fetchCachedSources} from '../video-cache';
 import {xhrServiceForTesting} from '../../../../src/service/xhr-impl';
 
 describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
- let xhrService;
  let cacheUrlService;
+ let extensionsService;
+ let xhrService;
 
  beforeEach(() => {
  xhrService = xhrServiceForTesting(env.win);
  env.sandbox.stub(Services, 'xhrFor').returns(xhrService);
 
+ extensionsService = {
+ installExtensionForDoc: env.sandbox.spy(() => Promise.resolve()),
+ };
+ env.sandbox.stub(Services, 'extensionsFor').returns(extensionsService);
+
  cacheUrlService = new AmpCacheUrlService();
  env.sandbox
  .stub(Services, 'cacheUrlServicePromiseForDoc')
  .resolves(cacheUrlService);
- env.win.document.head.appendChild(
- createExtensionScript(env.win, 'amp-cache-url', '0.1')
- );
  env.sandbox.stub(Services, 'documentInfoForDoc').returns({
  sourceUrl: 'https://example.com',
  canonicalUrl: 'https://canonical.com',
@@ -47,7 +49,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  const videoEl = createVideo([{src: 'video1.mp4'}]);
  const xhrSpy = env.sandbox.spy(xhrService, 'fetch');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(xhrSpy).to.have.been.calledWith(
  'https://example-com.cdn.ampproject.org/mbv/s/example.com/video1.mp4?amp_video_host_url=https%3A%2F%2Fcanonical.com'
@@ -58,7 +60,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  const videoEl = createVideo([{src: 'video1.mp4'}, {src: 'video2.mp4'}]);
  const xhrSpy = env.sandbox.spy(xhrService, 'fetch');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(xhrSpy).to.have.been.calledWith(
  'https://example-com.cdn.ampproject.org/mbv/s/example.com/video1.mp4?amp_video_host_url=https%3A%2F%2Fcanonical.com'
@@ -72,7 +74,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  ]);
  const xhrSpy = env.sandbox.spy(xhrService, 'fetch');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(xhrSpy).to.have.been.calledWith(
  'https://example-com.cdn.ampproject.org/mbv/s/example.com/video2.mp4?amp_video_host_url=https%3A%2F%2Fcanonical.com'
@@ -87,7 +89,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  videoEl.setAttribute('src', 'video1.mp4');
  const xhrSpy = env.sandbox.spy(xhrService, 'fetch');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(xhrSpy).to.have.been.calledWith(
  'https://example-com.cdn.ampproject.org/mbv/s/example.com/video1.mp4?amp_video_host_url=https%3A%2F%2Fcanonical.com'
@@ -100,7 +102,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  const videoEl = createVideo([{'src': 'https://website.com/video.html'}]);
  const xhrSpy = env.sandbox.spy(xhrService, 'fetch');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(xhrSpy).to.have.been.calledWith(
  'https://website-com.cdn.ampproject.org/mbv/s/website.com/video.html?amp_video_host_url=https%3A%2F%2Fcanonical.com'
@@ -111,7 +113,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  const videoEl = createVideo([{'src': 'video.html'}]);
  const xhrSpy = env.sandbox.spy(xhrService, 'fetch');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(xhrSpy).to.have.been.calledWith(
  'https://example-com.cdn.ampproject.org/mbv/s/example.com/video.html?amp_video_host_url=https%3A%2F%2Fcanonical.com'
@@ -131,7 +133,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  });
 
  const videoEl = createVideo([{src: 'video.mp4'}]);
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  const addedSource = videoEl.querySelector('source');
  expect(addedSource.getAttribute('src')).to.equal('video1.mp4');
@@ -153,7 +155,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
 
  const videoEl = createVideo([{src: 'video.mp4'}]);
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  const addedSources = videoEl.querySelectorAll('source');
  expect(addedSources[0].getAttribute('data-bitrate')).to.equal('2000');
@@ -177,7 +179,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  videoEl.setAttribute('src', 'video1.mp4');
  videoEl.setAttribute('type', 'video/mp4');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  const lastSource = videoEl.querySelector('source:last-of-type');
  expect(lastSource.getAttribute('src')).to.equal('video1.mp4');
@@ -205,7 +207,7 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
  ]);
  videoEl.setAttribute('src', 'video1.mp4');
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  const addedSources = videoEl.querySelectorAll('source');
  expect(addedSources).to.have.lengthOf(4); // 3 from cache + 1 fallback.
@@ -225,13 +227,13 @@ describes.realWin('amp-video cached-sources', {amp: true}, (env) => {
 
  const videoEl = createVideo([{src: 'video.mp4'}]);
 
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(videoEl.querySelector('source[data-bitrate]')).to.not.be.null;
  });
  it('should not create the sources if there is amp-orig-src attribute', async () => {
  const videoEl = createVideo([{'src': 'video.mp4', 'amp-orig-src': ''}]);
- await fetchCachedSources(videoEl, env.win);
+ await fetchCachedSources(videoEl, env.ampdoc);
 
  expect(videoEl.querySelector('source[data-bitrate]')).to.be.null;
  });

diff --git a/extensions/amp-video/0.1/test/validator-amp-video-cache.html b/extensions/amp-video/0.1/test/validator-amp-video-cache.html
@@ -27,7 +27,6 @@
  <script async src="https://cdn.ampproject.org/v0.js"></script>
  <script async custom-element="amp-video" src="https://cdn.ampproject.org/v0/amp-video-0.1.js"></script>
  <script async custom-element="amp-story" src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
- <script async custom-element="amp-cache-url" src="https://cdn.ampproject.org/v0/amp-cache-url-0.1.js"></script>
 </head>
 <body>
  <amp-story standalone title="My Story" publisher="Me" publisher-logo-src="http://me.com/logo.png" poster-portrait-src="http://me.com/portrait.jpg" poster-square-src="http://me.com/square.jpg" poster-landscape-src="http://me.com/landscape.jpg" background-audio="http://me.com/path/to/my.mp3" entity="Someone else" entity-logo-src="http://someoneelse.com/logo.png" entity-url="http://someoneelse.com/profile">

diff --git a/extensions/amp-video/0.1/test/validator-amp-video-cache.out b/extensions/amp-video/0.1/test/validator-amp-video-cache.out
@@ -28,7 +28,6 @@ PASS
 | <script async src="https://cdn.ampproject.org/v0.js"></script>
 | <script async custom-element="amp-video" src="https://cdn.ampproject.org/v0/amp-video-0.1.js"></script>
 | <script async custom-element="amp-story" src="https://cdn.ampproject.org/v0/amp-story-1.0.js"></script>
-| <script async custom-element="amp-cache-url" src="https://cdn.ampproject.org/v0/amp-cache-url-0.1.js"></script>
 | </head>
 | <body>
 | <amp-story standalone title="My Story" publisher="Me" publisher-logo-src="http://me.com/logo.png" poster-portrait-src="http://me.com/portrait.jpg" poster-square-src="http://me.com/square.jpg" poster-landscape-src="http://me.com/landscape.jpg" background-audio="http://me.com/path/to/my.mp3" entity="Someone else" entity-logo-src="http://someoneelse.com/logo.png" entity-url="http://someoneelse.com/profile">

diff --git a/extensions/amp-video/0.1/video-cache.js b/extensions/amp-video/0.1/video-cache.js
@@ -22,7 +22,6 @@ import {
  matches,
  removeElement,
 } from '../../../src/dom';
-import {extensionScriptInNode} from '../../../src/service/extension-script';
 import {toArray} from '../../../src/core/types/array';
 import {user} from '../../../src/log';
 
@@ -32,12 +31,12 @@ import {user} from '../../../src/log';
  * and appends the document canonical url as the queryParam `amp_video_host_url`.
  *
  * @param {!Element} videoEl
- * @param {!Window} win
+ * @param {!AmpDoc} ampdoc
  * @return {!Promise}
  */
-export function fetchCachedSources(videoEl, win) {
+export function fetchCachedSources(videoEl, ampdoc) {
+ const {win} = ampdoc;
  if (
- !extensionScriptInNode(win, 'amp-cache-url', '0.1') ||
  !(
  videoEl.getAttribute('src') ||
  videoEl.querySelector('source[src]')?.getAttribute('src')
@@ -47,10 +46,9 @@ export function fetchCachedSources(videoEl, win) {
  return Promise.resolve();
  }
  const {canonicalUrl, sourceUrl} = Services.documentInfoForDoc(win.document);
- const servicePromise = Services.cacheUrlServicePromiseForDoc(videoEl);
  maybeReplaceSrcWithSourceElement(videoEl, win);
  const videoUrl = resolveRelativeUrl(selectVideoSource(videoEl), sourceUrl);
- return servicePromise
+ return getCacheUrlService(videoEl, ampdoc)
  .then((service) => service.createCacheUrl(videoUrl))
  .then((cacheUrl) => {
  const requestUrl = addParamsToUrl(cacheUrl.replace('/c/', '/mbv/'), {
@@ -135,3 +133,17 @@ function maybeReplaceSrcWithSourceElement(videoEl, win) {
 
  videoEl.insertBefore(sourceEl, videoEl.firstChild);
 }
+
+/**
+ * Lazy loads the amp-cache-url extension if needed and retrieves its service.
+ * @param {!Element} videoEl
+ * @param {!AmpDoc} ampdoc
+ * @return {!Promise<../../../amp-cache-url/amp-cache-url.AmpCacheUrlService>}
+ */
+function getCacheUrlService(videoEl, ampdoc) {
+ return Services.extensionsFor(ampdoc.win)
+ .installExtensionForDoc(ampdoc, 'amp-cache-url')
+ .then(() => {
+ return Services.cacheUrlServicePromiseForDoc(videoEl);
+ });
+}
diff --git a/extensions/amp-video/validator-amp-video.protoascii b/extensions/amp-video/validator-amp-video.protoascii
@@ -192,7 +192,6 @@ tags: { # <amp-video> in amp-story
  }
  attrs: {
  name: "cache"
- requires_extension: "amp-cache-url"
  value: "google"
  }
  attr_lists: "extended-amp-global"