Delay unhiding body until Dynamic CSS is loaded

extensions/amp-access/0.1/test/test-amp-access.js

@@ -17,7 +17,7 @@
 import {AccessService} from '../../../../build/all/v0/amp-access-0.1.max';
 import {Observable} from '../../../../src/observable';
 import {installCidService} from '../../../../src/service/cid-impl';
-import {markElementScheduledForTesting} from '../../../../src/service';
+import {markElementScheduledForTesting} from '../../../../src/custom-element';
 import * as sinon from 'sinon';
 
 

extensions/amp-analytics/0.1/test/test-amp-analytics.js

@@ -17,7 +17,7 @@
 import {AmpAnalytics} from '../../../../build/all/v0/amp-analytics-0.1.max';
 import {adopt} from '../../../../src/runtime';
 import {getService} from '../../../../src/service';
-import {markElementScheduledForTesting} from '../../../../src/service';
+import {markElementScheduledForTesting} from '../../../../src/custom-element';
 import {installCidService} from '../../../../src/service/cid-impl';
 import {installViewerService} from '../../../../src/service/viewer-impl';
 import * as sinon from 'sinon';

extensions/amp-dynamic-css-classes/0.1/amp-dynamic-css-classes.js

@@ -18,6 +18,8 @@ import {parseUrl} from '../../../src/url';
 import {viewerFor} from '../../../src/viewer';
 import {log} from '../../../src/log';
 import {isExperimentOn} from '../../../src/experiments';
+import {getService} from '../../../src/service';
+import {vsyncFor} from '../../../src/vsync';
 
 /** @const */
 const TAG = 'AmpDynamicCssClasses';
@@ -116,7 +118,10 @@ function addReferrerClasses(win) {
   const classes = referrers.map(referrer => {
     return `amp-referrer-${referrer.replace(/\./g, '-')}`;
   });
-  addDynamicCssClasses(win, classes);
+
+  vsyncFor(win).mutate(() => {
+    addDynamicCssClasses(win, classes);
+  });
 }
 
 
@@ -127,7 +132,9 @@ function addReferrerClasses(win) {
 function addViewerClass(win) {
   const viewer = viewerFor(win);
   if (viewer.isEmbedded()) {
-    addDynamicCssClasses(win, ['amp-viewer']);
+    vsyncFor(win).mutate(() => {
+      addDynamicCssClasses(win, ['amp-viewer']);
+    });
   }
 }
 
@@ -144,4 +151,15 @@ function addRuntimeClasses(win) {
   }
 }
 
-addRuntimeClasses(AMP.win);
+/**
+ * @param {!Window} win
+ * @return {!Object} All services need to return an object to "load".
+ */
+function installDynamicClassesService(win) {
+  return getService(win, 'amp-dynamic-css-classes', () => {
+    addRuntimeClasses(win);
+    return {};
+  });
+};
+
+installDynamicClassesService(AMP.win);

extensions/amp-dynamic-css-classes/0.1/test/test-runtime-classes.js

@@ -16,6 +16,7 @@
 
 import {createServedIframe} from '../../../../testing/iframe';
 import {toggleExperiment} from '../../../../src/experiments';
