fix service tests

ampproject · Jun 27, 2017 · f34485f · f34485f
1 parent 0c5129e
commit f34485f
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 23 deletions.
diff --git a/src/element-service.js b/src/element-service.js
@@ -62,7 +62,7 @@ export function getElementServiceIfAvailable(win, id, extension, opt_element) {
   if (s) {
     return /** @type {!Promise<?Object>} */ (s);
   }
-  return elementServicePromiseOrNull(win, id, extension, opt_element);
+  return getElementServicePromiseOrNull(win, id, extension, opt_element);
 }
 
 /**
@@ -182,7 +182,7 @@ export function getElementServiceIfAvailableForDocInEmbedScope(
     const topWin = getTopWindow(win);
     // Don't return promise unless this is definitely FIE to avoid covfefe.
     if (win !== topWin) {
-      return elementServicePromiseOrNull(win, id, extension);
+      return getElementServicePromiseOrNull(win, id, extension);
     }
   }
   return /** @type {!Promise<?Object>} */ (Promise.resolve(null));
@@ -214,7 +214,7 @@ function assertService(service, id, extension) {
  * @return {!Promise<Object>}
  * @private
  */
-function elementServicePromiseOrNull(win, id, extension, opt_element) {
+function getElementServicePromiseOrNull(win, id, extension, opt_element) {
   // Microtask is necessary to ensure that window.ampExtendedElements has been
   // initialized.
   return Promise.resolve().then(() => {

diff --git a/src/service.js b/src/service.js
@@ -115,7 +115,8 @@ export function getExistingServiceForDocInEmbedScope(
       return local;
     }
   }
-  if (opt_fallbackToTopWin) {
+  // If an ampdoc is passed or fallback is allowed, continue resolving.
+  if (!nodeOrDoc.nodeType || opt_fallbackToTopWin) {
     return getServiceForDoc(nodeOrDoc, id);
   }
   return null;

diff --git a/test/functional/test-service.js b/test/functional/test-service.js
@@ -242,16 +242,25 @@ describe('service', () => {
         topService = getService(window, 'c');
       });
 
-      it('should return top service for top window', () => {
-        expect(getExistingServiceInEmbedScope(window, 'c'))
-            .to.equal(topService);
+      it('should not fall back to top window service by default', () => {
+        const service = getExistingServiceInEmbedScope(window, 'c');
+        expect(service).to.be.null;
       });
 
-      it('should return top service when not overriden', () => {
-        expect(getExistingServiceInEmbedScope(childWin, 'c'))
-            .to.equal(topService);
-        expect(getExistingServiceInEmbedScope(grandchildWin, 'c'))
-            .to.equal(topService);
+      it('should fall back top window service', () => {
+        const service = getExistingServiceInEmbedScope(
+            window, 'c', /* opt_fallbackToTopWin */ true);
+        expect(service).to.equal(topService);
+      });
+
+      it('should fall back to top service when not overriden', () => {
+        const childService = getExistingServiceInEmbedScope(
+            childWin, 'c', /* opt_fallbackToTopWin */ true);
+        expect(childService).to.equal(topService);
+
+        const grandchildService = getExistingServiceInEmbedScope(
+            grandchildWin, 'c', /* opt_fallbackToTopWin */ true);
+        expect(grandchildService).to.equal(topService);
       });
 
       it('should return overriden service', () => {
@@ -509,16 +518,28 @@ describe('service', () => {
         topService = getServiceForDoc(ampdoc, 'c');
       });
 
-      it('should return top service for ampdoc', () => {
-        expect(getExistingServiceForDocInEmbedScope(ampdoc, 'c'))
-            .to.equal(topService);
+      it('should not fall back to top ampdoc service with node', () => {
+        const service = getExistingServiceForDocInEmbedScope(node, 'c');
+        expect(service).to.be.null;
       });
 
-      it('should return top service when not overriden', () => {
-        expect(getExistingServiceForDocInEmbedScope(childWinNode, 'c'))
-            .to.equal(topService);
-        expect(getExistingServiceForDocInEmbedScope(grandChildWinNode, 'c'))
-            .to.equal(topService);
+      it('should fall back to top ampdoc service', () => {
+        const fromAmpdoc = getExistingServiceForDocInEmbedScope(ampdoc, 'c');
+        expect(fromAmpdoc).to.equal(topService);
+
+        const fromNodeWithFallback = getExistingServiceForDocInEmbedScope(
+            node, 'c', /* opt_fallbackToTopWin */ true);
+        expect(fromNodeWithFallback).to.equal(topService);
+      });
+
+      it('should fall back to top ampdoc service when not overriden', () => {
+        const childService = getExistingServiceForDocInEmbedScope(
+            childWinNode, 'c', /* opt_fallbackToTopWin */ true);
+        expect(childService).to.equal(topService);
+
+        const grandchildService = getExistingServiceForDocInEmbedScope(
+            grandChildWinNode, 'c', /* opt_fallbackToTopWin */ true);
+        expect(grandchildService).to.equal(topService);
       });
 
       it('should return overriden service', () => {
@@ -530,14 +551,18 @@ describe('service', () => {
         // Top-level service doesn't change.
         expect(getExistingServiceForDocInEmbedScope(ampdoc, 'c'))
             .to.equal(topService);
-        expect(getExistingServiceForDocInEmbedScope(node, 'c'))
-            .to.equal(topService);
+        expect(getExistingServiceForDocInEmbedScope(
+            node, 'c', /* opt_fallbackToTopWin */ true))
+                .to.equal(topService);
 
         // Notice that only direct overrides are allowed for now. This is
         // arbitrary can change in the future to allow hierarchical lookup
         // up the window chain.
         expect(getExistingServiceForDocInEmbedScope(grandChildWinNode, 'c'))
-            .to.equal(topService);
+            .to.be.null;
+        expect(getExistingServiceForDocInEmbedScope(
+            grandChildWinNode, 'c', /* opt_fallbackToTopWin */ true))
+                .to.equal(topService);
       });
     });