diff --git a/extensions/amp-bind/0.1/amp-bind.js b/extensions/amp-bind/0.1/amp-bind.js index 12f344ab1f4e8..a60b745c83f26 100644 --- a/extensions/amp-bind/0.1/amp-bind.js +++ b/extensions/amp-bind/0.1/amp-bind.js @@ -17,5 +17,10 @@ import {AmpState} from './amp-state'; import {Bind} from './bind-impl'; -AMP.registerServiceForDoc('bind', Bind); -AMP.registerElement('amp-state', AmpState); +/** @const {string} */ +const TAG = 'amp-bind'; + +AMP.extension(TAG, '0.1', function(AMP) { + AMP.registerServiceForDoc('bind', Bind); + AMP.registerElement('amp-state', AmpState); +}); diff --git a/extensions/amp-bind/0.1/test/test-amp-state.js b/extensions/amp-bind/0.1/test/test-amp-state.js index 742d1b70d2336..e75b5fff1db5e 100644 --- a/extensions/amp-bind/0.1/test/test-amp-state.js +++ b/extensions/amp-bind/0.1/test/test-amp-state.js @@ -20,7 +20,7 @@ import {viewerForDoc} from '../../../../src/services'; describes.realWin('AmpState', { amp: { runtimeOn: true, - extensions: ['amp-state:0.1'], + extensions: ['amp-bind:0.1'], }, }, env => { let ampState; diff --git a/src/custom-element.js b/src/custom-element.js index f0ff2f5b6bb09..fe42f3778041e 100644 --- a/src/custom-element.js +++ b/src/custom-element.js @@ -219,7 +219,11 @@ export function copyElementToChildWindow(parentWin, childWin, name) { */ export function upgradeElementInChildWindow(parentWin, childWin, name) { const toClass = getExtendedElements(parentWin)[name]; - dev().assert(toClass, '%s is not stubbed yet', name); + // Some extensions unofficially register unstubbed elements, e.g. amp-bind. + // Can be changed to assert() once official support (#9143) is implemented. + if (!toClass) { + dev().warn(TAG_, '%s is not stubbed yet', name); + } dev().assert(toClass != ElementStub, '%s is not upgraded yet', name); upgradeOrRegisterElement(childWin, name, toClass); } diff --git a/src/element-service.js b/src/element-service.js index 8e6894e3fa84c..5d668083eb987 100644 --- a/src/element-service.js +++ b/src/element-service.js @@ -15,6 +15,7 @@ */ import { + getExistingServiceForDocInEmbedScope, getServicePromise, getServicePromiseOrNull, getAmpdoc, @@ -40,14 +41,8 @@ import * as dom from './dom'; * @return {!Promise<*>} */ export function getElementService(win, id, extension, opt_element) { - const service = getElementServiceIfAvailable(win, id, extension, opt_element); - return service.then(service => { - return user().assert(service, - 'Service %s was requested to be provided through %s, ' + - 'but %s is not loaded in the current page. To fix this ' + - 'problem load the JavaScript file for %s in this page.', - id, extension, extension, extension); - }); + return getElementServiceIfAvailable(win, id, extension, opt_element).then( + service => assertService(service, id, extension)); } /** @@ -66,24 +61,7 @@ export function getElementServiceIfAvailable(win, id, extension, opt_element) { if (s) { return /** @type {!Promise} */ (s); } - // Microtask is necessary to ensure that window.ampExtendedElements has been - // initialized. - return Promise.resolve().then(() => { - if (!opt_element && isElementScheduled(win, extension)) { - return getServicePromise(win, id); - } - // Wait for HEAD to fully form before denying access to the service. - return dom.waitForBodyPromise(win.document).then(() => { - // If this service is provided by an element, then we can't depend on the - // service (they may not use the element). - if (opt_element) { - return getServicePromiseOrNull(win, id); - } else if (isElementScheduled(win, extension)) { - return getServicePromise(win, id); - } - return null; - }); - }); + return elementServicePromiseOrNull(win, id, extension, opt_element); } /** @@ -116,15 +94,9 @@ function isElementScheduled(win, elementName) { * @return {!Promise<*>} */ export function getElementServiceForDoc(nodeOrDoc, id, extension, opt_element) { - const service = getElementServiceIfAvailableForDoc(nodeOrDoc, id, extension, - opt_element); - return service.then(service => { - return user().assert(service, - 'Service %s was requested to be provided through %s, ' + - 'but %s is not loaded in the current page. To fix this ' + - 'problem load the JavaScript file for %s in this page.', - id, extension, extension, extension); - }); + return getElementServiceIfAvailableForDoc( + nodeOrDoc, id, extension, opt_element) + .then(service => assertService(service, id, extension)); } /** @@ -164,3 +136,94 @@ export function getElementServiceIfAvailableForDoc( }); }); } + +/** + * Returns a promise for a service in the closest embed scope of `nodeOrDoc`. + * If no embed-scope service is found, falls back to top-level service. + * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc + * @param {string} id of the service. + * @param {string} extension Name of the custom element that provides + * the implementation of this service. + * @return {!Promise} + */ +export function getElementServiceForDocInEmbedScope( + nodeOrDoc, id, extension) { + return getElementServiceIfAvailableForDocInEmbedScope( + nodeOrDoc, id, extension) + .then(service => { + if (service) { + return service; + } + // Fallback to ampdoc. + return getElementServiceForDoc(nodeOrDoc, id, extension); + }); +} + +/** + * Same as `getElementServiceForDocInEmbedScope` but without top-level fallback. + * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc + * @param {string} id of the service. + * @param {string} extension Name of the custom element that provides + * the implementation of this service. + * @return {!Promise} + */ +export function getElementServiceIfAvailableForDocInEmbedScope( + nodeOrDoc, id, extension) { + const s = getExistingServiceForDocInEmbedScope(nodeOrDoc, id); + if (s) { + return /** @type {!Promise} */ (Promise.resolve(s)); + } + if (nodeOrDoc.nodeType) { + const win = /** @type {!Document} */ ( + nodeOrDoc.ownerDocument || nodeOrDoc).defaultView; + return elementServicePromiseOrNull(win, id, extension); + } + return /** @type {!Promise} */ (Promise.resolve(null)); +} + +/** + * Throws user error if `service` is null. + * @param {Object} service + * @param {string} id + * @param {string} extension + * @return {!Object} + * @private + */ +function assertService(service, id, extension) { + return /** @type {!Object} */ (user().assert(service, + 'Service %s was requested to be provided through %s, ' + + 'but %s is not loaded in the current page. To fix this ' + + 'problem load the JavaScript file for %s in this page.', + id, extension, extension, extension)); +} + +/** + * Returns the promise for service with `id` on the given window if available. + * Otherwise, resolves with null (service was not registered). + * @param {!Window} win + * @param {string} id + * @param {string} extension + * @param {boolean=} opt_element + * @return {!Promise} + * @private + */ +function elementServicePromiseOrNull(win, id, extension, opt_element) { + // Microtask is necessary to ensure that window.ampExtendedElements has been + // initialized. + return Promise.resolve().then(() => { + if (!opt_element && isElementScheduled(win, extension)) { + return getServicePromise(win, id); + } + // Wait for HEAD to fully form before denying access to the service. + return dom.waitForBodyPromise(win.document).then(() => { + // If this service is provided by an element, then we can't depend on the + // service (they may not use the element). + if (opt_element) { + return getServicePromiseOrNull(win, id); + } else if (isElementScheduled(win, extension)) { + return getServicePromise(win, id); + } + return null; + }); + }); +} diff --git a/src/runtime.js b/src/runtime.js index 32244d3a79c95..ce6e0c25efe91 100644 --- a/src/runtime.js +++ b/src/runtime.js @@ -27,6 +27,7 @@ import {VisibilityState} from './visibility-state'; import { addDocFactoryToExtension, addElementToExtension, + addServiceToExtension, addShadowRootFactoryToExtension, installBuiltinElements, installExtensionsInShadowDoc, @@ -530,6 +531,8 @@ function prepareAndRegisterServiceForDoc(global, extensions, const ampdocService = ampdocServiceFor(global); const ampdoc = ampdocService.getAmpDoc(); registerServiceForDoc(ampdoc, name, opt_ctor, opt_factory); + + addServiceToExtension(extensions, name); } @@ -548,6 +551,8 @@ function prepareAndRegisterServiceForDocShadowMode(global, extensions, addDocFactoryToExtension(extensions, ampdoc => { registerServiceForDoc(ampdoc, name, opt_ctor, opt_factory); }, name); + + addServiceToExtension(extensions, name); } diff --git a/src/service.js b/src/service.js index a6f2aa866a61e..509e354d782cc 100644 --- a/src/service.js +++ b/src/service.js @@ -594,15 +594,15 @@ export function isEmbeddable(service) { /** - * Asserts that the specified service implements `EmbeddableService` interface - * and typecasts the instance to `EmbeddableService`. - * @param {!Object} service - * @return {!EmbeddableService} + * Adopts an embeddable (implements `EmbeddableService` interface) service + * in embed scope. + * @param {!Window} embedWin + * @param {string} serviceId */ -export function assertEmbeddable(service) { - dev().assert(isEmbeddable(service), - 'required to implement EmbeddableService'); - return /** @type {!EmbeddableService} */ (service); +export function adoptServiceForEmbed(embedWin, serviceId) { + const adopted = adoptServiceForEmbedIfEmbeddable(embedWin, serviceId); + dev().assert(adopted, + `${serviceId} required to implement EmbeddableService.`); } @@ -611,13 +611,18 @@ export function assertEmbeddable(service) { * in embed scope. * @param {!Window} embedWin * @param {string} serviceId + * @return {boolean} */ -export function adoptServiceForEmbed(embedWin, serviceId) { +export function adoptServiceForEmbedIfEmbeddable(embedWin, serviceId) { const frameElement = /** @type {!Node} */ (dev().assert( embedWin.frameElement, 'frameElement not found for embed')); const service = getServiceForDoc(frameElement, serviceId); - assertEmbeddable(service).adoptEmbedWindow(embedWin); + if (isEmbeddable(service)) { + service.adoptEmbedWindow(embedWin); + return true; + } + return false; } diff --git a/src/service/ampdoc-impl.js b/src/service/ampdoc-impl.js index 21ad48229880e..75e1634d8aea9 100644 --- a/src/service/ampdoc-impl.js +++ b/src/service/ampdoc-impl.js @@ -321,9 +321,7 @@ export class AmpDocSingle extends AmpDoc { waitForBodyPromise(this.win.document).then(() => this.getBody()); /** @private @const {!Promise} */ - this.readyPromise_ = isDocumentReady(this.win.document) ? - Promise.resolve() : - whenDocumentReady(this.win.document); + this.readyPromise_ = whenDocumentReady(this.win.document); } /** @override */ diff --git a/src/service/extensions-impl.js b/src/service/extensions-impl.js index 6e0dc8176e6ac..f1537c0b3fea5 100644 --- a/src/service/extensions-impl.js +++ b/src/service/extensions-impl.js @@ -16,6 +16,7 @@ import { adoptServiceForEmbed, + adoptServiceForEmbedIfEmbeddable, registerServiceBuilder, setParentWindow, } from '../service'; @@ -57,10 +58,10 @@ let ExtensionElementDef; /** * The structure that contains the resources declared by an extension. - * Currently only limitted to elements. * * @typedef {{ * elements: !Object, + * services: !Array, * }} */ let ExtensionDef; @@ -138,6 +139,17 @@ export function addElementToExtension( extensions.addElement_(name, implementationClass, css); } +/** + * Add a service to the extension currently being registered. This is a + * restricted method and it's allowed to be called only during the overall + * extension registration. + * @param {!Extensions} extensions + * @param {string} name + * @restricted + */ +export function addServiceToExtension(extensions, name) { + extensions.addService_(name); +} /** * Add a ampdoc factory to the extension currently being registered. This is a @@ -278,6 +290,15 @@ export class Extensions { holder.extension.elements[name] = {implementationClass, css}; } + /** + * Adds `name` to the list of services registered by the current extension. + * @param {string} name + */ + addService_(name) { + const holder = this.getCurrentExtensionHolder_(); + holder.extension.services.push(name); + } + /** * Registers an ampdoc factory. * @param {function(!./ampdoc-impl.AmpDoc)} factory @@ -389,7 +410,7 @@ export class Extensions { } // Adopt embeddable services. - adoptServicesForEmbed(childWin); + adoptStandardServicesForEmbed(childWin); // Install built-ins and legacy elements. copyBuiltinElementsToChildWindow(topWin, childWin); @@ -406,9 +427,10 @@ export class Extensions { // Install CSS. const promise = this.loadExtension(extensionId).then(extension => { - // TODO(dvoytenko): Adopt embeddable services from the extension when - // becomes necessary. This will require refactoring of extension - // loader that can be resolved via the parent ampdoc. + // Adopt embeddable extension services. + extension.services.forEach(service => { + adoptServiceForEmbedIfEmbeddable(childWin, service); + }); // Adopt the custom element. const elementDef = extension.elements[extensionId]; @@ -420,12 +442,15 @@ export class Extensions { /* completeCallback */ resolve, /* isRuntime */ false, extensionId); - }); + }).then(() => extension); // Forward `extension` to chained Promise. } - }).then(() => { + return extension; + }).then(extension => { // Notice that stubbing happens much sooner above // (see stubElementInChildWindow). - upgradeElementInChildWindow(topWin, childWin, extensionId); + Object.keys(extension.elements).forEach(element => { + upgradeElementInChildWindow(topWin, childWin, element); + }); }); promises.push(promise); }); @@ -441,9 +466,10 @@ export class Extensions { getExtensionHolder_(extensionId) { let holder = this.extensions_[extensionId]; if (!holder) { - const extension = { + const extension = /** @type {ExtensionDef} */ ({ elements: {}, - }; + services: [], + }); holder = /** @type {ExtensionHolderDef} */ ({ extension, docFactories: [], @@ -604,7 +630,7 @@ function installPolyfillsInChildWindow(childWin) { * Adopt predefined core services for the child window (friendly iframe). * @param {!Window} childWin */ -function adoptServicesForEmbed(childWin) { +function adoptStandardServicesForEmbed(childWin) { // The order of service adoptations is important. // TODO(dvoytenko): Refactor service registration if this set becomes // to pass the "embeddable" flag if this set becomes too unwieldy. diff --git a/src/services.js b/src/services.js index 558d89223b83b..f64128c8f49b8 100644 --- a/src/services.js +++ b/src/services.js @@ -24,6 +24,7 @@ import { import { getElementService, getElementServiceForDoc, + getElementServiceForDocInEmbedScope, getElementServiceIfAvailable, getElementServiceIfAvailableForDoc, } from './element-service'; @@ -92,7 +93,7 @@ export function batchedXhrFor(window) { */ export function bindForDoc(nodeOrDoc) { return /** @type {!Promise} */ ( - getElementServiceForDoc(nodeOrDoc, 'bind', 'amp-bind')); + getElementServiceForDocInEmbedScope(nodeOrDoc, 'bind', 'amp-bind')); } /** diff --git a/test/functional/test-extensions.js b/test/functional/test-extensions.js index 08a72130a7311..64d90e6d92df5 100644 --- a/test/functional/test-extensions.js +++ b/test/functional/test-extensions.js @@ -27,6 +27,7 @@ import { registerExtension, } from '../../src/service/extensions-impl'; import {extensionsFor} from '../../src/services'; +import {registerServiceBuilder} from '../../src/service'; import {resetScheduledElementForTesting} from '../../src/custom-element'; import {loadPromise} from '../../src/event-helper'; @@ -473,7 +474,10 @@ describes.sandboxed('Extensions', {}, () => { }, parentWin.AMP); const elements = {}; elements[extensionId] = {css: 'a{}'}; - return {elements}; + return { + elements, + services: [], + }; }); }); const promise = extensions.installExtensionsInChildWindow( @@ -494,6 +498,39 @@ describes.sandboxed('Extensions', {}, () => { }); }); + it('should adopt extension services', () => { + const fooSpy = sandbox.spy(); + const fakeServiceFoo = {adoptEmbedWindow: fooSpy}; + registerServiceBuilder(parentWin, 'fake-service-foo', + () => fakeServiceFoo, /* opt_instantiate */ true); + + const barSpy = sandbox.spy(); + const fakeServiceBar = {adoptEmbedWindow: barSpy}; + registerServiceBuilder(parentWin, 'fake-service-bar', + () => fakeServiceBar, /* opt_instantiate */ true); + + sandbox.stub(extensions, 'loadExtension', extensionId => { + return Promise.resolve().then(() => { + registerExtension(extensions, extensionId, AMP => { + AMP.registerElement(extensionId, AmpTest); + }, parentWin.AMP); + const elements = {}; + elements[extensionId] = {}; + return /* ExtensionDef */ { + elements, + services: ['fake-service-foo'], // fake-service-bar NOT included. + }; + }); + }); + + const promise = + extensions.installExtensionsInChildWindow(iframeWin, ['amp-test']); + return promise.then(() => { + expect(fooSpy).calledOnce; + expect(barSpy).notCalled; + }); + }); + it('should call pre-install callback before other installs', () => { const stub = sandbox.stub(extensions, 'loadExtension', extensionId => { registerExtension(extensions, extensionId, AMP => { @@ -501,7 +538,10 @@ describes.sandboxed('Extensions', {}, () => { }, parentWin.AMP); const elements = {}; elements[extensionId] = {css: 'a{}'}; - return Promise.resolve({elements}); + return Promise.resolve({ + elements, + services: [], + }); }); let preinstallCount = 0; const promise = extensions.installExtensionsInChildWindow( diff --git a/test/functional/test-service.js b/test/functional/test-service.js index c9f958abf77eb..5f963f9595db4 100644 --- a/test/functional/test-service.js +++ b/test/functional/test-service.js @@ -16,8 +16,8 @@ import { adoptServiceForEmbed, + adoptServiceForEmbedIfEmbeddable, assertDisposable, - assertEmbeddable, disposeServicesForDoc, getExistingServiceForDocInEmbedScope, getExistingServiceInEmbedScope, @@ -568,12 +568,6 @@ describe('service', () => { expect(isEmbeddable(nonEmbeddable)).to.be.false; }); - it('should assert embeddable interface', () => { - expect(assertEmbeddable(embeddable)).to.equal(embeddable); - expect(() => assertEmbeddable(nonEmbeddable)) - .to.throw(/required to implement EmbeddableService/); - }); - it('should adopt embeddable', () => { adoptServiceForEmbed(embedWin, 'embeddable'); expect(embeddable.adoptEmbedWindow).to.be.calledOnce; @@ -586,6 +580,14 @@ describe('service', () => { }).to.throw(/required to implement EmbeddableService/); }); + it('should adopt embeddable if embeddable', () => { + adoptServiceForEmbedIfEmbeddable(embedWin, 'embeddable'); + expect(embeddable.adoptEmbedWindow).to.be.calledOnce; + expect(embeddable.adoptEmbedWindow.args[0][0]).to.equal(embedWin); + + adoptServiceForEmbedIfEmbeddable(embedWin, 'nonEmbeddable'); // No-op. + }); + it('should refuse adopt of unknown service', () => { expect(() => { adoptServiceForEmbed(embedWin, 'unknown');