diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js index 5103c3b446416..f4212c0b7555e 100644 --- a/build-system/tasks/presubmit-checks.js +++ b/build-system/tasks/presubmit-checks.js @@ -606,6 +606,10 @@ const forbiddenTerms = { 'src/service/resources-impl.js', ], }, + '\\.scheduleLayoutOrPreload\\(': { + message: 'scheduleLayoutOrPreload is a restricted API.', + whitelist: ['src/service/owners-impl.js', 'src/service/resources-impl.js'], + }, '(win|Win)(dow)?(\\(\\))?\\.open\\W': { message: 'Use dom.openWindowDialog', whitelist: ['src/dom.js'], diff --git a/src/inabox/inabox-services.js b/src/inabox/inabox-services.js index be007400e9302..e0e256410e625 100644 --- a/src/inabox/inabox-services.js +++ b/src/inabox/inabox-services.js @@ -23,10 +23,8 @@ import {installHistoryServiceForDoc} from '../service/history-impl'; import {installIframeMessagingClient} from './inabox-iframe-messaging-client'; import {installInaboxCidService} from './inabox-cid'; import {installInaboxViewportService} from './inabox-viewport'; -import { - installOwnersServiceForDoc, - installResourcesServiceForDoc, -} from '../service/resources-impl'; +import {installOwnersServiceForDoc} from '../service/owners-impl'; +import {installResourcesServiceForDoc} from '../service/resources-impl'; import {installStandardActionsForDoc} from '../service/standard-actions-impl'; import {installUrlForDoc} from '../service/url-impl'; import {installUrlReplacementsServiceForDoc} from '../service/url-replacements-impl'; diff --git a/src/service/core-services.js b/src/service/core-services.js index 86f935df160b8..7aba14d28898a 100644 --- a/src/service/core-services.js +++ b/src/service/core-services.js @@ -28,12 +28,10 @@ import {installHistoryServiceForDoc} from './history-impl'; import {installImg} from '../../builtins/amp-img'; import {installInputService} from '../input'; import {installLayout} from '../../builtins/amp-layout'; -import { - installOwnersServiceForDoc, - installResourcesServiceForDoc, -} from './resources-impl'; +import {installOwnersServiceForDoc} from './owners-impl'; import {installPixel} from '../../builtins/amp-pixel'; import {installPlatformService} from './platform-impl'; +import {installResourcesServiceForDoc} from './resources-impl'; import {installStandardActionsForDoc} from './standard-actions-impl'; import {installStorageServiceForDoc} from './storage-impl'; import {installTemplatesService} from './template-impl'; diff --git a/src/service/owners-impl.js b/src/service/owners-impl.js index 3e990a72c7762..1931045e49eb7 100644 --- a/src/service/owners-impl.js +++ b/src/service/owners-impl.js @@ -15,12 +15,38 @@ */ /* eslint-disable no-unused-vars */ +import {Resource, ResourceState} from './resource'; +import {Services} from '../services'; +import {dev, devAssert} from '../log'; +import {isArray} from '../types'; +import {registerServiceBuilderForDoc} from '../service'; + +const TAG_ = 'Owners'; + +/** + * @param {!Element|!Array} elements + * @return {!Array} + */ +function elements_(elements) { + return /** @type {!Array} */ (isArray(elements) + ? elements + : [elements]); +} + /** * TODO(lannka): migrate all methods from OwnersDef to here. * * @interface */ export class Owners { + /** + * @param {!./ampdoc-impl.AmpDoc} ampdoc + */ + constructor(ampdoc) { + /** @const @private {!./resources-impl.ResourcesDef} */ + this.resources_ = Services.resourcesForDoc(ampdoc); + } + /** * Assigns an owner for the specified element. This means that the resources * within this element will be managed by the owner and not Resources manager. @@ -28,7 +54,9 @@ export class Owners { * @param {!AmpElement} owner * @package */ - setOwner(element, owner) {} + setOwner(element, owner) { + Resource.setOwner(element, owner); + } /** * Schedules preload for the specified sub-elements that are children of the @@ -39,7 +67,13 @@ export class Owners { * @param {!Element} parentElement * @param {!Element|!Array} subElements */ - schedulePreload(parentElement, subElements) {} + schedulePreload(parentElement, subElements) { + this.scheduleLayoutOrPreloadForSubresources_( + Resource.forElement(parentElement), + /* layout */ false, + elements_(subElements) + ); + } /** * Schedules layout for the specified sub-elements that are children of the @@ -50,7 +84,13 @@ export class Owners { * @param {!Element} parentElement * @param {!Element|!Array} subElements */ - scheduleLayout(parentElement, subElements) {} + scheduleLayout(parentElement, subElements) { + this.scheduleLayoutOrPreloadForSubresources_( + Resource.forElement(parentElement), + /* layout */ true, + elements_(subElements) + ); + } /** * Invokes `unload` on the elements' resource which in turn will invoke @@ -59,7 +99,14 @@ export class Owners { * @param {!Element} parentElement * @param {!Element|!Array} subElements */ - schedulePause(parentElement, subElements) {} + schedulePause(parentElement, subElements) { + const parentResource = Resource.forElement(parentElement); + subElements = elements_(subElements); + + this.discoverResourcesForArray_(parentResource, subElements, resource => { + resource.pause(); + }); + } /** * Invokes `resume` on the elements' resource which in turn will invoke @@ -68,7 +115,14 @@ export class Owners { * @param {!Element} parentElement * @param {!Element|!Array} subElements */ - scheduleResume(parentElement, subElements) {} + scheduleResume(parentElement, subElements) { + const parentResource = Resource.forElement(parentElement); + subElements = elements_(subElements); + + this.discoverResourcesForArray_(parentResource, subElements, resource => { + resource.resume(); + }); + } /** * Schedules unlayout for specified sub-elements that are children of the @@ -77,7 +131,14 @@ export class Owners { * @param {!Element} parentElement * @param {!Element|!Array} subElements */ - scheduleUnlayout(parentElement, subElements) {} + scheduleUnlayout(parentElement, subElements) { + const parentResource = Resource.forElement(parentElement); + subElements = elements_(subElements); + + this.discoverResourcesForArray_(parentResource, subElements, resource => { + resource.unlayout(); + }); + } /** * A parent resource, especially in when it's an owner (see {@link setOwner}), @@ -88,6 +149,126 @@ export class Owners { * @param {!Element|!Array} subElements * @param {boolean} inLocalViewport */ - updateInViewport(parentElement, subElements, inLocalViewport) {} + updateInViewport(parentElement, subElements, inLocalViewport) { + this.updateInViewportForSubresources_( + Resource.forElement(parentElement), + elements_(subElements), + inLocalViewport + ); + } + + /** + * @param {!Resource} resource + * @param {boolean} layout + * @param {number} parentPriority + * @private + */ + measureAndScheduleIfAllowed_(resource, layout, parentPriority) { + resource.measure(); + if ( + resource.getState() === ResourceState.READY_FOR_LAYOUT && + resource.isDisplayed() + ) { + this.resources_.scheduleLayoutOrPreload(resource, layout, parentPriority); + } + } + + /** + * Schedules layout or preload for the sub-resources of the specified + * resource. + * @param {!Resource} parentResource + * @param {boolean} layout + * @param {!Array} subElements + * @private + */ + scheduleLayoutOrPreloadForSubresources_(parentResource, layout, subElements) { + this.discoverResourcesForArray_(parentResource, subElements, resource => { + if (resource.getState() === ResourceState.NOT_BUILT) { + resource.whenBuilt().then(() => { + this.measureAndScheduleIfAllowed_( + resource, + layout, + parentResource.getLayoutPriority() + ); + }); + } else { + this.measureAndScheduleIfAllowed_( + resource, + layout, + parentResource.getLayoutPriority() + ); + } + }); + } + + /** + * Finds resources within the parent resource's shallow subtree. + * @param {!Resource} parentResource + * @param {!Array} elements + * @param {function(!Resource)} callback + */ + discoverResourcesForArray_(parentResource, elements, callback) { + elements.forEach(element => { + devAssert(parentResource.element.contains(element)); + this.discoverResourcesForElement_(element, callback); + }); + } + + /** + * Updates inViewport state for the specified sub-resources of a resource. + * @param {!Resource} parentResource + * @param {!Array} subElements + * @param {boolean} inLocalViewport + * @private + */ + updateInViewportForSubresources_( + parentResource, + subElements, + inLocalViewport + ) { + const inViewport = parentResource.isInViewport() && inLocalViewport; + this.discoverResourcesForArray_(parentResource, subElements, resource => { + resource.setInViewport(inViewport); + }); + } + + /** + * @param {!Element} element + * @param {function(!Resource)} callback + */ + discoverResourcesForElement_(element, callback) { + // Breadth-first search. + if (element.classList.contains('i-amphtml-element')) { + callback(Resource.forElement(element)); + // Also schedule amp-element that is a placeholder for the element. + const placeholder = element.getPlaceholder(); + if (placeholder) { + this.discoverResourcesForElement_(placeholder, callback); + } + } else { + const ampElements = element.getElementsByClassName('i-amphtml-element'); + const seen = []; + for (let i = 0; i < ampElements.length; i++) { + const ampElement = ampElements[i]; + let covered = false; + for (let j = 0; j < seen.length; j++) { + if (seen[j].contains(ampElement)) { + covered = true; + break; + } + } + if (!covered) { + seen.push(ampElement); + callback(Resource.forElement(ampElement)); + } + } + } + } +} + +/** + * @param {!./ampdoc-impl.AmpDoc} ampdoc + */ +export function installOwnersServiceForDoc(ampdoc) { + registerServiceBuilderForDoc(ampdoc, 'owners', Owners); } -/* eslint-enable no-unused-vars */ diff --git a/src/service/resources-impl.js b/src/service/resources-impl.js index 5112030dda315..783a33beb3827 100644 --- a/src/service/resources-impl.js +++ b/src/service/resources-impl.js @@ -16,7 +16,6 @@ import {FiniteStateMachine} from '../finite-state-machine'; import {FocusHistory} from '../focus-history'; -import {Owners} from './owners-impl'; import {Pass} from '../pass'; import {Resource, ResourceState} from './resource'; import {Services} from '../services'; @@ -29,7 +28,6 @@ import {dev, devAssert} from '../log'; import {dict, hasOwn} from '../utils/object'; import {getSourceUrl} from '../url'; import {checkAndFix as ieMediaCheckAndFix} from './ie-media-bug'; -import {isArray} from '../types'; import {isBlockedByConsent, reportError} from '../error'; import {isExperimentOn} from '../experiments'; import {loadPromise} from '../event-helper'; @@ -75,7 +73,7 @@ let ChangeSizeRequestDef; /** * @interface */ -class MutatorsAndOwnersDef extends Owners { +class MutatorsDef { /** * Requests the runtime to change the element's size. When the size is * successfully updated then the opt_callback is called. @@ -176,7 +174,7 @@ class MutatorsAndOwnersDef extends Owners { /** * @interface */ -export class ResourcesDef extends MutatorsAndOwnersDef { +export class ResourcesDef extends MutatorsDef { /** * Returns a list of resources. * @return {!Array} @@ -298,12 +296,25 @@ export class ResourcesDef extends MutatorsAndOwnersDef { * @param {number} newLayoutPriority */ updateLayoutPriority(element, newLayoutPriority) {} + + /** + * Schedules layout or preload for the specified resource. + * @param {!Resource} resource + * @param {boolean} layout + * @param {number=} opt_parentPriority + * @param {boolean=} opt_forceOutsideViewport + */ + scheduleLayoutOrPreload( + resource, + layout, + opt_parentPriority, + opt_forceOutsideViewport + ) {} } /* eslint-enable no-unused-vars */ /** * @implements {ResourcesDef} - * @implements {./owners-impl.Owners} */ export class Resources { /** @@ -840,11 +851,6 @@ export class Resources { dev().fine(TAG_, 'element upgraded:', resource.debugid); } - /** @override */ - setOwner(element, owner) { - Resource.setOwner(element, owner); - } - /** @override */ requireLayout(element, opt_parentPriority) { const promises = []; @@ -859,7 +865,7 @@ export class Resources { if (!resource.isDisplayed()) { return; } - this.scheduleLayoutOrPreload_( + this.scheduleLayoutOrPreload( resource, /* layout */ true, opt_parentPriority, @@ -875,54 +881,6 @@ export class Resources { return Promise.all(promises); } - /** @override */ - scheduleLayout(parentElement, subElements) { - this.scheduleLayoutOrPreloadForSubresources_( - Resource.forElement(parentElement), - /* layout */ true, - elements_(subElements) - ); - } - - /** @override */ - schedulePause(parentElement, subElements) { - const parentResource = Resource.forElement(parentElement); - subElements = elements_(subElements); - - this.discoverResourcesForArray_(parentResource, subElements, resource => { - resource.pause(); - }); - } - - /** @override */ - scheduleResume(parentElement, subElements) { - const parentResource = Resource.forElement(parentElement); - subElements = elements_(subElements); - - this.discoverResourcesForArray_(parentResource, subElements, resource => { - resource.resume(); - }); - } - - /** @override */ - scheduleUnlayout(parentElement, subElements) { - const parentResource = Resource.forElement(parentElement); - subElements = elements_(subElements); - - this.discoverResourcesForArray_(parentResource, subElements, resource => { - resource.unlayout(); - }); - } - - /** @override */ - schedulePreload(parentElement, subElements) { - this.scheduleLayoutOrPreloadForSubresources_( - Resource.forElement(parentElement), - /* layout */ false, - elements_(subElements) - ); - } - /** @override */ updateLayoutPriority(element, newLayoutPriority) { const resource = Resource.forElement(element); @@ -940,12 +898,42 @@ export class Resources { } /** @override */ - updateInViewport(parentElement, subElements, inLocalViewport) { - this.updateInViewportForSubresources_( - Resource.forElement(parentElement), - elements_(subElements), - inLocalViewport + scheduleLayoutOrPreload( + resource, + layout, + opt_parentPriority, + opt_forceOutsideViewport + ) { + devAssert( + resource.getState() != ResourceState.NOT_BUILT && resource.isDisplayed(), + 'Not ready for layout: %s (%s)', + resource.debugid, + resource.getState() ); + const forceOutsideViewport = opt_forceOutsideViewport || false; + if (!this.isLayoutAllowed_(resource, forceOutsideViewport)) { + return; + } + + if (layout) { + this.schedule_( + resource, + LAYOUT_TASK_ID_, + LAYOUT_TASK_OFFSET_, + opt_parentPriority || 0, + forceOutsideViewport, + resource.startLayout.bind(resource) + ); + } else { + this.schedule_( + resource, + PRELOAD_TASK_ID_, + PRELOAD_TASK_OFFSET_, + opt_parentPriority || 0, + forceOutsideViewport, + resource.startLayout.bind(resource) + ); + } } /** @override */ @@ -1787,7 +1775,7 @@ export class Resources { for (let i = 0; i < this.resources_.length; i++) { const r = this.resources_[i]; // TODO(dvoytenko): This extra build has to be merged with the - // scheduleLayoutOrPreload_ method below. + // scheduleLayoutOrPreload method below. // Force build for all resources visible, measured, and in the viewport. if ( !r.isBuilt() && @@ -1810,7 +1798,7 @@ export class Resources { // layers. This is currently a short-term fix to the problem that // the fixed elements get incorrect top coord. if (r.isDisplayed() && r.overlaps(loadRect)) { - this.scheduleLayoutOrPreload_(r, /* layout */ true); + this.scheduleLayoutOrPreload(r, /* layout */ true); } } } @@ -1837,7 +1825,7 @@ export class Resources { r.idleRenderOutsideViewport() ) { dev().fine(TAG_, 'idleRenderOutsideViewport layout:', r.debugid); - this.scheduleLayoutOrPreload_(r, /* layout */ false); + this.scheduleLayoutOrPreload(r, /* layout */ false); idleScheduledCount++; } } @@ -1855,7 +1843,7 @@ export class Resources { r.isDisplayed() ) { dev().fine(TAG_, 'idle layout:', r.debugid); - this.scheduleLayoutOrPreload_(r, /* layout */ false); + this.scheduleLayoutOrPreload(r, /* layout */ false); idleScheduledCount++; } } @@ -2298,96 +2286,6 @@ export class Resources { return true; } - /** - * Schedules layout or preload for the specified resource. - * @param {!Resource} resource - * @param {boolean} layout - * @param {number=} opt_parentPriority - * @param {boolean=} opt_forceOutsideViewport - * @private - */ - scheduleLayoutOrPreload_( - resource, - layout, - opt_parentPriority, - opt_forceOutsideViewport - ) { - devAssert( - resource.getState() != ResourceState.NOT_BUILT && resource.isDisplayed(), - 'Not ready for layout: %s (%s)', - resource.debugid, - resource.getState() - ); - const forceOutsideViewport = opt_forceOutsideViewport || false; - if (!this.isLayoutAllowed_(resource, forceOutsideViewport)) { - return; - } - - if (layout) { - this.schedule_( - resource, - LAYOUT_TASK_ID_, - LAYOUT_TASK_OFFSET_, - opt_parentPriority || 0, - forceOutsideViewport, - resource.startLayout.bind(resource) - ); - } else { - this.schedule_( - resource, - PRELOAD_TASK_ID_, - PRELOAD_TASK_OFFSET_, - opt_parentPriority || 0, - forceOutsideViewport, - resource.startLayout.bind(resource) - ); - } - } - - /** - * Schedules layout or preload for the sub-resources of the specified - * resource. - * @param {!Resource} parentResource - * @param {boolean} layout - * @param {!Array} subElements - * @private - */ - scheduleLayoutOrPreloadForSubresources_(parentResource, layout, subElements) { - this.discoverResourcesForArray_(parentResource, subElements, resource => { - if (resource.getState() == ResourceState.NOT_BUILT) { - resource.whenBuilt().then(() => { - this.measureAndScheduleIfAllowed_( - resource, - layout, - parentResource.getLayoutPriority() - ); - }); - } else { - this.measureAndScheduleIfAllowed_( - resource, - layout, - parentResource.getLayoutPriority() - ); - } - }); - } - - /** - * @param {!Resource} resource - * @param {boolean} layout - * @param {number} parentPriority - * @private - */ - measureAndScheduleIfAllowed_(resource, layout, parentPriority) { - resource.measure(); - if ( - resource.getState() == ResourceState.READY_FOR_LAYOUT && - resource.isDisplayed() - ) { - this.scheduleLayoutOrPreload_(resource, layout, parentPriority); - } - } - /** * Schedules a task. * @param {!Resource} resource @@ -2434,70 +2332,6 @@ export class Resources { task.resource.layoutScheduled(task.scheduleTime); } - /** - * Updates inViewport state for the specified sub-resources of a resource. - * @param {!Resource} parentResource - * @param {!Array} subElements - * @param {boolean} inLocalViewport - * @private - */ - updateInViewportForSubresources_( - parentResource, - subElements, - inLocalViewport - ) { - const inViewport = parentResource.isInViewport() && inLocalViewport; - this.discoverResourcesForArray_(parentResource, subElements, resource => { - resource.setInViewport(inViewport); - }); - } - - /** - * Finds resources within the parent resource's shallow subtree. - * @param {!Resource} parentResource - * @param {!Array} elements - * @param {function(!Resource)} callback - */ - discoverResourcesForArray_(parentResource, elements, callback) { - elements.forEach(element => { - devAssert(parentResource.element.contains(element)); - this.discoverResourcesForElement_(element, callback); - }); - } - - /** - * @param {!Element} element - * @param {function(!Resource)} callback - */ - discoverResourcesForElement_(element, callback) { - // Breadth-first search. - if (element.classList.contains('i-amphtml-element')) { - callback(Resource.forElement(element)); - // Also schedule amp-element that is a placeholder for the element. - const placeholder = element.getPlaceholder(); - if (placeholder) { - this.discoverResourcesForElement_(placeholder, callback); - } - } else { - const ampElements = element.getElementsByClassName('i-amphtml-element'); - const seen = []; - for (let i = 0; i < ampElements.length; i++) { - const ampElement = ampElements[i]; - let covered = false; - for (let j = 0; j < seen.length; j++) { - if (seen[j].contains(ampElement)) { - covered = true; - break; - } - } - if (!covered) { - seen.push(ampElement); - callback(Resource.forElement(ampElement)); - } - } - } - } - /** * Calls iterator on each sub-resource * @param {!FiniteStateMachine} vsm @@ -2627,16 +2461,6 @@ export class Resources { } } -/** - * @param {!Element|!Array} elements - * @return {!Array} - */ -function elements_(elements) { - return /** @type {!Array} */ (isArray(elements) - ? elements - : [elements]); -} - /** * The internal structure of a ChangeHeightRequest. * @typedef {{ @@ -2653,12 +2477,3 @@ export let SizeDef; export function installResourcesServiceForDoc(ampdoc) { registerServiceBuilderForDoc(ampdoc, 'resources', Resources); } - -/** - * @param {!./ampdoc-impl.AmpDoc} ampdoc - */ -export function installOwnersServiceForDoc(ampdoc) { - registerServiceBuilderForDoc(ampdoc, 'owners', ampdoc => { - return Services.resourcesForDoc(ampdoc); - }); -}