From 19a40cd3ad589369a4f4a5faf1af4530d7c9c906 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 23 Jul 2015 21:00:39 -0700
Subject: [PATCH] resourceloader: Implement support for 'site' into mw.loader

* No longer a dedicated <script> with only=scripts.
  This means it creates no extra script request and becomes a versioned
  request using data from the startup module.

* No longer in group=site.
  This means it collapses into the existing bottom queue.
  Not even one dedicated script request, but zero.

* No longer exclude from module storage. This can be cached like any other module.
  It was previously excluded because it was already loaded separately.

* Change mw.loader#execute to special-case the 'site' module with $.globalEval.

* Add hack to ensure the styles of the 'site' module still load without
  JavaScript, in the top, and after the ResourceLoaderDynamicStyles marker.
  This unfortunately stays its own request. Not sure how to avoid that.

Bug: T32358
Bug: T106736
Bug: T102077
Change-Id: I291a8c3aae1a71760bec58161891c1bd77c9b724
---
 includes/OutputPage.php                       | 27 +++++++++++--------
 includes/resourceloader/ResourceLoader.php    |  6 ++++-
 .../ResourceLoaderSiteModule.php              |  9 -------
 resources/src/mediawiki/mediawiki.js          | 24 ++++++++++++-----
 4 files changed, 38 insertions(+), 28 deletions(-)

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index a551fe17d5..1c76f0bc41 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2286,9 +2286,10 @@ class OutputPage extends ContextSource {
 			// add skin specific modules
 			$modules = $sk->getDefaultModules();
 
-			// enforce various default modules for all skins
+			// Enforce various default modules for all skins
 			$coreModules = array(
-				// keep this list as small as possible
+				// Keep this list as small as possible
+				'site',
 				'mediawiki.page.startup',
 				'mediawiki.user',
 			);
@@ -3004,7 +3005,8 @@ class OutputPage extends ContextSource {
 		// Separate user.tokens as otherwise caching will be allowed (T84960)
 		$links[] = $this->makeResourceLoaderLink( 'user.tokens', ResourceLoaderModule::TYPE_COMBINED );
 
-		// Scripts and messages "only" requests marked for top inclusion
+		// "Scripts only" modules marked for top inclusion
+		$styleModules = $this->getModuleScripts( true, 'top' );
 		$links[] = $this->makeResourceLoaderLink(
 			$this->getModuleScripts( true, 'top' ),
 			ResourceLoaderModule::TYPE_SCRIPTS
@@ -3064,11 +3066,6 @@ class OutputPage extends ContextSource {
 		// Legacy Scripts
 		$links[] = "\n" . $this->mScripts;
 
-		// Add site JS if enabled
-		$links[] = $this->makeResourceLoaderLink( 'site', ResourceLoaderModule::TYPE_SCRIPTS,
-			/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
-		);
-
 		// Add user JS if enabled
 		if ( $this->getConfig()->get( 'AllowUserJs' )
 			&& $this->getUser()->isLoggedIn()
@@ -3683,9 +3680,17 @@ class OutputPage extends ContextSource {
 			if ( !$module ) {
 				continue;
 			}
+			if ( $name === 'site' ) {
+				// HACK: The site module shouldn't be fragmented with a cache group and
+				// http request. But in order to ensure its styles are separated and after the
+				// ResourceLoaderDynamicStyles marker, pretend it is in a group called 'site'.
+				// The scripts remain ungrouped and rides the bottom queue.
+				$styles['site'][] = $name;
+				continue;
+			}
 			$group = $module->getGroup();
-			// Modules in groups different than the ones listed on top (see $styles assignment)
-			// will be placed in the "other" group
+			// Modules in groups other than the ones needing special treatment (see $styles assignment)
+			// will be placed in the "other" style category.
 			$styles[isset( $styles[$group] ) ? $group : 'other'][] = $name;
 		}
 
@@ -3704,7 +3709,7 @@ class OutputPage extends ContextSource {
 			array( 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' )
 		) . "\n";
 
-		// Add site, private and user styles
+		// Add site-specific and user-specific styles
 		// 'private' at present only contains user.options, so put that before 'user'
 		// Any future private modules will likely have a similar user-specific character
 		foreach ( array( 'site', 'noscript', 'private', 'user' ) as $group ) {
diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index bf34c221f4..305b197d03 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1105,7 +1105,11 @@ MESSAGE;
 		$name, $scripts, $styles, $messages, $templates
 	) {
 		if ( is_string( $scripts ) ) {
-			$scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" );
+			// Site module is a legacy script that runs in the global scope (no closure).
+			// Transportation as string instructs mw.loader.implement to use globalEval.
+			if ( $name !== 'site' ) {
+				$scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" );
+			}
 		} elseif ( !is_array( $scripts ) ) {
 			throw new MWException( 'Invalid scripts error. Array of URLs or string of code expected.' );
 		}
diff --git a/includes/resourceloader/ResourceLoaderSiteModule.php b/includes/resourceloader/ResourceLoaderSiteModule.php
index 19e0baeb5e..380b7a53f3 100644
--- a/includes/resourceloader/ResourceLoaderSiteModule.php
+++ b/includes/resourceloader/ResourceLoaderSiteModule.php
@@ -47,13 +47,4 @@ class ResourceLoaderSiteModule extends ResourceLoaderWikiModule {
 		}
 		return $pages;
 	}
-
-	/**
-	 * Get group name
-	 *
-	 * @return string
-	 */
-	public function getGroup() {
-		return 'site';
-	}
 }
diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index c0b1642543..700513e4ee 100644
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1158,11 +1158,14 @@
 
 				if ( !hasOwn.call( registry, module ) ) {
 					throw new Error( 'Module has not been registered yet: ' + module );
-				} else if ( registry[module].state === 'registered' ) {
+				}
+				if ( registry[module].state === 'registered' ) {
 					throw new Error( 'Module has not been requested from the server yet: ' + module );
-				} else if ( registry[module].state === 'loading' ) {
+				}
+				if ( registry[module].state === 'loading' ) {
 					throw new Error( 'Module has not completed loading yet: ' + module );
-				} else if ( registry[module].state === 'ready' ) {
+				}
+				if ( registry[module].state === 'ready' ) {
 					throw new Error( 'Module has already been executed: ' + module );
 				}
 
@@ -1210,11 +1213,18 @@
 						if ( $.isArray( script ) ) {
 							nestedAddScript( script, markModuleReady, registry[module].async, 0 );
 						} else if ( $.isFunction( script ) ) {
-							registry[module].state = 'ready';
 							// Pass jQuery twice so that the signature of the closure which wraps
 							// the script can bind both '$' and 'jQuery'.
+							registry[module].state = 'ready';
 							script( $, $ );
 							handlePending( module );
+						} else if ( typeof script === 'string' ) {
+							// Site module is a legacy script that runs in the global scope. This is transported
+							// as a string instead of a function to avoid needing to use string manipulation to
+							// undo the function wrapper.
+							registry[module].state = 'ready';
+							$.globalEval( script );
+							handlePending( module );
 						}
 					} catch ( e ) {
 						// This needs to NOT use mw.log because these errors are common in production mode
@@ -1773,8 +1783,8 @@
 					if ( typeof module !== 'string' ) {
 						throw new Error( 'module must be of type string, not ' + typeof module );
 					}
-					if ( script && !$.isFunction( script ) && !$.isArray( script ) ) {
-						throw new Error( 'script must be of type function or array, not ' + typeof script );
+					if ( script && !$.isFunction( script ) && !$.isArray( script ) && typeof script !== 'string' ) {
+						throw new Error( 'script must be of type function, array, or script; not ' + typeof script );
 					}
 					if ( style && !$.isPlainObject( style ) ) {
 						throw new Error( 'style must be of type object, not ' + typeof style );
@@ -2172,7 +2182,7 @@
 							// Module failed to load
 							descriptor.state !== 'ready' ||
 							// Unversioned, private, or site-/user-specific
-							( !descriptor.version || $.inArray( descriptor.group, [ 'private', 'user', 'site' ] ) !== -1 ) ||
+							( !descriptor.version || $.inArray( descriptor.group, [ 'private', 'user' ] ) !== -1 ) ||
 							// Partial descriptor
 							$.inArray( undefined, [ descriptor.script, descriptor.style,
 									descriptor.messages, descriptor.templates ] ) !== -1
-- 
2.20.1