From: Timo Tijhof <ttijhof@wikimedia.org>
Date: Thu, 7 Mar 2013 00:24:05 +0000 (+0100)
Subject: mw.loader: Buffer cssText in addEmbeddedCSS.
X-Git-Tag: 1.31.0-rc.0~20428^2
X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=705d50c146ae157e636381f28da669048a719ea7;p=lhc%2Fweb%2Fwiklou.git

mw.loader: Buffer cssText in addEmbeddedCSS.

I52252e699a improved the repaint situation (bug 45810) a
bit, but not as much as we hoped. Inserting a stylesheet and
applying it to the page is a fairly expensive operation.
Invalidating the previous one in the process didn't help, but
fixing that didn't make a lot of difference.

This is a much more significant improvement by reducing the
number of <style> tags in the first place.

When a load.php request is handled, it reponds with many
mw.loader.implement() calls, one directly after the other
(not asynchronous in any way).

In the long term we could change the server side to combine
these better but for now the easiest fix with the largest gain
is to buffer it, yield once, and then insert it all at once.

This means we insert only one <style> for each load.php request.

Bug: 45810
Change-Id: I430fba9998b133a85dd3ac38237dc44b38630a9c
---

diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js
index af1204710e..2ceb3eac78 100644
--- a/resources/mediawiki/mediawiki.js
+++ b/resources/mediawiki/mediawiki.js
@@ -391,7 +391,9 @@ var mw = ( function ( $, undefined ) {
 				// List of callback functions waiting for modules to be ready to be called
 				jobs = [],
 				// Selector cache for the marker element. Use getMarker() to get/use the marker!
-				$marker = null;
+				$marker = null,
+				// Buffer for addEmbeddedCSS.
+				cssBuffer = '';
 
 			/* Private methods */
 
@@ -465,15 +467,46 @@ var mw = ( function ( $, undefined ) {
 				return cssText.indexOf( '@import' ) === -1;
 			}
 
+			/**
+			 * @param {string} [cssText=cssBuffer] If called without cssText,
+			 * the internal buffer will be inserted instead.
+			 */
 			function addEmbeddedCSS( cssText ) {
 				var $style, styleEl;
 
+				// Yield once before inserting the <style> tag. There are likely
+				// more calls coming up which we can combine this way.
+				// Appending a stylesheet and waiting for the browser to repaint
+				// is fairly expensive, this reduces it (bug 45810)
+				if ( cssText ) {
+					// Be careful not to extend the buffer with css that needs a new stylesheet
+					if ( !cssBuffer || canExpandStylesheetWith( cssText ) ) {
+						// Linebreak for somewhat distinguishable sections
+						// (the rl-cachekey comment separating each)
+						cssBuffer += '\n' + cssText;
+						// TODO: Use requestAnimationFrame in the future which will
+						// perform even better by not injecting styles while the browser
+						// is paiting.
+						setTimeout( addEmbeddedCSS );
+						return;
+					}
+
+				// This is a delayed call and we got a buffer still
+				} else if ( cssBuffer ) {
+					cssText = cssBuffer;
+					cssBuffer = '';
+				} else {
+					// This is a delayed call, but buffer is already cleared by
+					// another delayed call.
+					return;
+				}
+
 				// By default, always create a new <style>. Appending text
 				// to a <style> tag means the contents have to be re-parsed (bug 45810).
 				// Except, of course, in IE below 9, in there we default to
 				// re-using and appending to a <style> tag due to the
 				// IE stylesheet limit (bug 31676).
-				if ( ( 'documentMode' in document && document.documentMode <= 9 ) && canExpandStylesheetWith( cssText ) ) {
+				if ( 'documentMode' in document && document.documentMode <= 9 ) {
 
 					$style = getMarker().prev();
 					// Verify that the the element before Marker actually is a
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index b599b027d4..e8663f8ee0 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -333,11 +333,15 @@
 		mw.loader.implement(
 			'test.implement.a',
 			function () {
-				assert.equal(
-					$element.css( 'float' ),
-					'right',
-					'style is applied'
-				);
+				QUnit.stop();
+				setTimeout(function () {
+					assert.equal(
+						$element.css( 'float' ),
+						'right',
+						'style is applied'
+					);
+					QUnit.start();
+				});
 			},
 			{
 				'all': '.mw-test-implement-a { float: right; }'
@@ -426,11 +430,15 @@
 		mw.loader.implement(
 			'test.implement.c',
 			function () {
-				assert.equal(
-					$element.css( 'float' ),
-					'right',
-					'style is applied'
-				);
+				QUnit.stop();
+				setTimeout(function () {
+					assert.equal(
+						$element.css( 'float' ),
+						'right',
+						'style is applied'
+					);
+					QUnit.start();
+				});
 			},
 			{
 				'all': '.mw-test-implement-c { float: right; }'