Skip to content

Commit

Permalink
amp-bind: Fix embedding in FIF (ampproject#9541)
Browse files Browse the repository at this point in the history
* move element service changes from ampproject#9447

* store element service ids in extension struct

* add unit test, fix other tests
  • Loading branch information
William Chou authored and Aaron Turner committed Jun 6, 2017
1 parent 6aa2e8b commit 8ad0227
Show file tree
Hide file tree
Showing 11 changed files with 222 additions and 73 deletions.
9 changes: 7 additions & 2 deletions extensions/amp-bind/0.1/amp-bind.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
2 changes: 1 addition & 1 deletion extensions/amp-bind/0.1/test/test-amp-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 5 additions & 1 deletion src/custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
133 changes: 98 additions & 35 deletions src/element-service.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

import {
getExistingServiceForDocInEmbedScope,
getServicePromise,
getServicePromiseOrNull,
getAmpdoc,
Expand All @@ -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));
}

/**
Expand All @@ -66,24 +61,7 @@ export function getElementServiceIfAvailable(win, id, extension, opt_element) {
if (s) {
return /** @type {!Promise<?Object>} */ (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);
}

/**
Expand Down Expand Up @@ -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));
}

/**
Expand Down Expand Up @@ -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<!Object>}
*/
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<?Object>}
*/
export function getElementServiceIfAvailableForDocInEmbedScope(
nodeOrDoc, id, extension) {
const s = getExistingServiceForDocInEmbedScope(nodeOrDoc, id);
if (s) {
return /** @type {!Promise<?Object>} */ (Promise.resolve(s));
}
if (nodeOrDoc.nodeType) {
const win = /** @type {!Document} */ (
nodeOrDoc.ownerDocument || nodeOrDoc).defaultView;
return elementServicePromiseOrNull(win, id, extension);
}
return /** @type {!Promise<?Object>} */ (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<Object>}
* @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;
});
});
}
5 changes: 5 additions & 0 deletions src/runtime.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {VisibilityState} from './visibility-state';
import {
addDocFactoryToExtension,
addElementToExtension,
addServiceToExtension,
addShadowRootFactoryToExtension,
installBuiltinElements,
installExtensionsInShadowDoc,
Expand Down Expand Up @@ -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);
}


Expand All @@ -548,6 +551,8 @@ function prepareAndRegisterServiceForDocShadowMode(global, extensions,
addDocFactoryToExtension(extensions, ampdoc => {
registerServiceForDoc(ampdoc, name, opt_ctor, opt_factory);
}, name);

addServiceToExtension(extensions, name);
}


Expand Down
25 changes: 15 additions & 10 deletions src/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.`);
}


Expand All @@ -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;
}


Expand Down
4 changes: 1 addition & 3 deletions src/service/ampdoc-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
48 changes: 37 additions & 11 deletions src/service/extensions-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import {
adoptServiceForEmbed,
adoptServiceForEmbedIfEmbeddable,
registerServiceBuilder,
setParentWindow,
} from '../service';
Expand Down Expand Up @@ -57,10 +58,10 @@ let ExtensionElementDef;

/**
* The structure that contains the resources declared by an extension.
* Currently only limitted to elements.
*
* @typedef {{
* elements: !Object<string, !ExtensionElementDef>,
* services: !Array<string>,
* }}
*/
let ExtensionDef;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -389,7 +410,7 @@ export class Extensions {
}

// Adopt embeddable services.
adoptServicesForEmbed(childWin);
adoptStandardServicesForEmbed(childWin);

// Install built-ins and legacy elements.
copyBuiltinElementsToChildWindow(topWin, childWin);
Expand All @@ -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];
Expand All @@ -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);
});
Expand All @@ -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: [],
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 8ad0227

Please sign in to comment.