Various micro optimizations of the component boot flow. (ampproject#2…

…3767) - Removes unnecessary `hasAttributes` call from `propagateAttributes` - Bundles several `classList.add` operations into one. - Add special case for AMP Doc retrieval that is faster if no shadow doc was ever added which is common. - Makes hot `getTopWindow` function faster by ensuring the property that is rarely defined is at least known.
edelight · Aug 7, 2019 · 0ade95b · 0ade95b
1 parent 7309134
commit 0ade95b
Show file tree

Hide file tree

Showing 10 changed files with 632 additions and 16 deletions.
diff --git a/build-system/build.conf.js b/build-system/build.conf.js
@@ -32,7 +32,7 @@ const defaultPlugins = [
 
 const esmRemovedImports = {
  './polyfills/document-contains': ['installDocContains'],
- './polyfills/domtokenlist-toggle': ['installDOMTokenListToggle'],
+ './polyfills/domtokenlist': ['installDOMTokenList'],
  './polyfills/fetch': ['installFetch'],
  './polyfills/math-sign': ['installMathSign'],
  './polyfills/object-assign': ['installObjectAssign'],

diff --git a/build-system/dep-check-config.js b/build-system/dep-check-config.js
@@ -684,7 +684,7 @@ exports.rules = [
  '3p/polyfills.js->src/polyfills/math-sign.js',
  '3p/polyfills.js->src/polyfills/object-assign.js',
  '3p/polyfills.js->src/polyfills/object-values.js',
- 'src/polyfills.js->src/polyfills/domtokenlist-toggle.js',
+ 'src/polyfills.js->src/polyfills/domtokenlist.js',
  'src/polyfills.js->src/polyfills/document-contains.js',
  'src/polyfills.js->src/polyfills/fetch.js',
  'src/polyfills.js->src/polyfills/math-sign.js',
@@ -695,7 +695,7 @@ exports.rules = [
  'src/polyfills.js->src/polyfills/custom-elements.js',
  'src/friendly-iframe-embed.js->src/polyfills/custom-elements.js',
  'src/friendly-iframe-embed.js->src/polyfills/document-contains.js',
- 'src/friendly-iframe-embed.js->src/polyfills/domtokenlist-toggle.js',
+ 'src/friendly-iframe-embed.js->src/polyfills/domtokenlist.js',
  ],
  },
  {

diff --git a/examples/lots-of-images.amp.html b/examples/lots-of-images.amp.html
diff --git a/src/base-element.js b/src/base-element.js
@@ -678,8 +678,9 @@ export class BaseElement {
  attributes = isArray(attributes) ? attributes : [attributes];
  for (let i = 0; i < attributes.length; i++) {
  const attr = attributes[i];
- if (this.element.hasAttribute(attr)) {
- element.setAttribute(attr, this.element.getAttribute(attr));
+ const val = this.element.getAttribute(attr);
+ if (null !== val) {
+ element.setAttribute(attr, val);
  } else if (opt_removeMissingAttrs) {
  element.removeAttribute(attr);
  }

diff --git a/src/friendly-iframe-embed.js b/src/friendly-iframe-embed.js
@@ -40,7 +40,7 @@ import {getExperimentBranch, isExperimentOn} from './experiments';
 import {getMode} from './mode';
 import {installAmpdocServices} from './service/core-services';
 import {install as installCustomElements} from './polyfills/custom-elements';
-import {install as installDOMTokenListToggle} from './polyfills/domtokenlist-toggle';
+import {install as installDOMTokenList} from './polyfills/domtokenlist';
 import {install as installDocContains} from './polyfills/document-contains';
 import {installCustomElements as installRegisterElement} from 'document-register-element/build/document-register-element.patched';
 import {installStylesForDoc, installStylesLegacy} from './style-installer';
@@ -869,7 +869,7 @@ export function whenContentIniLoad(elementOrAmpDoc, hostWin, rect) {
  */
 function installPolyfillsInChildWindow(parentWin, childWin) {
  installDocContains(childWin);
- installDOMTokenListToggle(childWin);
+ installDOMTokenList(childWin);
  // TODO(jridgewell): Ship custom-elements-v1. For now, we use this hack so it
  // is DCE'd from production builds. Note: When the hack is removed, remove the
  // @suppress {suspiciousCode} annotation at the top of this function.

diff --git a/src/polyfills.js b/src/polyfills.js
@@ -19,7 +19,7 @@
 import {getMode} from './mode';
 import {install as installArrayIncludes} from './polyfills/array-includes';
 import {install as installCustomElements} from './polyfills/custom-elements';
-import {install as installDOMTokenListToggle} from './polyfills/domtokenlist-toggle';
+import {install as installDOMTokenList} from './polyfills/domtokenlist';
 import {install as installDocContains} from './polyfills/document-contains';
 import {install as installFetch} from './polyfills/fetch';
 import {install as installGetBoundingClientRect} from './get-bounding-client-rect';
@@ -39,7 +39,7 @@ installArrayIncludes(self);
 
 // Polyfills that depend on DOM availability
 if (self.document) {
- installDOMTokenListToggle(self);
+ installDOMTokenList(self);
  installDocContains(self);
  installGetBoundingClientRect(self);
 

diff --git a/src/polyfills/domtokenlist-toggle.js → src/polyfills/domtokenlist.js b/src/polyfills/domtokenlist-toggle.js → src/polyfills/domtokenlist.js
@@ -35,7 +35,8 @@ function domTokenListTogglePolyfill(token, opt_force) {
 }
 
 /**
- * Polyfills `DOMTokenList.prototype.toggle` API in IE.
+ * Polyfills `DOMTokenList.prototype.toggle` API and makes `.add` accepts N
+ * classes in IE.
  * @param {!Window} win
  */
 export function install(win) {
@@ -46,6 +47,13 @@ export function install(win) {
  writable: true,
  value: domTokenListTogglePolyfill,
  });
+
+ const {add} = win.DOMTokenList.prototype;
+ win.DOMTokenList.prototype.add = function() {
+ for (let i = 0; i < arguments.length; i++) {
+ add.call(this, arguments[i]);
+ }
+ };
  }
 }
 

diff --git a/src/service.js b/src/service.js
@@ -318,7 +318,7 @@ export function getParentWindow(win) {
  * @return {!Window}
  */
 export function getTopWindow(win) {
- return win.__AMP_TOP || win;
+ return win.__AMP_TOP || (win.__AMP_TOP = win);
 }
 
 /**

diff --git a/src/service/ampdoc-impl.js b/src/service/ampdoc-impl.js
@@ -61,6 +61,9 @@ export class AmpDocService {
 
  /** @private {boolean} */
  this.ampdocFieExperimentOn_ = isExperimentOn(win, 'ampdoc-fie');
+
+ /** @private {boolean} */
+ this.mightHaveShadowRoots_ = !isSingleDoc;
  }
 
  /**
@@ -149,6 +152,10 @@ export class AmpDocService {
  continue;
  }
 
+ if (!this.mightHaveShadowRoots_) {
+ break;
+ }
+
  // Shadow doc.
  const shadowRoot =
  n.nodeType == /* DOCUMENT */ 9 ? n : getShadowRootNode(n);
@@ -203,6 +210,7 @@ export class AmpDocService {
  * @restricted
  */
  installShadowDoc(url, shadowRoot) {
+ this.mightHaveShadowRoots_ = true;
  devAssert(
  !shadowRoot[AMPDOC_PROP],
  'The shadow root already contains ampdoc'
@@ -213,7 +221,7 @@ export class AmpDocService {
  }
 
  /**
- * Creates and installs the ampdoc for the shadow root.
+ * Creates and installs the ampdoc for the fie root.
  * @param {string} url
  * @param {!Window} childWin
  * @param {!AmpDocOptions=} opt_options

diff --git a/test/unit/test-polyfill-domtokenlist-toggle.js b/test/unit/test-polyfill-domtokenlist-toggle.js
@@ -14,11 +14,11 @@
  * limitations under the License.
  */
 
-import {install} from '../../src/polyfills/domtokenlist-toggle';
+import {install} from '../../src/polyfills/domtokenlist';
 import {toArray} from '../../src/types';
 
 describes.fakeWin(
- 'DOMTokenList.toggle on non-IE',
+ 'DOMTokenList.toggle/add on non-IE',
  {
  win: {
  navigator: {
@@ -29,10 +29,12 @@ describes.fakeWin(
  env => {
  let sandbox;
  let originalToggle;
+ let originalAdd;
  let element;
 
  beforeEach(() => {
  originalToggle = env.win.DOMTokenList.prototype.toggle;
+ originalAdd = env.win.DOMTokenList.prototype.add;
  sandbox = sinon.sandbox;
 
  element = env.win.document.createElement('div');
@@ -41,23 +43,31 @@ describes.fakeWin(
 
  afterEach(() => {
  env.win.DOMTokenList.prototype.toggle = originalToggle;
+ env.win.DOMTokenList.prototype.add = originalAdd;
  if (element.parentNode) {
  element.parentNode.removeChild(element);
  }
  sandbox.restore();
  });
 
- it('should NOT override in non-IE browsers', () => {
+ it('should NOT override toggle in non-IE browsers', () => {
  env.win.DOMTokenList = window.DOMTokenList;
  install(env.win);
  const newToggle = env.win.DOMTokenList.prototype.toggle;
  expect(newToggle).to.equal(originalToggle);
  });
+
+ it('should NOT override add in non-IE browsers', () => {
+ env.win.DOMTokenList = window.DOMTokenList;
+ install(env.win);
+ const newAdd = env.win.DOMTokenList.prototype.add;
+ expect(newAdd).to.equal(originalAdd);
+ });
  }
 );
 
 describes.fakeWin(
- 'DOMTokenList.toggle On IE',
+ 'DOMTokenList.toggle/add On IE',
  {
  win: {
  navigator: {
@@ -68,10 +78,12 @@ describes.fakeWin(
  env => {
  let sandbox;
  let originalToggle;
+ let originalAdd;
  let element;
 
  beforeEach(() => {
  originalToggle = env.win.DOMTokenList.prototype.toggle;
+ originalAdd = env.win.DOMTokenList.prototype.add;
  sandbox = sinon.sandbox;
 
  element = env.win.document.createElement('div');
@@ -80,6 +92,7 @@ describes.fakeWin(
 
  afterEach(() => {
  env.win.DOMTokenList.prototype.toggle = originalToggle;
+ env.win.DOMTokenList.prototype.add = originalAdd;
  if (element.parentNode) {
  element.parentNode.removeChild(element);
  }
@@ -115,5 +128,22 @@ describes.fakeWin(
  .false;
  expect(toArray(element.classList)).to.not.contain('first');
  });
+
+ it('should polyfill DOMTokenList.add API', () => {
+ env.win.DOMTokenList = window.DOMTokenList;
+ install(env.win);
+ const polyfillAdd = env.win.DOMTokenList.prototype.add;
+
+ expect(polyfillAdd).to.be.ok;
+ expect(polyfillAdd).to.not.equal(originalToggle);
+
+ element.classList.add('foo');
+ expect(toArray(element.classList)).to.contain('foo');
+ element.classList.add('one', 'two', 'three');
+ expect(toArray(element.classList)).to.contain('foo');
+ expect(toArray(element.classList)).to.contain('one');
+ expect(toArray(element.classList)).to.contain('two');
+ expect(toArray(element.classList)).to.contain('three');
+ });
  }
 );