From f46f21915a84fc0e0eca84ccdddfd1ed01418ee7 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 17 Oct 2013 12:33:26 +0200
Subject: [PATCH] resourceloader: Use state "error" instead of "missing" in
 case of exceptions

Changes:
* If we catch an exception while making the response for a module, set its
  client state to "error" instead of "missing".
  State "missing" should only be used if the module could not be
  found in the registry.
  This matches the behaviour of the client.

Clean up:
* Merge the two mw.loader.state calls into one.
* Add @throws documentation for compileLESSFile (follows-up cdc8b9e).
* Don't use 3 different ways to assert an array being empty,
  using 1 and sticking to it.
  - !$arr
  - $arr === array()
  - count( $arr )

Change-Id: I54c3de6d836702ffbe3044bc58a38e83e758bc33
---
 includes/resourceloader/ResourceLoader.php    | 44 ++++++++++++-------
 .../ResourceLoaderFileModule.php              |  2 +
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 6380efcf95..91acd1143d 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -68,7 +68,8 @@ class ResourceLoader {
 	 */
 	public function preloadModuleInfo( array $modules, ResourceLoaderContext $context ) {
 		if ( !count( $modules ) ) {
-			return; // or else Database*::select() will explode, plus it's cheaper!
+			// Or else Database*::select() will explode, plus it's cheaper!
+			return;
 		}
 		$dbr = wfGetDB( DB_SLAVE );
 		$skin = $context->getSkin();
@@ -451,7 +452,7 @@ class ResourceLoader {
 		wfProfileIn( __METHOD__ );
 		$errors = '';
 
-		// Split requested modules into two groups, modules and missing
+		// Find out which modules are missing and instantiate the others
 		$modules = array();
 		$missing = array();
 		foreach ( $context->getModules() as $name ) {
@@ -527,7 +528,7 @@ class ResourceLoader {
 		}
 
 		// Save response to file cache unless there are errors
-		if ( isset( $fileCache ) && !$errors && !$missing ) {
+		if ( isset( $fileCache ) && !$errors && !count( $missing ) ) {
 			// Cache single modules...and other requests if there are enough hits
 			if ( ResourceFileCache::useFileCache( $context ) ) {
 				if ( $fileCache->isCacheWorthy() ) {
@@ -704,21 +705,24 @@ class ResourceLoader {
 	/**
 	 * Generates code for a response
 	 *
-	 * @param $context ResourceLoaderContext: Context in which to generate a response
+	 * @param $context ResourceLoaderContext Context in which to generate a response
 	 * @param array $modules List of module objects keyed by module name
-	 * @param array $missing List of unavailable modules (optional)
-	 * @return String: Response data
+	 * @param array $missing List of requested module names that are unregistered (optional)
+	 * @return string Response data
 	 */
 	public function makeModuleResponse( ResourceLoaderContext $context,
-		array $modules, $missing = array()
+		array $modules, array $missing = array()
 	) {
 		$out = '';
 		$exceptions = '';
-		if ( $modules === array() && $missing === array() ) {
+		$states = array();
+
+		if ( !count( $modules ) && !count( $missing ) ) {
 			return '/* No modules requested. Max made me put this here */';
 		}
 
 		wfProfileIn( __METHOD__ );
+
 		// Pre-fetch blobs
 		if ( $context->shouldIncludeMessages() ) {
 			try {
@@ -734,6 +738,11 @@ class ResourceLoader {
 			$blobs = array();
 		}
 
+
+		foreach ( $missing as $name ) {
+			$states[$name] = 'missing';
+		}
+
 		// Generate output
 		$isRaw = false;
 		foreach ( $modules as $name => $module ) {
@@ -838,8 +847,8 @@ class ResourceLoader {
 				// Add exception to the output as a comment
 				$exceptions .= self::formatException( $e );
 
-				// Register module as missing
-				$missing[] = $name;
+				// Respond to client with error-state instead of module implementation
+				$states[$name] = 'error';
 				unset( $modules[$name] );
 			}
 			$isRaw |= $module->isRaw();
@@ -848,14 +857,17 @@ class ResourceLoader {
 
 		// Update module states
 		if ( $context->shouldIncludeScripts() && !$context->getRaw() && !$isRaw ) {
-			// Set the state of modules loaded as only scripts to ready
 			if ( count( $modules ) && $context->getOnly() === 'scripts' ) {
-				$out .= self::makeLoaderStateScript(
-					array_fill_keys( array_keys( $modules ), 'ready' ) );
+				// Set the state of modules loaded as only scripts to ready as
+				// they don't have an mw.loader.implement wrapper that sets the state
+				foreach ( $modules as $name => $module ) {
+					$states[$name] = 'ready';
+				}
 			}
-			// Set the state of modules which were requested but unavailable as missing
-			if ( is_array( $missing ) && count( $missing ) ) {
-				$out .= self::makeLoaderStateScript( array_fill_keys( $missing, 'missing' ) );
+
+			// Set the state of modules we didn't respond to with mw.loader.implement
+			if ( count( $states ) ) {
+				$out .= self::makeLoaderStateScript( $states );
 			}
 		}
 
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index 9ed181ed77..7b85001878 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -745,6 +745,8 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
 	 * recompiles as necessary.
 	 *
 	 * @since 1.22
+	 * @throws Exception If Less encounters a parse error
+	 * @throws MWException If Less compilation returns unexpection result
 	 * @param string $fileName File path of LESS source
 	 * @return string: CSS source
 	 */
-- 
2.20.1