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

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 6380efc..91acd11 100644 (file)
--- 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 9ed181e..7b85001 100644 (file)
--- 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
         */