Disallow passing ampdoc to FIE service getters

extensions/amp-a4a/0.1/test/test-amp-a4a.js

@@ -1918,7 +1918,7 @@ describe('amp-a4a', () => {
  }),
  'Some style is "background: green"').to.be.true;
  expect(frameDoc.body.innerHTML.trim()).to.equal('<p>some text</p>');
- expect(Services.urlReplacementsForDoc(frameDoc))
+ expect(Services.urlReplacementsForDoc(frameDoc.documentElement))
  .to.not.equal(Services.urlReplacementsForDoc(a4aElement));
  });
  });

extensions/amp-a4a/0.1/test/test-real-time-config-manager.js

@@ -62,7 +62,7 @@ describes.realWin('real-time-config-manager', {amp: true}, env => {
 
  // RealTimeConfigManager uses the UrlReplacements service scoped to the A4A
  // (FIE), but for testing stub in the parent service for simplicity.
- const urlReplacements = Services.urlReplacementsForDoc(env.ampdoc);
+ const urlReplacements = Services.urlReplacementsForDoc(element);
  sandbox.stub(Services, 'urlReplacementsForDoc')
  .withArgs(a4aElement.element).returns(urlReplacements);
 

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

@@ -57,10 +57,10 @@ describes.realWin('Linker Manager', {amp: true}, env => {
  // LinkerManager uses Url/UrlReplacements services scoped to the element,
  // but for testing stub in the top-level ampdoc service for simplicity.
  element = {};
- const urlReplacements = Services.urlReplacementsForDoc(ampdoc);
+ const urlReplacements = Services.urlReplacementsForDoc(doc.documentElement);
  sandbox.stub(Services, 'urlReplacementsForDoc')
  .withArgs(element).returns(urlReplacements);
- const url = Services.urlForDoc(ampdoc);
+ const url = Services.urlForDoc(doc.documentElement);
  sandbox.stub(Services, 'urlForDoc').withArgs(element).returns(url);
 
  handlers = [];

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

@@ -162,15 +162,14 @@ describe('amp-analytics.VariableService', function() {
 
  describes.fakeWin('macros', {amp: true}, env => {
  let win;
- let ampdoc;
  let urlReplacementService;
 
  beforeEach(() => {
- ampdoc = env.ampdoc;
  win = env.win;
  installVariableService(win);
  variables = variableServiceFor(win);
- urlReplacementService = Services.urlReplacementsForDoc(ampdoc);
+ const {documentElement} = win.document;
+ urlReplacementService = Services.urlReplacementsForDoc(documentElement);
  });
 
  function check(input, output) {

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

@@ -50,13 +50,11 @@ describes.realWin('amp-analytics', {
  },
 }, function(env) {
  let win, doc;
- let ampdoc;
  let requestVerifier;
 
  beforeEach(() => {
  win = env.win;
  doc = win.document;
- ampdoc = env.ampdoc;
  const wi = mockWindowInterface(env.sandbox);
  wi.getLocation.returns(win.location);
  requestVerifier = new ImagePixelVerifier(wi);
@@ -131,7 +129,7 @@ describes.realWin('amp-analytics', {
  it('should produce request: ' + name +
  '. If this test fails update vendor-requests.json', function* () {
  const urlReplacements =
- Services.urlReplacementsForDoc(ampdoc);
+ Services.urlReplacementsForDoc(doc.documentElement);
  const analytics = getAnalyticsTag(clearVendorOnlyConfig(config));
  sandbox.stub(urlReplacements.getVariableSource(), 'get').callsFake(
  function(name) {

extensions/amp-form/0.1/test/test-amp-form.js

@@ -1443,7 +1443,7 @@ describes.repeated('', {
  it('should install action handler and handle submit action', () => {
  const form = getForm();
  document.body.appendChild(form);
- const actions = Services.actionServiceForDoc(env.ampdoc);
+ const actions = Services.actionServiceForDoc(form);
 
  sandbox.stub(actions, 'installActionHandler');
  const ampForm = new AmpForm(form);

extensions/amp-gwd-animation/0.1/amp-gwd-animation.js

@@ -209,9 +209,8 @@ export class GwdAnimation extends AMP.BaseElement {
  * @private
  */
  executeInvocation_(invocation) {
- const elementOrAmpDoc = this.fie_ ? this.element : this.getAmpDoc();
  const service = user().assert(
- getExistingServiceForDocInEmbedScope(elementOrAmpDoc, GWD_SERVICE_NAME),
+ getExistingServiceForDocInEmbedScope(this.element, GWD_SERVICE_NAME),
  'Cannot execute action because the GWD service is not registered.');
 
  const argPaths = ACTION_IMPL_ARGS[invocation.method];

extensions/amp-gwd-animation/0.1/test/test-amp-gwd-animation.js

@@ -121,7 +121,7 @@ describes.sandboxed('AMP GWD Animation', {}, () => {
  element = el;
  impl = element.implementation_;
  runtime = getExistingServiceForDocInEmbedScope(
- variant.ampdoc == 'fie' ? element : ampdoc, GWD_SERVICE_NAME);
+ element, GWD_SERVICE_NAME);
  page1Elem = doc.getElementById('page1');
  });
  });
@@ -451,8 +451,7 @@ describes.sandboxed('AMP GWD Animation', {}, () => {
  const triggeredAmpEventNames = [];
  const triggeredEvents = [];
 
- const actionService = Services.actionServiceForDoc(
- variant.ampdoc == 'fie' ? element : ampdoc);
+ const actionService = Services.actionServiceForDoc(element);
  sandbox.stub(actionService, 'trigger').callsFake(
  (target, name, event) => {
  triggeredAmpEventNames.push(name);

src/element-service.js

@@ -86,7 +86,7 @@ function isElementScheduled(win, elementName) {
  * implementation loaded. Users should typically wrap this as a special purpose
  * function (e.g. Services.viewportForDoc(...)) for type safety and because the
  * factory should not be passed around.
- * @param {!Element|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
+ * @param {!Element|!ShadowRoot|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
  * @param {string} id of the service.
  * @param {string} extension Name of the custom extension that provides the
  * implementation of this service.
@@ -104,7 +104,7 @@ export function getElementServiceForDoc(elementOrAmpDoc, id, extension,
 /**
  * Same as getElementService but produces null if the given element is not
  * actually available on the current page.
- * @param {!Element|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
+ * @param {!Element|!ShadowRoot|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
  * @param {string} id of the service.
  * @param {string} extension Name of the custom extension that provides the
  * implementation of this service.
@@ -138,35 +138,31 @@ export function getElementServiceIfAvailableForDoc(
 
 /**
  * Returns a promise for service for the given id in the embed scope of
- * a given node, if it exists. Otherwise, falls back to ampdoc scope IFF
- * the given node is in the top-level window.
- * @param {!Element|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
+ * a given element, if it exists. Falls back to ampdoc scope if the element
+ * is not embedded.
+ *
+ * @param {!Element|!ShadowRoot} element
  * @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(
- elementOrAmpDoc, id, extension) {
- const s = getExistingServiceForDocInEmbedScope(elementOrAmpDoc, id);
+ element, id, extension
+) {
+ const s = getExistingServiceForDocInEmbedScope(element, id);
  if (s) {
  return /** @type {!Promise<?Object>} */ (Promise.resolve(s));
  }
- // Return embed-scope element service promise if scheduled.
- if (elementOrAmpDoc.nodeType) {
- const win = toWin(elementOrAmpDoc.ownerDocument.defaultView);
- const topWin = getTopWindow(win);
- // In embeds, doc-scope services are window-scope. But make sure to
- // only do this for embeds (not the top window), otherwise we'd grab
- // a promise from the wrong service holder which would never resolve.
- if (win !== topWin) {
- return getElementServicePromiseOrNull(win, id, extension);
- } else {
- // Fallback to ampdoc IFF the given node is _not_ FIE.
- return getElementServiceIfAvailableForDoc(elementOrAmpDoc, id, extension);
- }
+ const win = toWin(element.ownerDocument.defaultView);
+ const topWin = getTopWindow(win);
+ // In embeds, doc services are stored on the embed window.
+ if (win !== topWin) {
+ return getElementServicePromiseOrNull(win, id, extension);
+ } else {
+ // Only fallback to element's ampdoc (top-level) if not embedded.
+ return getElementServiceIfAvailableForDoc(element, id, extension);
  }
- return /** @type {!Promise<?Object>} */ (Promise.resolve(null));
 }
 
 /**

src/service.js

@@ -78,68 +78,39 @@ export class EmbeddableService {
 }
 
 
-/**
- * Returns a service with the given id. Assumes that it has been registered
- * already.
- * @param {!Window} win
- * @param {string} id
- * @param {boolean=} opt_fallbackToTopWin
- * @return {Object} The service.
- */
-export function getExistingServiceInEmbedScope(win, id, opt_fallbackToTopWin) {
- // First, try to resolve via local (embed) window.
- const local = getLocalExistingServiceForEmbedWinOrNull(win, id);
- if (local) {
- return local;
- }
- if (opt_fallbackToTopWin) {
- return getService(win, id);
- }
- return null;
-}
-
 /**
  * Returns a service with the given id. Assumes that it has been constructed
  * already.
  *
- * Unlike most service getters, passing `Node` is necessary for some FIE-scope
- * services since sometimes we only have the FIE Document for context.
- *
- * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc
+ * @param {!Element|!ShadowRoot} element
  * @param {string} id
  * @param {boolean=} opt_fallbackToTopWin
- * @return {Object} The service.
+ * @return {?Object}
  */
 export function getExistingServiceForDocInEmbedScope(
- nodeOrDoc, id, opt_fallbackToTopWin)
+ element, id, opt_fallbackToTopWin)
 {
- const isAmpDoc = !nodeOrDoc.nodeType;
- // If an ampdoc is passed, resolve via ampdoc.
- if (isAmpDoc) {
- const ampdoc =
- /** @type {!./service/ampdoc-impl.AmpDoc} */ (nodeOrDoc);
- return getServiceForDoc(ampdoc, id);
- }
- // Now, nodeOrDoc must be a node.
- const document =
- /** @type {!Document} */ (nodeOrDoc.ownerDocument || nodeOrDoc);
+ const document = element.ownerDocument;
  const win = toWin(document.defaultView);
  // First, try to resolve via local embed window (if applicable).
  const isEmbed = win != getTopWindow(win);
  if (isEmbed) {
- const local = getLocalExistingServiceForEmbedWinOrNull(win, id);
- if (local) {
- return local;
+ if (isServiceRegistered(win, id)) {
+ const embedService = getServiceInternal(win, id);
+ if (embedService) {
+ return embedService;
+ }
  }
  // Don't continue if fallback is not allowed.
  if (!opt_fallbackToTopWin) {
  return null;
  }
  }
- // If not node is not embedded, resolve via ampdoc.
- return getServiceForDocOrNullInternal(nodeOrDoc, id);
+ // Resolve via the element's ampdoc. This falls back to the top-level service.
+ return getServiceForDocOrNullInternal(element, id);
 }
 
+
 /**
  * Installs a service override on amp-doc level.
  * @param {!Window} embedWin
@@ -150,28 +121,12 @@ export function installServiceInEmbedScope(embedWin, id, service) {
  const topWin = getTopWindow(embedWin);
  dev().assert(embedWin != topWin,
  'Service override can only be installed in embed window: %s', id);
- dev().assert(!getLocalExistingServiceForEmbedWinOrNull(embedWin, id),
+ dev().assert(!isServiceRegistered(embedWin, id),
  'Service override has already been installed: %s', id);
  registerServiceInternal(embedWin, embedWin, id, () => service);
  getServiceInternal(embedWin, id); // Force service to build.
 }
 
-/**
- * @param {!Window} embedWin
- * @param {string} id
- * @return {?Object}
- */
-function getLocalExistingServiceForEmbedWinOrNull(embedWin, id) {
- // Note that this method currently only resolves against the given window.
- // It does not try to go all the way up the parent window chain. We can change
- // this in the future, but for now this gives us a better performance.
- const topWin = getTopWindow(embedWin);
- if (embedWin != topWin && isServiceRegistered(embedWin, id)) {
- return getServiceInternal(embedWin, id);
- } else {
- return null;
- }
-}
 
 /**
  * Registers a service given a class to be used as implementation.
@@ -289,11 +244,11 @@ export function getServiceForDoc(elementOrAmpDoc, id) {
 /**
  * Returns a service for the given id and ampdoc (a per-ampdoc singleton).
  * If service `id` is not registered, returns null.
- * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc
+ * @param {!Element|!ShadowRoot} element
  * @param {string} id
  */
-function getServiceForDocOrNullInternal(nodeOrDoc, id) {
- const ampdoc = getAmpdoc(nodeOrDoc);
+function getServiceForDocOrNullInternal(element, id) {
+ const ampdoc = getAmpdoc(element);
  const holder = getAmpdocServiceHolder(ampdoc);
  if (isServiceRegistered(holder, id)) {
  return getServiceInternal(holder, id);
@@ -303,31 +258,11 @@ function getServiceForDocOrNullInternal(nodeOrDoc, id) {
 }
 
 
-/**
- * tl;dr -- Use getServiceForDoc() instead of this.
- *
- * Privileged variant of getServiceForDoc() that accepts non-element params,
- * e.g. window.document. This is currently necessary for doc-level services
- * used in startup, e.g. Chunks. Eventually we want to remove this function
- * and have callers find the appropriate AmpDoc and use getServiceForDoc().
- *
- * @param {!Node|!./service/ampdoc-impl.AmpDoc} nodeOrDoc
- * @param {string} id
- * @return {T}
- * @template T
- */
-export function getServiceForDocDeprecated(nodeOrDoc, id) {
- const ampdoc = getAmpdoc(nodeOrDoc);
- const holder = getAmpdocServiceHolder(ampdoc);
- return getServiceInternal(holder, id);
-}
-
-
 /**
  * Returns a promise for a service for the given id and ampdoc. Also expects
  * a service that has the actual implementation. The promise resolves when
  * the implementation loaded.
- * @param {!Element|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
+ * @param {!Element|!ShadowRoot|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
  * @param {string} id
  * @return {!Promise<!Object>}
  */
@@ -340,7 +275,7 @@ export function getServicePromiseForDoc(elementOrAmpDoc, id) {
 /**
  * Like getServicePromiseForDoc but returns null if the service was never
  * registered for this ampdoc.
- * @param {!Element|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
+ * @param {!Element|!ShadowRoot|!./service/ampdoc-impl.AmpDoc} elementOrAmpDoc
  * @param {string} id
  * @return {?Promise<!Object>}
  */

src/services.js

@@ -158,7 +158,7 @@ export class Services {
  }
 
  /**
- * @param {!Element} element
+ * @param {!Element|!ShadowRoot} element
  * @return {!Promise<?../extensions/amp-bind/0.1/bind-impl.Bind>}
  */
  static bindForDocOrNull(element) {
@@ -474,7 +474,7 @@ export class Services {
  }
 
  /**
- * @param {!Element} element
+ * @param {!Element|!ShadowRoot} element
  * @return {!./service/url-replacements-impl.UrlReplacements}
  */
  static urlReplacementsForDoc(element) {