ampdoc-fie cleanup: remove getExistingServiceForDocInEmbedScope (#30803)

* ampdoc-fie cleanup: remove getExistingServiceForDocInEmbedScope * cleanup
ampproject · Oct 21, 2020 · 890f3f4 · 890f3f4
1 parent 6418ea9
commit 890f3f4
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 48 deletions.
diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js
@@ -332,7 +332,9 @@ const forbiddenTerms = {
       'extensions/amp-analytics/0.1/instrumentation.js',
       'extensions/amp-analytics/0.1/variables.js',
       'extensions/amp-fx-collection/0.1/providers/fx-provider.js',
+      'extensions/amp-gwd-animation/0.1/amp-gwd-animation.js',
       'src/chunk.js',
+      'src/element-service.js',
       'src/service.js',
       'src/service/cid-impl.js',
       'src/service/origin-experiments-impl.js',

diff --git a/extensions/amp-gwd-animation/0.1/amp-gwd-animation.js b/extensions/amp-gwd-animation/0.1/amp-gwd-animation.js
@@ -22,11 +22,11 @@ import {
 import {CSS} from '../../../build/amp-gwd-animation-0.1.css';
 import {Services} from '../../../src/services';
 import {getDetail} from '../../../src/event-helper';
+import {getFriendlyIframeEmbedOptional} from '../../../src/iframe-helper';
 import {
-  getExistingServiceForDocInEmbedScope,
   getParentWindowFrameElement,
+  getServiceForDocOrNull,
 } from '../../../src/service';
-import {getFriendlyIframeEmbedOptional} from '../../../src/iframe-helper';
 import {userAssert} from '../../../src/log';
 
 /**
@@ -217,7 +217,7 @@ export class GwdAnimation extends AMP.BaseElement {
    */
   executeInvocation_(invocation) {
     const service = userAssert(
-      getExistingServiceForDocInEmbedScope(this.element, GWD_SERVICE_NAME),
+      getServiceForDocOrNull(this.element, GWD_SERVICE_NAME),
       'Cannot execute action because the GWD service is not registered.'
     );
 

diff --git a/extensions/amp-gwd-animation/0.1/test/test-amp-gwd-animation.js b/extensions/amp-gwd-animation/0.1/test/test-amp-gwd-animation.js
@@ -25,7 +25,7 @@ import {
 import {GWD_PAGEDECK_ID, TAG, addAction} from '../amp-gwd-animation';
 import {Services} from '../../../../src/services';
 import {createCustomEvent} from '../../../../src/event-helper';
-import {getExistingServiceForDocInEmbedScope} from '../../../../src/service';
+import {getServiceForDocOrNull} from '../../../../src/service';
 
 describes.sandboxed('AMP GWD Animation', {}, () => {
   /**
@@ -115,10 +115,7 @@ describes.sandboxed('AMP GWD Animation', {}, () => {
             return createGwdAnimationElement(doc, config).then((el) => {
               element = el;
               impl = element.implementation_;
-              runtime = getExistingServiceForDocInEmbedScope(
-                element,
-                GWD_SERVICE_NAME
-              );
+              runtime = getServiceForDocOrNull(element, GWD_SERVICE_NAME);
               page1Elem = doc.getElementById('page1');
             });
           });

diff --git a/src/element-service.js b/src/element-service.js
@@ -17,8 +17,8 @@
 import * as dom from './dom';
 import {
   getAmpdoc,
-  getExistingServiceForDocInEmbedScope,
   getService,
+  getServiceForDocOrNull,
   getServicePromise,
   getServicePromiseForDoc,
   getServicePromiseOrNull,
@@ -159,7 +159,7 @@ export function getElementServiceIfAvailableForDocInEmbedScope(
   id,
   extension
 ) {
-  const s = getExistingServiceForDocInEmbedScope(element, id);
+  const s = getServiceForDocOrNull(element, id);
   if (s) {
     return /** @type {!Promise<?Object>} */ (Promise.resolve(s));
   }

diff --git a/src/service.js b/src/service.js
@@ -56,20 +56,6 @@ export class Disposable {
   dispose() {}
 }
 
-/**
- * Returns a service with the given id. Assumes that it has been constructed
- * already.
- *
- * @param {!Element|!ShadowRoot} element
- * @param {string} id
- * @return {?Object}
- */
-export function getExistingServiceForDocInEmbedScope(element, id) {
-  // TODO(#22733): completely remove this method once ampdoc-fie launches.
-  // Resolve via the element's ampdoc.
-  return getServiceForDocOrNull(element, id);
-}
-
 /**
  * Installs a service override on amp-doc level.
  * @param {!Window} embedWin

diff --git a/src/services.js b/src/services.js
@@ -16,7 +16,6 @@
 
 import {
   getAmpdoc,
-  getExistingServiceForDocInEmbedScope,
   getExistingServiceOrNull,
   getService,
   getServiceForDoc,
@@ -97,7 +96,7 @@ export class Services {
    * @return {!./service/action-impl.ActionService}
    */
   static actionServiceForDoc(element) {
-    return /** @type {!./service/action-impl.ActionService} */ (getExistingServiceForDocInEmbedScope(
+    return /** @type {!./service/action-impl.ActionService} */ (getServiceForDocOrNull(
       element,
       'action'
     ));
@@ -108,7 +107,7 @@ export class Services {
    * @return {!./service/standard-actions-impl.StandardActions}
    */
   static standardActionsForDoc(element) {
-    return /** @type {!./service/standard-actions-impl.StandardActions} */ (getExistingServiceForDocInEmbedScope(
+    return /** @type {!./service/standard-actions-impl.StandardActions} */ (getServiceForDocOrNull(
       element,
       'standard-actions'
     ));
@@ -315,7 +314,7 @@ export class Services {
    * @return {!./service/hidden-observer-impl.HiddenObserver}
    */
   static hiddenObserverForDoc(element) {
-    return /** @type {!./service/hidden-observer-impl.HiddenObserver} */ (getExistingServiceForDocInEmbedScope(
+    return /** @type {!./service/hidden-observer-impl.HiddenObserver} */ (getServiceForDocOrNull(
       element,
       'hidden-observer'
     ));
@@ -581,7 +580,7 @@ export class Services {
    * @return {?./service/localization.LocalizationService}
    */
   static localizationForDoc(element) {
-    return /** @type {?./service/localization.LocalizationService} */ (getExistingServiceForDocInEmbedScope(
+    return /** @type {?./service/localization.LocalizationService} */ (getServiceForDocOrNull(
       element,
       'localization'
     ));
@@ -660,7 +659,7 @@ export class Services {
    * @return {!./service/url-replacements-impl.UrlReplacements}
    */
   static urlReplacementsForDoc(element) {
-    return /** @type {!./service/url-replacements-impl.UrlReplacements} */ (getExistingServiceForDocInEmbedScope(
+    return /** @type {!./service/url-replacements-impl.UrlReplacements} */ (getServiceForDocOrNull(
       element,
       'url-replace'
     ));
@@ -720,7 +719,7 @@ export class Services {
    * @return {!./service/url-impl.Url}
    */
   static urlForDoc(element) {
-    return /** @type {!./service/url-impl.Url} */ (getExistingServiceForDocInEmbedScope(
+    return /** @type {!./service/url-impl.Url} */ (getServiceForDocOrNull(
       element,
       'url'
     ));

diff --git a/test/unit/test-service.js b/test/unit/test-service.js
@@ -17,11 +17,11 @@
 import {
   assertDisposable,
   disposeServicesForDoc,
-  getExistingServiceForDocInEmbedScope,
   getExistingServiceOrNull,
   getParentWindowFrameElement,
   getService,
   getServiceForDoc,
+  getServiceForDocOrNull,
   getServicePromise,
   getServicePromiseForDoc,
   getServicePromiseOrNull,
@@ -505,18 +505,15 @@ describe('service', () => {
       });
 
       it('should return the service via node', () => {
-        const fromNode = getExistingServiceForDocInEmbedScope(node, 'c');
+        const fromNode = getServiceForDocOrNull(node, 'c');
         expect(fromNode).to.equal(topService);
       });
 
       it('should find ampdoc and return its service', () => {
-        const fromChildNode = getExistingServiceForDocInEmbedScope(
-          childWinNode,
-          'c'
-        );
+        const fromChildNode = getServiceForDocOrNull(childWinNode, 'c');
         expect(fromChildNode).to.equal(topService);
 
-        const fromGrandchildNode = getExistingServiceForDocInEmbedScope(
+        const fromGrandchildNode = getServiceForDocOrNull(
           grandChildWinNode,
           'c'
         );
@@ -534,13 +531,10 @@ describe('service', () => {
           }
           return ampdoc;
         });
-        const fromChildNode = getExistingServiceForDocInEmbedScope(
-          childWinNode,
-          'c'
-        );
+        const fromChildNode = getServiceForDocOrNull(childWinNode, 'c');
         expect(fromChildNode).to.equal(null);
 
-        const fromGrandchildNode = getExistingServiceForDocInEmbedScope(
+        const fromGrandchildNode = getServiceForDocOrNull(
           grandChildWinNode,
           'c'
         );
@@ -559,14 +553,11 @@ describe('service', () => {
           }
           return ampdoc;
         });
-        const fromChildNode = getExistingServiceForDocInEmbedScope(
-          childWinNode,
-          'c'
-        );
+        const fromChildNode = getServiceForDocOrNull(childWinNode, 'c');
         expect(fromChildNode).to.deep.equal({count: 2});
         expect(fromChildNode).to.not.equal(topService);
 
-        const fromGrandchildNode = getExistingServiceForDocInEmbedScope(
+        const fromGrandchildNode = getServiceForDocOrNull(
           grandChildWinNode,
           'c'
         );