From: Timo Tijhof <krinklemail@gmail.com>
Date: Fri, 31 Jul 2015 19:34:10 +0000 (-0700)
Subject: resourceloader: Remove ESI support (previously disabled)
X-Git-Tag: 1.31.0-rc.0~10555
X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=96fc60533fb583cae13de4bed74ade9e149d2495;p=lhc%2Fweb%2Fwiklou.git

resourceloader: Remove ESI support (previously disabled)

These were never enabled or used in production and are not
compatible with the upcoming async changes (T107399). To avoid
having to maintain compatibility with this, remove it for now.

The current on-going request to operations for ESI support is unrelated
to this code.

Considered making makeResourceLoaderLink() protected as it's not
used anywhere in @wikimedia Git outside mediawiki-core. And the unit
test actually treated it as protected already. However it's called
in SpecialJavaScriptTest so leaving that as-is for now.

In Icba6d7a87b239 the signature will change again with the removal
of the $loadCall parameter, which is obsolete in an async world
due to document.write being forbidden.

Change-Id: I9f557cc794638ffd15329934865e21e1027f7cfa
---

diff --git a/RELEASE-NOTES-1.26 b/RELEASE-NOTES-1.26
index fc0e752327..11da802d18 100644
--- a/RELEASE-NOTES-1.26
+++ b/RELEASE-NOTES-1.26
@@ -16,6 +16,8 @@ production.
   use the 'rawcontinue' parameter to receive raw query-continue data, but the
   new style is encouraged as it's harder to implement incorrectly.
 * Deprecated API formats dump and wddx have been completely removed.