+import {vsyncFor} from '../../../../src/vsync';
 
 function overwrite(object, property, value) {
   Object.defineProperty(object, property, {
@@ -36,12 +37,20 @@ const PinterestUA = 'Mozilla/5.0 (Linux; Android 5.1.1; SM-G920F' +
 describe('dynamic classes are inserted at runtime', () => {
   let documentElement;
 
+  function mockVsync(win) {
+    const vsync = vsyncFor(win);
+    vsync.schedule_ = () => {
+      vsync.runScheduledTasks_();
+    };
+  }
+
   function setup(enabled, userAgent, referrer) {
     return createServedIframe(iframeSrc).then(fixture => {
       const win = fixture.win;
       documentElement = fixture.doc.documentElement;
 
       toggleExperiment(win, 'dynamic-css-classes', enabled);
+      mockVsync(win);
 
       if (userAgent !== undefined) {
         overwrite(win.navigator, 'userAgent', userAgent);
@@ -98,4 +107,16 @@ describe('dynamic classes are inserted at runtime', () => {
       });
     });
   });
+
+  it('should delay unhiding the body', () => {
+    return createServedIframe(iframeSrc).then(fixture => {
+      expect(fixture.doc.body).to.be.hidden;
+
+      const win = fixture.win;
+      mockVsync(win);
+      return win.insertDynamicCssScript().then(() => fixture);
+    }).then(fixture => {
+      expect(fixture.doc.body).to.be.visible;
+    });
+  });
 });

src/amp.js

@@ -30,6 +30,7 @@ import {stubElements} from './custom-element';
 import {adopt} from './runtime';
 import {cssText} from '../build/css';
 import {maybeValidate} from './validator-integration';
+import {waitForExtensions} from './render-delaying-extensions';
 
 // We must under all circumstances call makeBodyVisible.
 // It is much better to have AMP tags not rendered than having
@@ -57,8 +58,10 @@ try {
       installGlobalClickListener(window);
 
       maybeValidate(window);
-    } finally {
+      makeBodyVisible(document, waitForExtensions(window));
+    } catch (e) {
       makeBodyVisible(document);
+    } finally {
       perf.tick('e_is');
       // TODO(erwinm): move invocation of the `flush` method when we have the
       // new ticks in place to batch the ticks properly.

src/cid.js

@@ -18,7 +18,7 @@
  * @fileoverview Factory for ./service/cid-impl.js
  */
 
-import {getElementService} from './service';
+import {getElementService} from './custom-element';
 
 /**
  * @param {!Window} window

src/custom-element.js

@@ -27,6 +27,7 @@ import {reportError} from './error';
 import {resourcesFor} from './resources';
 import {timer} from './timer';
 import {vsyncFor} from './vsync';
+import {getServicePromise} from './service';
 import * as dom from './dom';
 
 
@@ -1061,3 +1062,63 @@ export function registerElement(win, name, implementationClass) {
     prototype: createAmpElementProto(win, name, implementationClass)
   });
 }
+
+/**
+ * @param {!Window} win
+ * @param {string} elementName Name of an extended custom element.
+ * @return {boolean} Whether this element is scheduled to be loaded.
+ */
+function isElementScheduled(win, elementName) {
+  assert(win.ampExtendedElements, 'win.ampExtendedElements not created yet');
+  return !!win.ampExtendedElements[elementName];
+}
+
+/**
+ * In order to provide better error messages we only allow to retrieve
+ * services from other elements if those elements are loaded in the page.
+ * This makes it possible to mark an element as loaded in a test.
+ * @param {!Window} win
+ * @param {string} elementName Name of an extended custom element.
+ */
+export function markElementScheduledForTesting(win, elementName) {
+  if (!win.ampExtendedElements) {
+    win.ampExtendedElements = {};
+  }
+  win.ampExtendedElements[elementName] = true;
+}
+
+/**
+ * Resets our scheduled elements.
+ * @param {!Window} win
+ * @param {string} elementName Name of an extended custom element.
+ */
+export function resetScheduledElementForTesting(win, elementName) {
+  if (win.ampExtendedElements) {
+    win.ampExtendedElements[elementName] = null;
+  }
+}
+
+
+/**
+ * Returns a promise for a service for the given id and window. Also expects
+ * an element that has the actual implementation. The promise resolves when
+ * the implementation loaded.
+ * Users should typically wrap this as a special purpose function (e.g.
+ * viewportFor(win)) for type safety and because the factory should not be
+ * passed around.
+ * @param {!Window} win
+ * @param {string} id of the service.
+ * @param {string} provideByElement Name of the custom element that provides
+ *     the implementation of this service.
+ * @return {!Promise<*>}
+ */
+export function getElementService(win, id, providedByElement) {
+  return Promise.resolve().then(() => {
+    assert(isElementScheduled(win, providedByElement),
+        'Service %s was requested to be provided through %s, ' +
+        'but %s is not loaded in the current page. To fix this ' +
+        'problem load the JavaScript file for %s in this page.',
+        id, providedByElement, providedByElement, providedByElement);
+    return getServicePromise(win, id);
+  });
+}

src/promise.js

@@ -29,7 +29,7 @@ export function all(promises) {
     return Promise.resolve([]);
   }
 
-  const results = [];
+  const results = Array(left);
   return new Promise((resolve, reject) => {
     promises.forEach((promise, index) => {
       promise.then(result => {
@@ -38,9 +38,7 @@ export function all(promises) {
         if (left == 0) {
           resolve(results);
         }
-      }, error => {
-        reject(error);
-      });
+      }, reject);
     });
   });
 }

src/render-delaying-extensions.js

@@ -0,0 +1,66 @@
+/**
+ * Copyright 2016 The AMP HTML Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS-IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {getServicePromise} from './service';
+import {all} from './promise';
+import {timer} from './timer';
+
+/**
+ * List of extensions that, if they're included on the page, must be loaded
+ * before the page should be shown to users.
+ * Do not add an extension unless absolutely necessary.
+ * @const {!Array<string>}
+ */
+const EXTENSIONS = [
+  'amp-dynamic-css-classes'
+];
+
+/**
+ * Maximum milliseconds to wait for all extensions to load before erroring.
+ * @const
+ */
+const LOAD_TIMEOUT = 3000;
+
+
+/**
+ * Detects any extensions that are were included on the page that need to
+ * delay unhiding the body (to avoid Flash of Unstyled Content), and returns
+ * a promise that will resolve when they have loaded or reject after a timeout.
+ * @param {!Window} win
+ * @return {?Promise}
+ */
+export function waitForExtensions(win) {
+  const extensions = includedExtensions(win);
+
+  if (extensions.length) {
+    return timer.timeoutPromise(LOAD_TIMEOUT, all(extensions));
+  }
+}
+
+/**
+ * Detects which, if any, render-delaying extensions are included on the page.
+ * @param {!Window} win
+ * @return {!Array<string>}
+ */
+export function includedExtensions(win) {
+  const document = win.document;
+
+  return EXTENSIONS.filter(extension => {
+    return document.querySelector(`[custom-element="${extension}"]`);
+  }).map(extension => {
+    return getServicePromise(win, extension);
+  });
+}

src/service.js

@@ -47,8 +47,13 @@ export function getService(win, id, opt_factory) {
   const services = getServices(win);
   let s = services[id];
   if (!s) {
-    s = services[id] = {};
+    s = services[id] = {
+      obj: null,
+      promise: null,
+      resolve: null,
+    };
   }
+
   if (!s.obj) {
     assert(opt_factory, 'Factory not given and service missing %s', id);
     s.obj = opt_factory(win);
@@ -68,34 +73,12 @@ export function getService(win, id, opt_factory) {
  * Users should typically wrap this as a special purpose function (e.g.
  * viewportFor(win)) for type safety and because the factory should not be
  * passed around.
+ * TODO
  * @param {!Window} win
  * @param {string} id of the service.
- * @param {string} provideByElement Name of the custom element that provides
- *     the implementation of this service.
  * @return {!Promise<*>}
  */
-export function getElementService(win, id, providedByElement) {
-  // Call `getElementService_` in a micro-task to ensure that `stubElements`
-  // has been called.
-  return Promise.resolve().then(() => {
-    return getElementService_(win, id, providedByElement);
-  });
-}
-
-/**
- * @param {!Window} win
- * @param {string} id of the service.
- * @param {string} provideByElement Name of the custom element that provides
- *     the implementation of this service.
- * @return {!Promise<*>}
- * @private
- */
-function getElementService_(win, id, providedByElement) {
-  assert(isElementScheduled(win, providedByElement),
-      'Service %s was requested to be provided through %s, ' +
-      'but %s is not loaded in the current page. To fix this ' +
-      'problem load the JavaScript file for %s in this page.',
-      id, providedByElement, providedByElement, providedByElement);
+export function getServicePromise(win, id) {
   const services = getServices(win);
   const s = services[id];
   if (s) {
@@ -106,6 +89,7 @@ function getElementService_(win, id, providedByElement) {
     }
     return s.promise;
   }
+
   // TODO(@cramforce): Add a check that if the element is eventually registered
   // that the service is actually provided and this promise resolves.
   let resolve;
@@ -121,30 +105,6 @@ function getElementService_(win, id, providedByElement) {
   return p;
 }
 
-/**
- * @param {!Window} win
- * @param {string} elementName Name of an extended custom element.
- * @return {boolean} Whether this element is scheduled to be loaded.
- */
-function isElementScheduled(win, elementName) {
-  assert(win.ampExtendedElements, 'win.ampExtendedElements not created yet');
-  return !!win.ampExtendedElements[elementName];
-}
-
-/**
- * In order to provide better error messages we only allow to retrieve
- * services from other elements if those elements are loaded in the page.
- * This makes it possible to mark an element as loaded in a test.
- * @param {!Window} win
- * @param {string} elementName Name of an extended custom element.
- */
-export function markElementScheduledForTesting(win, elementName) {
-  if (!win.ampExtendedElements) {
-    win.ampExtendedElements = {};
-  }
-  win.ampExtendedElements[elementName] = true;
-}
-
 /**
  * Returns the object that holds the services registered in a window.
  * @param {!Window} win