Skip to content

Commit

Permalink
Move mutator interfaces to a separate service
Browse files Browse the repository at this point in the history
  • Loading branch information
powerivq committed Oct 31, 2019
1 parent af45c7e commit 0d66374
Show file tree
Hide file tree
Showing 41 changed files with 2,318 additions and 2,220 deletions.
27 changes: 17 additions & 10 deletions build-system/tasks/presubmit-checks.js
Expand Up @@ -223,6 +223,14 @@ const forbiddenTerms = {
'testing/iframe.js',
],
},
'installMutatorServiceForDoc': {
message: privateServiceFactory,
whitelist: [
'src/inabox/inabox-services.js',
'src/service/core-services.js',
'src/service/resources-impl.js',
],
},
'installPerformanceService': {
message: privateServiceFactory,
whitelist: [
Expand All @@ -232,6 +240,14 @@ const forbiddenTerms = {
'src/service/performance-impl.js',
],
},
'installResourcesServiceForDoc': {
message: privateServiceFactory,
whitelist: [
'src/inabox/inabox-services.js',
'src/service/core-services.js',
'src/service/resources-impl.js',
],
},
'installStorageServiceForDoc': {
message: privateServiceFactory,
whitelist: [
Expand Down Expand Up @@ -283,15 +299,6 @@ const forbiddenTerms = {
'src/service/vsync-impl.js',
],
},
'installResourcesServiceForDoc': {
message: privateServiceFactory,
whitelist: [
'src/inabox/inabox-services.js',
'src/service/core-services.js',
'src/service/resources-impl.js',
'src/service/standard-actions-impl.js',
],
},
'installXhrService': {
message: privateServiceFactory,
whitelist: [
Expand Down Expand Up @@ -567,7 +574,7 @@ const forbiddenTerms = {
},
'\\.schedulePass\\(': {
message: 'schedulePass is heavy, think twice before using it',
whitelist: ['src/service/resources-impl.js'],
whitelist: ['src/service/mutator-impl.js', 'src/service/resources-impl.js'],
},
'\\.requireLayout\\(': {
message:
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-access/0.1/amp-access.js
Expand Up @@ -92,8 +92,8 @@ export class AccessService {
/** @private @const {!../../../src/service/template-impl.Templates} */
this.templates_ = Services.templatesFor(ampdoc.win);

/** @private @const {!../../../src/service/resources-interface.ResourcesInterface} */
this.resources_ = Services.resourcesForDoc(ampdoc);
/** @private @const {!../../../src/service/mutator-interface.MutatorInterface} */
this.mutator_ = Services.mutatorForDoc(ampdoc);

/** @private @const {?../../../src/service/performance-impl.Performance} */
this.performance_ = Services.performanceForOrNull(ampdoc.win);
Expand Down Expand Up @@ -476,7 +476,7 @@ export class AccessService {
if (on == wasOn) {
return Promise.resolve();
}
return this.resources_.mutateElement(element, () => {
return this.mutator_.mutateElement(element, () => {
if (on) {
element.removeAttribute('amp-access-hide');
} else {
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-access/0.1/test/test-amp-access.js
Expand Up @@ -321,7 +321,7 @@ describes.fakeWin(
adapterMock = sandbox.mock(adapter);

sandbox
.stub(service.resources_, 'mutateElement')
.stub(service.mutator_, 'mutateElement')
.callsFake((unusedElement, mutator) => {
mutator();
return Promise.resolve();
Expand Down Expand Up @@ -588,7 +588,7 @@ describes.fakeWin(
service = new AccessService(ampdoc);

mutateElementStub = sandbox
.stub(service.resources_, 'mutateElement')
.stub(service.mutator_, 'mutateElement')
.callsFake((unusedElement, mutator) => {
mutator();
return Promise.resolve();
Expand Down Expand Up @@ -1785,7 +1785,7 @@ describes.fakeWin(
adapterDonutsMock = sandbox.mock(adapterDonuts);

sandbox
.stub(service.resources_, 'mutateElement')
.stub(service.mutator_, 'mutateElement')
.callsFake((unusedElement, mutator) => {
mutator();
return Promise.resolve();
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-ad/0.1/amp-ad-ui.js
Expand Up @@ -96,7 +96,7 @@ export class AmpAdUIHandler {
if (this.containerElement_) {
// Collapse the container element if there's one
attemptCollapsePromise = this.element_
.getResources()
.getMutator()
.attemptCollapse(this.containerElement_);
attemptCollapsePromise.then(() => {});
} else {
Expand Down
6 changes: 3 additions & 3 deletions extensions/amp-auto-ads/0.1/placement.js
Expand Up @@ -126,8 +126,8 @@ export class Placement {
/** @const {!../../../src/service/ampdoc-impl.AmpDoc} */
this.ampdoc = ampdoc;

/** @const @private {!../../../src/service/resources-interface.ResourcesInterface} */
this.resources_ = Services.resourcesForDoc(anchorElement);
/** @const @private {!../../../src/service/mutator-interface.MutatorInterface} */
this.mutator_ = Services.mutatorForDoc(anchorElement);

/** @const @private {!../../../src/service/viewport/viewport-interface.ViewportInterface} */
this.viewport_ = Services.viewportForDoc(anchorElement);
Expand Down Expand Up @@ -253,7 +253,7 @@ export class Placement {
return whenUpgradedToCustomElement(this.getAdElement())
.then(() => this.getAdElement().whenBuilt())
.then(() => {
return this.resources_.attemptChangeSize(
return this.mutator_.attemptChangeSize(
this.getAdElement(),
placement.height,
placement.width,
Expand Down
8 changes: 4 additions & 4 deletions extensions/amp-bind/0.1/bind-impl.js
Expand Up @@ -177,8 +177,8 @@ export class Bind {
*/
this.maxNumberOfBindings_ = 1000;

/** @const @private {!../../../src/service/resources-interface.ResourcesInterface} */
this.resources_ = Services.resourcesForDoc(ampdoc);
/** @const @private {!../../../src/service/mutator-interface.MutatorInterface} */
this.mutator_ = Services.mutatorForDoc(ampdoc);

/**
* The current values of all bound expressions on the page.
Expand Down Expand Up @@ -1233,7 +1233,7 @@ export class Bind {
if (updates.length === 0) {
return Promise.resolve();
}
return this.resources_.mutateElement(element, () => {
return this.mutator_.mutateElement(element, () => {
const mutations = map();
let width, height;

Expand All @@ -1258,7 +1258,7 @@ export class Bind {
// TODO(choumx): Add new Resources method for adding change-size
// request without scheduling vsync pass since `mutateElement()`
// will schedule a pass after a short delay anyways.
this.resources_./*OK*/ changeSize(element, height, width);
this.mutator_./*OK*/ changeSize(element, height, width);
}

if (typeof element.mutatedAttributesCallback === 'function') {
Expand Down
20 changes: 10 additions & 10 deletions extensions/amp-form/0.1/amp-form-textarea.js
Expand Up @@ -176,8 +176,8 @@ export function handleInitialOverflowElements(textareas) {
* @visibleForTesting
*/
export function getHasOverflow(element) {
const resources = Services.resourcesForDoc(element);
return resources.measureElement(() => {
const mutator = Services.mutatorForDoc(element);
return mutator.measureElement(() => {
return element./*OK*/ scrollHeight > element./*OK*/ clientHeight;
});
}
Expand Down Expand Up @@ -206,16 +206,16 @@ function resizeTextareaElements(elements) {
* @param {!Element} element
*/
function handleTextareaDrag(element) {
const resources = Services.resourcesForDoc(element);
const mutator = Services.mutatorForDoc(element);

Promise.all([
resources.measureElement(() => element./*OK*/ scrollHeight),
mutator.measureElement(() => element./*OK*/ scrollHeight),
listenOncePromise(element, 'mouseup'),
]).then(results => {
const heightMouseDown = results[0];
let heightMouseUp = 0;

return resources.measureMutateElement(
return mutator.measureMutateElement(
element,
() => {
heightMouseUp = element./*OK*/ scrollHeight;
Expand Down Expand Up @@ -248,7 +248,7 @@ function maybeRemoveResizeBehavior(element, startHeight, endHeight) {
* @visibleForTesting
*/
export function maybeResizeTextarea(element) {
const resources = Services.resourcesForDoc(element);
const mutator = Services.mutatorForDoc(element);
const win = /** @type {!Window} */ (devAssert(
element.ownerDocument.defaultView
));
Expand All @@ -263,7 +263,7 @@ export function maybeResizeTextarea(element) {
// element's content, or 2. the element itself.
const minScrollHeightPromise = getShrinkHeight(element);

return resources.measureMutateElement(
return mutator.measureMutateElement(
element,
() => {
const computed = computedStyle(win, element);
Expand Down Expand Up @@ -319,7 +319,7 @@ function getShrinkHeight(textarea) {
const doc = /** @type {!Document} */ (devAssert(textarea.ownerDocument));
const win = /** @type {!Window} */ (devAssert(doc.defaultView));
const body = /** @type {!HTMLBodyElement} */ (devAssert(doc.body));
const resources = Services.resourcesForDoc(textarea);
const mutator = Services.mutatorForDoc(textarea);

const clone = textarea.cloneNode(/*deep*/ false);
clone.classList.add(AMP_FORM_TEXTAREA_CLONE_CSS);
Expand All @@ -328,7 +328,7 @@ function getShrinkHeight(textarea) {
let resultingHeight = 0;
let shouldKeepTop = false;

return resources
return mutator
.measureMutateElement(
body,
() => {
Expand All @@ -354,7 +354,7 @@ function getShrinkHeight(textarea) {
}
)
.then(() => {
return resources.measureMutateElement(
return mutator.measureMutateElement(
body,
() => {
resultingHeight = clone./*OK*/ scrollHeight;
Expand Down
8 changes: 4 additions & 4 deletions extensions/amp-form/0.1/amp-form.js
Expand Up @@ -146,8 +146,8 @@ export class AmpForm {
/** @const @private {!../../../src/service/action-impl.ActionService} */
this.actions_ = Services.actionServiceForDoc(this.form_);

/** @const @private {!../../../src/service/resources-interface.ResourcesInterface} */
this.resources_ = Services.resourcesForDoc(this.form_);
/** @const @private {!../../../src/service/mutator-interface.MutatorInterface} */
this.mutator_ = Services.mutatorForDoc(this.form_);

/** @const @private {!../../../src/service/viewer-interface.ViewerInterface} */
this.viewer_ = Services.viewerForDoc(this.form_);
Expand Down Expand Up @@ -1177,7 +1177,7 @@ export class AmpForm {
.then(rendered => {
rendered.id = messageId;
rendered.setAttribute('i-amphtml-rendered', '');
return this.resources_.mutateElement(
return this.mutator_.mutateElement(
dev().assertElement(container),
() => {
container.appendChild(rendered);
Expand All @@ -1196,7 +1196,7 @@ export class AmpForm {
// this container are now visible so they get scheduled for layout.
// This will be unnecessary when the AMP Layers implementation is
// complete.
this.resources_.mutateElement(container, () => {});
this.mutator_.mutateElement(container, () => {});
}
}

Expand Down
8 changes: 4 additions & 4 deletions extensions/amp-form/0.1/form-validators.js
Expand Up @@ -85,8 +85,8 @@ export class FormValidator {
/** @protected @const {!../../../src/service/ampdoc-impl.AmpDoc} */
this.ampdoc = Services.ampdoc(form);

/** @const @protected {!../../../src/service/resources-interface.ResourcesInterface} */
this.resources = Services.resourcesForDoc(form);
/** @const @protected {!../../../src/service/mutator-interface.MutatorInterface} */
this.mutator = Services.mutatorForDoc(form);

/** @protected @const {!Document|!ShadowRoot} */
this.root = this.ampdoc.getRootNode();
Expand Down Expand Up @@ -379,7 +379,7 @@ export class AbstractCustomValidator extends FormValidator {
input.setAttribute('aria-invalid', 'true');
input.setAttribute('aria-describedby', validationId);

this.resources.mutateElement(validation, () =>
this.mutator.mutateElement(validation, () =>
validation.classList.add('visible')
);
}
Expand All @@ -397,7 +397,7 @@ export class AbstractCustomValidator extends FormValidator {
input.removeAttribute('aria-invalid');
input.removeAttribute('aria-describedby');

this.resources.mutateElement(visibleValidation, () =>
this.mutator.mutateElement(visibleValidation, () =>
visibleValidation.classList.remove('visible')
);
}
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-form/0.1/test/test-amp-form-textarea.js
Expand Up @@ -178,7 +178,7 @@ describes.realWin(
return mutator() || Promise.resolve();
},
};
sandbox.stub(Services, 'resourcesForDoc').returns(fakeResources);
sandbox.stub(Services, 'mutatorForDoc').returns(fakeResources);
sandbox
.stub(eventHelper, 'listenOncePromise')
.returns(Promise.resolve());
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-form/0.1/test/test-amp-form.js
Expand Up @@ -76,9 +76,9 @@ describes.repeated(
createTextNode = ownerDoc.createTextNode.bind(ownerDoc);

// Force sync mutateElement to make testing easier.
const resources = Services.resourcesForDoc(env.ampdoc);
const mutator = Services.mutatorForDoc(env.ampdoc);
mutateElementStub = sandbox
.stub(resources, 'mutateElement')
.stub(mutator, 'mutateElement')
.callsArg(1);

// This needs to be stubbed to stop the function from,
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-form/0.1/test/test-form-validators.js
Expand Up @@ -116,8 +116,8 @@ describes.realWin('form-validators', {amp: true}, env => {
sandbox = env.sandbox;

// Force sync mutateElement to make testing easier.
const resources = Services.resourcesForDoc(env.ampdoc);
sandbox.stub(resources, 'mutateElement').callsArg(1);
const mutator = Services.mutatorForDoc(env.ampdoc);
sandbox.stub(mutator, 'mutateElement').callsArg(1);
});

describe('getFormValidator', () => {
Expand Down
Expand Up @@ -55,7 +55,7 @@ function flyIn(fxElement, axis, coeff) {

// only do this on the first element
if (!fxElement.initialTrigger) {
Services.resourcesForDoc(element).mutateElement(element, () => {
Services.mutatorForDoc(element).mutateElement(element, () => {
const style = computedStyle(fxElement.win, element);
const prop = axisIsX ? 'left' : 'top';
const propAsLength = style[prop] === 'auto' ? '0px' : style[prop];
Expand Down
10 changes: 5 additions & 5 deletions extensions/amp-fx-collection/0.1/providers/fx-provider.js
Expand Up @@ -113,7 +113,7 @@ function scrollToggle(element, isShown, position) {
scrollToggleFloatIn(element, offset);
};

Services.resourcesForDoc(element).measureMutateElement(
Services.mutatorForDoc(element).measureMutateElement(
element,
measure,
mutate
Expand Down Expand Up @@ -150,8 +150,8 @@ export class FxElement {
/** @private @const {!../../../../src/service/viewport/viewport-interface.ViewportInterface} */
this.viewport_ = Services.viewportForDoc(element);

/** @const @private {!../../../../src/service/resources-interface.ResourcesInterface} */
this.resources_ = Services.resourcesForDoc(element);
/** @const @private {!../../../../src/service/mutator-interface.MutatorInterface} */
this.mutator_ = Services.mutatorForDoc(element);

/** @type {?number} */
this.viewportHeight = null;
Expand Down Expand Up @@ -245,7 +245,7 @@ export class FxElement {

/** @private */
updateViewportHeight_() {
this.resources_.measureElement(() => {
this.mutator_.measureElement(() => {
this.viewportHeight = this.viewport_.getHeight();
});
}
Expand All @@ -261,7 +261,7 @@ export class FxElement {
* @private
*/
getAdjustedViewportHeight_() {
return this.resources_.measureElement(() => {
return this.mutator_.measureElement(() => {
const viewportHeight = this.viewport_.getHeight();

let offsetTop = 0;
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-lightbox/0.1/amp-lightbox.js
Expand Up @@ -713,7 +713,7 @@ function setTransparentBody(win, body) {
const state = {};
const ampdoc = Services.ampdocServiceFor(win).getAmpDoc(body);

Services.resourcesForDoc(ampdoc).measureMutateElement(
Services.mutatorForDoc(ampdoc).measureMutateElement(
body,
function measure() {
state.alreadyTransparent =
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-mathml/0.1/amp-mathml.js
Expand Up @@ -73,7 +73,7 @@ export class AmpMathml extends AMP.BaseElement {
data['width'] = undefined;
}
this.element
.getResources()
.getMutator()
./*OK*/ changeSize(this.element, data['height'], data['width']);
},
/* opt_is3P */ true
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-next-page/0.1/test/test-amp-next-page.js
Expand Up @@ -65,7 +65,7 @@ describes.realWin(

fetchDocumentMock = sandbox.mock(DocFetcher);
sandbox
.stub(Services.resourcesForDoc(ampdoc), 'mutateElement')
.stub(Services.mutatorForDoc(ampdoc), 'mutateElement')
.callsFake((unused, mutator) => {
mutator();
return Promise.resolve();
Expand Down

0 comments on commit 0d66374

Please sign in to comment.