+* $wgResourceLoaderUseESI was deprecated and removed. This was an experimental
+  feature that was never enabled by default.
 
 === New features in 1.26 ===
 * (T51506) Now action=info gives estimates of actual watchers for a page.
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 2517bc5c3d..4d1b3299cd 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -3510,13 +3510,6 @@ $wgResourceLoaderMaxage = array(
  */
 $wgResourceLoaderDebug = false;
 
-/**
- * Enable embedding of certain resources using Edge Side Includes. This will
- * improve performance but only works if there is something in front of the
- * web server (e..g a Squid or Varnish server) configured to process the ESI.
- */
-$wgResourceLoaderUseESI = false;
-
 /**
  * Put each statement on its own line when minifying JavaScript. This makes
  * debugging in non-debug mode a bit easier.
diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index c972045655..530a1d5b2e 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2766,15 +2766,14 @@ class OutputPage extends ContextSource {
 	 * @todo Document
 	 * @param array|string $modules One or more module names
 	 * @param string $only ResourceLoaderModule TYPE_ class constant
-	 * @param bool $useESI
 	 * @param array $extraQuery Array with extra query parameters to add to each
 	 *   request. array( param => value ).
 	 * @param bool $loadCall If true, output an (asynchronous) mw.loader.load()
 	 *   call rather than a "<script src='...'>" tag.
 	 * @return string The html "<script>", "<link>" and "<style>" tags
 	 */
-	public function makeResourceLoaderLink( $modules, $only, $useESI = false,
-		array $extraQuery = array(), $loadCall = false
+	public function makeResourceLoaderLink( $modules, $only, array $extraQuery = array(),
+		$loadCall = false
 	) {
 		$modules = (array)$modules;
 
@@ -2798,7 +2797,7 @@ class OutputPage extends ContextSource {
 			if ( ResourceLoader::inDebugMode() ) {
 				// Recursively call us for every item
 				foreach ( $modules as $name ) {
-					$link = $this->makeResourceLoaderLink( $name, $only, $useESI );
+					$link = $this->makeResourceLoaderLink( $name, $only );
 					$links['html'] = array_merge( $links['html'], $link['html'] );
 					$links['states'] += $link['states'];
 				}
@@ -2813,7 +2812,6 @@ class OutputPage extends ContextSource {
 		// Create keyed-by-source and then keyed-by-group list of module objects from modules list
 		$sortedModules = array();
 		$resourceLoader = $this->getResourceLoader();
-		$resourceLoaderUseESI = $this->getConfig()->get( 'ResourceLoaderUseESI' );
 		foreach ( $modules as $name ) {
 			$module = $resourceLoader->getModule( $name );
 			# Check that we're allowed to include this module on this page
@@ -2908,40 +2906,31 @@ class OutputPage extends ContextSource {
 				$moduleContext = new ResourceLoaderContext( $resourceLoader, new FauxRequest( $query ) );
 				$url = $resourceLoader->createLoaderURL( $source, $moduleContext, $extraQuery );
 
-				if ( $useESI && $resourceLoaderUseESI ) {
-					$esi = Xml::element( 'esi:include', array( 'src' => $url ) );
-					if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
-						$link = Html::inlineStyle( $esi );
-					} else {
-						$link = Html::inlineScript( $esi );
-					}
+				// Automatically select style/script elements
+				if ( $only === ResourceLoaderModule::TYPE_STYLES ) {
+					$link = Html::linkedStyle( $url );
+				} elseif ( $loadCall ) {
+					$link = ResourceLoader::makeInlineScript(
+						Xml::encodeJsCall( 'mw.loader.load', array( $url, 'text/javascript', true ) )
+					);
 				} else {
-					// Automatically select style/script elements
-					if ( $only === ResourceLoaderModule::TYPE_STYLES ) {
-						$link = Html::linkedStyle( $url );
-					} elseif ( $loadCall ) {
+					$link = Html::linkedScript( $url );
+					if ( !$context->getRaw() && !$isRaw ) {
+						// Wrap only=script / only=combined requests in a conditional as
+						// browsers not supported by the startup module would unconditionally
+						// execute this module. Otherwise users will get "ReferenceError: mw is
+						// undefined" or "jQuery is undefined" from e.g. a "site" module.
 						$link = ResourceLoader::makeInlineScript(
-							Xml::encodeJsCall( 'mw.loader.load', array( $url, 'text/javascript', true ) )
+							Xml::encodeJsCall( 'document.write', array( $link ) )
 						);
-					} else {
-						$link = Html::linkedScript( $url );
-						if ( !$context->getRaw() && !$isRaw ) {
-							// Wrap only=script / only=combined requests in a conditional as
-							// browsers not supported by the startup module would unconditionally
-							// execute this module. Otherwise users will get "ReferenceError: mw is
-							// undefined" or "jQuery is undefined" from e.g. a "site" module.
-							$link = ResourceLoader::makeInlineScript(
-								Xml::encodeJsCall( 'document.write', array( $link ) )
-							);
-						}
+					}
 
-						// For modules requested directly in the html via <link> or <script>,
-						// tell mw.loader they are being loading to prevent duplicate requests.
-						foreach ( $grpModules as $key => $module ) {
-							// Don't output state=loading for the startup module..
-							if ( $key !== 'startup' ) {
-								$links['states'][$key] = 'loading';
-							}
+					// For modules requested directly in the html via <link> or <script>,
+					// tell mw.loader they are being loading to prevent duplicate requests.
+					foreach ( $grpModules as $key => $module ) {
+						// Don't output state=loading for the startup module..
+						if ( $key !== 'startup' ) {
+							$links['states'][$key] = 'loading';
 						}
 					}
 				}
@@ -2994,7 +2983,7 @@ class OutputPage extends ContextSource {
 	function getHeadScripts() {
 		// Startup - this will immediately load jquery and mediawiki modules
 		$links = array();
-		$links[] = $this->makeResourceLoaderLink( 'startup', ResourceLoaderModule::TYPE_SCRIPTS, true );
+		$links[] = $this->makeResourceLoaderLink( 'startup', ResourceLoaderModule::TYPE_SCRIPTS );
 
 		// Load config before anything else
 		$links[] = ResourceLoader::makeInlineScript(
@@ -3048,12 +3037,12 @@ class OutputPage extends ContextSource {
 		$links = array();
 
 		$links[] = $this->makeResourceLoaderLink( $this->getModuleScripts( true, 'bottom' ),
-			ResourceLoaderModule::TYPE_SCRIPTS, /* $useESI = */ false, /* $extraQuery = */ array(),
+			ResourceLoaderModule::TYPE_SCRIPTS, /* $extraQuery = */ array(),
 			/* $loadCall = */ $inHead
 		);
 
 		$links[] = $this->makeResourceLoaderLink( $this->getModuleStyles( true, 'bottom' ),
-			ResourceLoaderModule::TYPE_STYLES, /* $useESI = */ false, /* $extraQuery = */ array(),
+			ResourceLoaderModule::TYPE_STYLES, /* $extraQuery = */ array(),
 			/* $loadCall = */ $inHead
 		);
 
@@ -3080,7 +3069,7 @@ class OutputPage extends ContextSource {
 		) {
 			// We're on a preview of a JS subpage. Exclude this page from the user module (T28283)
 			// and include the draft contents as a raw script instead.
-			$links[] = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_COMBINED, false,
+			$links[] = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_COMBINED,
 				array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() ), $inHead
 			);
 			// Load the previewed JS
@@ -3106,13 +3095,13 @@ class OutputPage extends ContextSource {
 		} else {
 			// Include the user module normally, i.e., raw to avoid it being wrapped in a closure.
 			$links[] = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_COMBINED,
-				/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
+				/* $extraQuery = */ array(), /* $loadCall = */ $inHead
 			);
 		}
 
 		// Group JS is only enabled if site JS is enabled.
 		$links[] = $this->makeResourceLoaderLink( 'user.groups', ResourceLoaderModule::TYPE_COMBINED,
-			/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
+			/* $extraQuery = */ array(), /* $loadCall = */ $inHead
 		);
 
 		return self::getHtmlFromLoaderLinks( $links );
@@ -3671,7 +3660,7 @@ class OutputPage extends ContextSource {
 		) {
 			// We're on a preview of a CSS subpage
 			// Exclude this page from the user module in case it's in there (bug 26283)
-			$link = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_STYLES, false,
+			$link = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_STYLES,
 				array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() )
 			);
 			$otherTags = array_merge( $otherTags, $link['html'] );
diff --git a/includes/specials/SpecialJavaScriptTest.php b/includes/specials/SpecialJavaScriptTest.php
index 4c73f1676e..442b7642cf 100644
--- a/includes/specials/SpecialJavaScriptTest.php
+++ b/includes/specials/SpecialJavaScriptTest.php
@@ -247,15 +247,16 @@ HTML;
 			'debug' => ResourceLoader::inDebugMode() ? 'true' : 'false',
 		) );
 
-		$styles = $out->makeResourceLoaderLink(
-			'jquery.qunit', ResourceLoaderModule::TYPE_STYLES, false
+		$styles = $out->makeResourceLoaderLink( 'jquery.qunit',
+			ResourceLoaderModule::TYPE_STYLES
 		);
 		// Use 'raw' since this is a plain HTML page without ResourceLoader
-		$scripts = $out->makeResourceLoaderLink(
-			'jquery.qunit', ResourceLoaderModule::TYPE_SCRIPTS, false, array( 'raw' => 'true' )
+		$scripts = $out->makeResourceLoaderLink( 'jquery.qunit',
+			ResourceLoaderModule::TYPE_SCRIPTS,
+			array( 'raw' => 'true' )
 		);
 
-		$head = trim( $styles['html'] . $scripts['html'] );
+		$head = implode( "\n", array_merge( $styles['html'], $scripts['html'] ) );
 		$summary = $this->getSummaryHtml();
 		$html = <<<HTML
 <!DOCTYPE html>
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 0b381682cc..69f55c382b 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -174,16 +174,6 @@ class OutputPageTest extends MediaWikiTestCase {
 					. "mw.test.baz({token:123});},{\"css\":[\".mw-icon{transition:none}\\n"
 					. "\"]});\n\n} );</script>"
 			),
-			// Load module script with ESI
-			array(
-				array( 'test.foo', ResourceLoaderModule::TYPE_SCRIPTS, true ),
-				'<script><esi:include src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.foo&amp;only=scripts&amp;skin=fallback&amp;*" /></script>'
-			),
-			// Load module styles with ESI
-			array(
-				array( 'test.foo', ResourceLoaderModule::TYPE_STYLES, true ),
-				'<style><esi:include src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.foo&amp;only=styles&amp;skin=fallback&amp;*" /></style>',
-			),
 			// Load no modules
 			array(
 				array( array(), ResourceLoaderModule::TYPE_COMBINED ),
@@ -219,7 +209,6 @@ class OutputPageTest extends MediaWikiTestCase {
 	public function testMakeResourceLoaderLink( $args, $expectedHtml ) {
 		$this->setMwGlobals( array(
 			'wgResourceLoaderDebug' => false,
-			'wgResourceLoaderUseESI' => true,
 			'wgLoadScript' => 'http://127.0.0.1:8080/w/load.php',
 			// Affects whether CDATA is inserted
 			'wgWellFormedXml' => false,