From 2216206c4fc1f453f2952dabaaacbb3683700d71 Mon Sep 17 00:00:00 2001 From: William Chou Date: Tue, 27 Jun 2017 16:19:04 -0400 Subject: [PATCH] fix service tests --- src/element-service.js | 6 ++-- src/service.js | 3 +- test/functional/test-service.js | 59 +++++++++++++++++++++++---------- 3 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/element-service.js b/src/element-service.js index 3c35f06e00ea..db1080a9b29b 100644 --- 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} */ (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} */ (Promise.resolve(null)); @@ -214,7 +214,7 @@ function assertService(service, id, extension) { * @return {!Promise} * @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 index fc9495579fdb..d9ad8c4821b4 100644 --- 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 index 3c8aa8121527..f85b27bb8c6e 100644 --- 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,13 +551,17 @@ describe('service', () => { // Top-level service doesn't change. expect(getExistingServiceForDocInEmbedScope(ampdoc, 'c')) .to.equal(topService); - expect(getExistingServiceForDocInEmbedScope(node, 'c')) + 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.be.null; + expect(getExistingServiceForDocInEmbedScope( + grandChildWinNode, 'c', /* opt_fallbackToTopWin */ true)) .to.equal(topService); }); });