Revert 'Move mutator implementations out to a standalone service' (am…

…pproject#26479)
gmajoulet · Jan 25, 2020 · d8da5a5 · d8da5a5
1 parent 0ce5f2f
commit d8da5a5
Show file tree

Hide file tree

Showing 13 changed files with 2,190 additions and 2,293 deletions.
diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js
@@ -224,14 +224,6 @@ const forbiddenTerms = {
       'testing/iframe.js',
     ],
   },
-  'installMutatorServiceForDoc': {
-    message: privateServiceFactory,
-    whitelist: [
-      'src/inabox/inabox-services.js',
-      'src/service/core-services.js',
-      'src/service/mutator-impl.js',
-    ],
-  },
   'installPerformanceService': {
     message: privateServiceFactory,
     whitelist: [
@@ -241,14 +233,6 @@ 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: [
@@ -300,6 +284,15 @@ 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: [
@@ -579,7 +572,7 @@ const forbiddenTerms = {
   },
   '\\.schedulePass\\(': {
     message: 'schedulePass is heavy, think twice before using it',
-    whitelist: ['src/service/mutator-impl.js', 'src/service/resources-impl.js'],
+    whitelist: ['src/service/resources-impl.js'],
   },
   '\\.requireLayout\\(': {
     message:
@@ -856,7 +849,6 @@ const forbiddenTerms = {
       'test/unit/test-mode.js',
       'test/unit/test-motion.js',
       'test/unit/test-mustache.js',
-      'test/unit/test-mutator.js',
       'test/unit/test-object.js',
       'test/unit/test-observable.js',
       'test/unit/test-pass.js',

diff --git a/extensions/amp-access-poool/0.1/poool-impl.js b/extensions/amp-access-poool/0.1/poool-impl.js
@@ -64,8 +64,8 @@ export class PooolVendor {
     /** @private {!../../amp-access/0.1/amp-access-source.AccessSource} */
     this.accessSource_ = accessSource;
 
-    /** @const @private {!../../../src/service/mutator-interface.MutatorInterface} */
-    this.mutator_ = Services.mutatorForDoc(this.ampdoc);
+    /** @const @private {!../../../src/service/resources-interface.ResourcesInterface} */
+    this.resources_ = Services.resourcesForDoc(this.ampdoc);
 
     /** @private {string} */
     this.accessUrl_ = ACCESS_CONFIG['authorization'];
@@ -216,7 +216,7 @@ export class PooolVendor {
       .querySelector('[poool-access-preview]');
 
     if (articlePreview) {
-      this.mutator_.mutateElement(articlePreview, () => {
+      this.resources_.mutateElement(articlePreview, () => {
         articlePreview.setAttribute('amp-access-hide', '');
       });
     }
@@ -226,7 +226,7 @@ export class PooolVendor {
       .querySelector('[poool-access-content]');
 
     if (articleContent) {
-      this.mutator_.mutateElement(articleContent, () => {
+      this.resources_.mutateElement(articleContent, () => {
         articleContent.removeAttribute('amp-access-hide');
       });
     }

diff --git a/extensions/amp-story-auto-ads/0.1/story-ad-button-text-fitter.js b/extensions/amp-story-auto-ads/0.1/story-ad-button-text-fitter.js
@@ -31,16 +31,16 @@ export class ButtonTextFitter {
    * @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
    */
   constructor(ampdoc) {
-    /** @const @private {!../../../src/service/mutator-interface.MutatorInterface} */
-    this.mutator_ = Services.mutatorForDoc(ampdoc);
+    /** @private @const {!../../../src/service/resources-interface.ResourcesInterface} */
+    this.resources_ = Services.resourcesForDoc(ampdoc);
 
     /** @private {!Document} */
     this.doc_ = ampdoc.win.document;
 
     /** @private {!Element} */
     this.measurer_ = this.doc_.createElement('div');
 
-    this.mutator_.mutateElement(this.measurer_, () => {
+    this.resources_.mutateElement(this.measurer_, () => {
       this.doc_.body.appendChild(this.measurer_);
       setStyles(this.measurer_, {
         position: 'absolute',
@@ -62,7 +62,7 @@ export class ButtonTextFitter {
    */
   fit(pageElement, container, content) {
     let success = false;
-    return this.mutator_
+    return this.resources_
       .mutateElement(container, () => {
         this.measurer_.textContent = content;
         const fontSize = calculateFontSize(

diff --git a/src/inabox/inabox-mutator.js b/src/inabox/inabox-mutator.js
@@ -15,18 +15,18 @@
  */
 
 import {Services} from '../services';
-import {registerServiceBuilderForDoc} from '../service';
 
 /**
  * @implements {../service/mutator-interface.MutatorInterface}
  */
 export class InaboxMutator {
   /**
    * @param {!../service/ampdoc-impl.AmpDoc} ampdoc
+   * @param {!../service/resources-interface.ResourcesInterface} resources
    */
-  constructor(ampdoc) {
+  constructor(ampdoc, resources) {
     /** @const @private {!../service/resources-interface.ResourcesInterface} */
-    this.resources_ = Services.resourcesForDoc(ampdoc);
+    this.resources_ = resources;
 
     /** @private @const {!../service/vsync-impl.Vsync} */
     this.vsync_ = Services./*OK*/ vsyncFor(ampdoc.win);
@@ -101,10 +101,3 @@ export class InaboxMutator {
     });
   }
 }
-
-/**
- * @param {!../service/ampdoc-impl.AmpDoc} ampdoc
- */
-export function installInaboxMutatorServiceForDoc(ampdoc) {
-  registerServiceBuilderForDoc(ampdoc, 'mutator', InaboxMutator);
-}
diff --git a/src/inabox/inabox-resources.js b/src/inabox/inabox-resources.js
@@ -15,6 +15,7 @@
  */
 
 import {Deferred} from '../utils/promise';
+import {InaboxMutator} from './inabox-mutator';
 import {Observable} from '../observable';
 import {Pass} from '../pass';
 import {READY_SCAN_SIGNAL} from '../service/resources-interface';
@@ -56,6 +57,9 @@ export class InaboxResources {
     /** @const @private {!Deferred} */
     this.firstPassDone_ = new Deferred();
 
+    /** @const @private {!InaboxMutator} */
+    this.mutator_ = new InaboxMutator(ampdoc, this);
+
     const input = Services.inputFor(this.win);
     input.setupInputModeClasses(ampdoc);
   }
@@ -125,12 +129,6 @@ export class InaboxResources {
     return this.pass_.schedule(opt_delay);
   }
 
-  /** @override */
-  updateOrEnqueueMutateTask(unusedResource, unusedNewRequest) {}
-
-  /** @override */
-  schedulePassVsync() {}
-
   /** @override */
   onNextPass(callback) {
     this.passObservable_.add(callback);
@@ -145,10 +143,55 @@ export class InaboxResources {
   }
 
   /** @override */
-  setRelayoutTop(unusedRelayoutTop) {}
+  changeSize(element, newHeight, newWidth, opt_callback, opt_newMargins) {
+    this.mutator_./*OK*/ changeSize(
+      element,
+      newHeight,
+      newWidth,
+      opt_callback,
+      opt_newMargins
+    );
+  }
 
   /** @override */
-  maybeHeightChanged() {}
+  attemptChangeSize(element, newHeight, newWidth, opt_newMargins) {
+    return this.mutator_.attemptChangeSize(
+      element,
+      newHeight,
+      newWidth,
+      opt_newMargins
+    );
+  }
+
+  /** @override */
+  expandElement(element) {
+    this.mutator_.expandElement(element);
+  }
+
+  /** @override */
+  attemptCollapse(element) {
+    return this.mutator_.attemptCollapse(element);
+  }
+
+  /** @override */
+  collapseElement(element) {
+    this.mutator_.collapseElement(element);
+  }
+
+  /** @override */
+  measureElement(measurer) {
+    return this.mutator_.measureElement(measurer);
+  }
+
+  /** @override */
+  mutateElement(element, mutator) {
+    return this.mutator_.mutateElement(element, mutator);
+  }
+
+  /** @override */
+  measureMutateElement(element, measurer, mutator) {
+    return this.mutator_.measureMutateElement(element, measurer, mutator);
+  }
 
   /**
    * @return {!Promise} when first pass executed.

diff --git a/src/inabox/inabox-services.js b/src/inabox/inabox-services.js
@@ -22,7 +22,6 @@ import {installHiddenObserverForDoc} from '../service/hidden-observer-impl';
 import {installHistoryServiceForDoc} from '../service/history-impl';
 import {installIframeMessagingClient} from './inabox-iframe-messaging-client';
 import {installInaboxCidService} from './inabox-cid';
-import {installInaboxMutatorServiceForDoc} from './inabox-mutator';
 import {installInaboxResourcesServiceForDoc} from './inabox-resources';
 import {installInaboxViewerServiceForDoc} from './inabox-viewer';
 import {installInaboxViewportService} from './inabox-viewport';
@@ -48,7 +47,6 @@ export function installAmpdocServicesForInabox(ampdoc) {
   installHistoryServiceForDoc(ampdoc);
   installInaboxResourcesServiceForDoc(ampdoc);
   installOwnersServiceForDoc(ampdoc);
-  installInaboxMutatorServiceForDoc(ampdoc);
   installUrlReplacementsServiceForDoc(ampdoc);
   installActionServiceForDoc(ampdoc);
   installStandardActionsForDoc(ampdoc);

diff --git a/src/service/core-services.js b/src/service/core-services.js
@@ -27,7 +27,6 @@ import {installHistoryServiceForDoc} from './history-impl';
 import {installImg} from '../../builtins/amp-img';
 import {installInputService} from '../input';
 import {installLayout} from '../../builtins/amp-layout';
-import {installMutatorServiceForDoc} from './mutator-impl';
 import {installOwnersServiceForDoc} from './owners-impl';
 import {installPixel} from '../../builtins/amp-pixel';
 import {installPlatformService} from './platform-impl';
@@ -107,9 +106,6 @@ export function installAmpdocServices(ampdoc) {
   isEmbedded
     ? adoptServiceForEmbedDoc(ampdoc, 'owners')
     : installOwnersServiceForDoc(ampdoc);
-  isEmbedded
-    ? adoptServiceForEmbedDoc(ampdoc, 'mutator')
-    : installMutatorServiceForDoc(ampdoc);
   isEmbedded
     ? adoptServiceForEmbedDoc(ampdoc, 'url-replace')
     : installUrlReplacementsServiceForDoc(ampdoc);