From: Timo Tijhof <krinklemail@gmail.com>
Date: Sun, 26 Aug 2018 03:09:02 +0000 (+0100)
Subject: resourceloader: Simplify addEmbeddedCSS by using object refs
X-Git-Tag: 1.34.0-rc.0~4269^2~1
X-Git-Url: https://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/comptes/journal.php?a=commitdiff_plain;h=19f4db30385518772f00ddf14bd948f467624b24;p=lhc%2Fweb%2Fwiklou.git

resourceloader: Simplify addEmbeddedCSS by using object refs

* Previously, the same function was used both for adding to the buffer,
  and flushing the buffer (via self-calling alternate signatures).
  The flushing logic was split off to a flushCssBuffer function.

* Previously, when encountering an '@import' statement, it performed a
  synchronous flush, instead of the usual asynchronous ones. There was
  no reason for this, other than my laziness. I suspect because I was
  using strings, which can't be passed by reference, and I didn't
  think of another way.

  I'm now storing the string in an object, which can be passed by
  reference to the flush function. This means, as before, we can
  keep appending to its string after the flush is scheduled. But,
  unlike before, it also means we can reset our local reference and
  start a new buffer at any time and schedule that one, too.

Bug: T202703
Change-Id: Ifc6dd59e9e8885d65ba425bc579ecbfb09f2ac64
---

diff --git a/resources/src/startup/mediawiki.js b/resources/src/startup/mediawiki.js
index 95dde2dd98..99cb31c085 100644
--- a/resources/src/startup/mediawiki.js
+++ b/resources/src/startup/mediawiki.js
@@ -633,10 +633,8 @@
 				 */
 				marker = document.querySelector( 'meta[name="ResourceLoaderDynamicStyles"]' ),
 
-				// For addEmbeddedCSS()
-				cssBuffer = '',
-				cssBufferTimer = null,
-				cssCallbacks = [],
+				// For #addEmbeddedCSS()
+				nextCssBuffer,
 				rAF = window.requestAnimationFrame || setTimeout;
 
 			/**
@@ -659,66 +657,62 @@
 				return el;
 			}
 
+			/**
+			 * @private
+			 * @param {Object} cssBuffer
+			 */
+			function flushCssBuffer( cssBuffer ) {
+				var i;
+				// Mark this object as inactive now so that further calls to addEmbeddedCSS() from
+				// the callbacks go to a new buffer instead of this one (T105973)
+				cssBuffer.active = false;
+				newStyleTag( cssBuffer.cssText, marker );
+				for ( i = 0; i < cssBuffer.callbacks.length; i++ ) {
+					cssBuffer.callbacks[ i ]();
+				}
+			}
+
 			/**
 			 * Add a bit of CSS text to the current browser page.
 			 *
-			 * The CSS will be appended to an existing ResourceLoader-created `<style>` tag
-			 * or create a new one based on whether the given `cssText` is safe for extension.
+			 * The creation and insertion of the `<style>` element is debounced for two reasons:
+			 *
+			 * - Performing the insertion before the next paint round via requestAnimationFrame
+			 *   avoids forced or wasted style recomputations, which are expensive in browsers.
+			 * - Reduce how often new stylesheets are inserted by letting additional calls to this
+			 *   function accumulate into a buffer for at least one JavaScript tick. Modules are
+			 *   received from the server in batches, which means there is likely going to be many
+			 *   calls to this function in a row within the same tick / the same call stack.
+			 *   See also T47810.
 			 *
 			 * @private
-			 * @param {string} [cssText=cssBuffer] If called without cssText,
-			 *  the internal buffer will be inserted instead.
-			 * @param {Function} [callback]
+			 * @param {string} cssText CSS text to be added in a `<style>` tag.
+			 * @param {Function} callback Called after the insertion has occurred
 			 */
 			function addEmbeddedCSS( cssText, callback ) {
-				function fireCallbacks() {
-					var i,
-						oldCallbacks = cssCallbacks;
-					// Reset cssCallbacks variable so it's not polluted by any calls to
-					// addEmbeddedCSS() from one of the callbacks (T105973)
-					cssCallbacks = [];
-					for ( i = 0; i < oldCallbacks.length; i++ ) {
-						oldCallbacks[ i ]();
-					}
-				}
-
-				if ( callback ) {
-					cssCallbacks.push( callback );
+				// Create a buffer if:
+				// - We don't have one yet.
+				// - The previous one is closed.
+				// - The next CSS chunk syntactically needs to be at the start of a stylesheet (T37562).
+				if ( !nextCssBuffer || nextCssBuffer.active === false || cssText.slice( 0, '@import'.length ) === '@import' ) {
+					nextCssBuffer = {
+						cssText: '',
+						callbacks: [],
+						active: null
+					};
 				}
 
-				// Yield once before creating the <style> tag. This lets multiple stylesheets
-				// accumulate into one buffer, allowing us to reduce how often new stylesheets
-				// are inserted in the browser. Appending a stylesheet and waiting for the
-				// browser to repaint is fairly expensive. (T47810)
-				if ( cssText ) {
-					// Don't extend the buffer if the item needs its own stylesheet.
-					// Keywords like `@import` are only valid at the start of a stylesheet (T37562).
-					if ( !cssBuffer || cssText.slice( 0, '@import'.length ) !== '@import' ) {
-						// Linebreak for somewhat distinguishable sections
-						cssBuffer += '\n' + cssText;
-						if ( !cssBufferTimer ) {
-							cssBufferTimer = rAF( function () {
-								// Wrap in anonymous function that takes no arguments
-								// Support: Firefox < 13
-								// Firefox 12 has non-standard behaviour of passing a number
-								// as first argument to a setTimeout callback.
-								// http://benalman.com/news/2009/07/the-mysterious-firefox-settime/
-								addEmbeddedCSS();
-							} );
-						}
-						return;
-					}
+				// Linebreak for somewhat distinguishable sections
+				nextCssBuffer.cssText += '\n' + cssText;
+				nextCssBuffer.callbacks.push( callback );
 
-				// This is a scheduled flush for the buffer
-				} else {
-					cssBufferTimer = null;
-					cssText = cssBuffer;
-					cssBuffer = '';
+				if ( nextCssBuffer.active === null ) {
+					nextCssBuffer.active = true;
+					// The flushCssBuffer callback has its parameter bound by reference, which means
+					// 1) We can still extend the buffer from our object reference after this point.
+					// 2) We can safely re-assign the variable (not the object) to start a new buffer.
+					rAF( flushCssBuffer.bind( null, nextCssBuffer ) );
 				}
-
-				newStyleTag( cssText, marker );
-
-				fireCallbacks();
 			}
 
 			/**