Minor resource loader changes:

* Broke some long lines, converted some overly complex ternary operators to if()
* Added lots of line breaks into the output, for easier debugging.
* Added profiling for various functions.
* wfGetDb -> wfGetDB
* Fixed escaping in ResourceLoaderStartUpModule::getScript(), escape for JS rather than assuming Html::linkedScript() won't have any bad characters in it.

diff --git a/includes/MessageBlobStore.php b/includes/MessageBlobStore.php
index 0c23143..da9ce1b 100644 (file)
--- a/includes/MessageBlobStore.php
+++ b/includes/MessageBlobStore.php
@@ -37,7 +37,9 @@ class MessageBlobStore {
         */
        public static function get( $modules, $lang ) {
                // TODO: Invalidate blob when module touched
+               wfProfileIn( __METHOD__ );
                if ( !count( $modules ) ) {
+                       wfProfileOut( __METHOD__ );
                        return array();
                }
                // Try getting from the DB first
@@ -52,6 +54,7 @@ class MessageBlobStore {
                        }
                }
 
+               wfProfileOut( __METHOD__ );
                return $blobs;
        }
 
@@ -115,7 +118,8 @@ class MessageBlobStore {
         * Update all message blobs for a given module.
         * @param $module string Module name
         * @param $lang string Language code (optional)
-        * @return mixed If $lang is set, the new message blob for that language is returned if present. Otherwise, null is returned.
+        * @return mixed If $lang is set, the new message blob for that language is 
+        *    returned if present. Otherwise, null is returned.
         */
        public static function updateModule( $module, $lang = null ) {
                $retval = null;

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index cec102a..18ebfe2 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2325,9 +2325,9 @@ class OutputPage {
                                ksort( $query );
                                // Automatically select style/script elements
                                if ( $only === 'styles' ) {
-                                       $links .= Html::linkedStyle( wfAppendQuery( $wgLoadScript, $query ) );
+                                       $links .= Html::linkedStyle( wfAppendQuery( $wgLoadScript, $query ) ) . "\n";
                                } else {
-                                       $links .= Html::linkedScript( wfAppendQuery( $wgLoadScript, $query ) );
+                                       $links .= Html::linkedScript( wfAppendQuery( $wgLoadScript, $query ) ) . "\n";
                                }
                        }
                }
@@ -2378,8 +2378,8 @@ class OutputPage {
                if ( $this->getModules() ) {
                        $modules = FormatJson::encode( $this->getModules() );
                        $scripts .= Html::inlineScript(
-                               "if ( mediaWiki !== undefined ) { mediaWiki.loader.load( {$modules} ); mediaWiki.loader.go(); }"
-                       );
+                               "if ( mediaWiki !== undefined ) { mediaWiki.loader.load( {$modules} ); mediaWiki.loader.go(); }\n"
+                       ) . "\n";
                }
                
                // Add user JS if enabled - trying to load user.options as a bundle if possible

diff --git a/includes/ResourceLoader.php b/includes/ResourceLoader.php
index 889fbbb..555d341 100644 (file)
--- a/includes/ResourceLoader.php
+++ b/includes/ResourceLoader.php
@@ -41,10 +41,13 @@ class ResourceLoader {
                
                // Safety check - this should never be called more than once
                if ( !self::$initialized ) {
-                       // This needs to be first, because hooks might call ResourceLoader public interfaces which will call this
+                       wfProfileIn( __METHOD__ );
+                       // This needs to be first, because hooks might call ResourceLoader 
+                       // public interfaces which will call this
                        self::$initialized = true;
                        self::register( include( "$IP/resources/Resources.php" ) );
                        wfRunHooks( 'ResourceLoaderRegisterModules' );
+                       wfProfileOut( __METHOD__ );
                }
        }
 
@@ -53,14 +56,17 @@ class ResourceLoader {
         *
         * @param $filter String: name of filter to run
         * @param $data String: text to filter, such as JavaScript or CSS text
-        * @param $file String: path to file being filtered, (optional: only required for CSS to resolve paths)
+        * @param $file String: path to file being filtered, (optional: only required 
+        *     for CSS to resolve paths)
         * @return String: filtered data
         */
        protected static function filter( $filter, $data ) {
                global $wgMemc;
+               wfProfileIn( __METHOD__ );
 
                // For empty or whitespace-only things, don't do any processing
                if ( trim( $data ) === '' ) {
+                       wfProfileOut( __METHOD__ );
                        return $data;
                }
 
@@ -69,6 +75,7 @@ class ResourceLoader {
                $cached = $wgMemc->get( $key );
 
                if ( $cached !== false && $cached !== null ) {
+                       wfProfileOut( __METHOD__ );
                        return $cached;
                }
 
@@ -86,6 +93,7 @@ class ResourceLoader {
                                        break;
                                default:
                                        // Don't cache anything, just pass right through
+                                       wfProfileOut( __METHOD__ );
                                        return $data;
                        }
                } catch ( Exception $exception ) {
@@ -95,6 +103,7 @@ class ResourceLoader {
                // Save to memcached
                $wgMemc->set( $key, $result );
 
+               wfProfileOut( __METHOD__ );
                return $result;
        }
 
@@ -103,18 +112,21 @@ class ResourceLoader {
        /**
         * Registers a module with the ResourceLoader system.
         *
-        * Note that registering the same object under multiple names is not supported and may silently fail in all
-        * kinds of interesting ways.
+        * Note that registering the same object under multiple names is not supported 
+        * and may silently fail in all kinds of interesting ways.
         *
         * @param $name Mixed: string of name of module or array of name/object pairs
-        * @param $object ResourceLoaderModule: module object (optional when using multiple-registration calling style)
-        * @return Boolean: false if there were any errors, in which case one or more modules were not registered
+        * @param $object ResourceLoaderModule: module object (optional when using 
+        *    multiple-registration calling style)
+        * @return Boolean: false if there were any errors, in which case one or more 
+        *    modules were not registered
         *
-        * @todo We need much more clever error reporting, not just in detailing what happened, but in bringing errors to
-        * the client in a way that they can easily see them if they want to, such as by using FireBug
+        * @todo We need much more clever error reporting, not just in detailing what 
+        *    happened, but in bringing errors to the client in a way that they can 
+        *    easily see them if they want to, such as by using FireBug
         */
        public static function register( $name, ResourceLoaderModule $object = null ) {
-               
+               wfProfileIn( __METHOD__ );
                self::initialize();
                
                // Allow multiple modules to be registered in one call
@@ -123,6 +135,7 @@ class ResourceLoader {
                                self::register( $key, $value );
                        }
 
+                       wfProfileOut( __METHOD__ );
                        return;
                }
 
@@ -134,6 +147,7 @@ class ResourceLoader {
                // Attach module
                self::$modules[$name] = $object;
                $object->setName( $name );
+               wfProfileOut( __METHOD__ );
        }
 
        /**
@@ -162,13 +176,14 @@ class ResourceLoader {
        }
 
        /**
-        * Gets registration code for all modules, except pre-registered ones listed in self::$preRegisteredModules
+        * Gets registration code for all modules, except pre-registered ones listed in 
+        * self::$preRegisteredModules
         *
         * @param $context ResourceLoaderContext object
         * @return String: JavaScript code for registering all modules with the client loader
         */
        public static function getModuleRegistrations( ResourceLoaderContext $context ) {
-               
+               wfProfileIn( __METHOD__ );
                self::initialize();
                
                $scripts = '';
@@ -178,27 +193,34 @@ class ResourceLoader {
                        // Support module loader scripts
                        if ( ( $loader = $module->getLoaderScript() ) !== false ) {
                                $deps = FormatJson::encode( $module->getDependencies() );
-                               $version = wfTimestamp( TS_ISO_8601, round( $module->getModifiedTime( $context ), -2 ) );
-                               $scripts .= "( function( name, version, dependencies ) { $loader } )( '$name', '$version', $deps );";
+                               $version = wfTimestamp( TS_ISO_8601, 
+                                       round( $module->getModifiedTime( $context ), -2 ) );
+                               $scripts .= "( function( name, version, dependencies ) { $loader } )\n" . 
+                                       "( '$name', '$version', $deps );\n";
                        }
                        // Automatically register module
                        else {
-                               // Modules without dependencies pass two arguments (name, timestamp) to mediaWiki.loader.register()
+                               // Modules without dependencies pass two arguments (name, timestamp) to 
+                               // mediaWiki.loader.register()
                                if ( !count( $module->getDependencies() ) ) {
                                        $registrations[] = array( $name, $module->getModifiedTime( $context ) );
                                }
-                               // Modules with dependencies pass three arguments (name, timestamp, dependencies) to mediaWiki.loader.register()
+                               // Modules with dependencies pass three arguments (name, timestamp, dependencies) 
+                               // to mediaWiki.loader.register()
                                else {
-                                       $registrations[] = array( $name, $module->getModifiedTime( $context ), $module->getDependencies() );
+                                       $registrations[] = array( $name, $module->getModifiedTime( $context ), 
+                                               $module->getDependencies() );
                                }
                        }
                }
-               return $scripts . "mediaWiki.loader.register( " . FormatJson::encode( $registrations ) . " );";
+               $out = $scripts . "mediaWiki.loader.register( " . FormatJson::encode( $registrations ) . " );\n";
+               wfProfileOut( __METHOD__ );
+               return $out;
        }
 
        /**
-        * Get the highest modification time of all modules, based on a given combination of language code,
-        * skin name and debug mode flag.
+        * Get the highest modification time of all modules, based on a given 
+        * combination of language code, skin name and debug mode flag.
         *
         * @param $context ResourceLoaderContext object
         * @return Integer: UNIX timestamp
@@ -225,6 +247,7 @@ class ResourceLoader {
                global $wgResourceLoaderVersionedClientMaxage, $wgResourceLoaderVersionedServerMaxage;
                global $wgResourceLoaderUnversionedServerMaxage, $wgResourceLoaderUnversionedClientMaxage;
 
+               wfProfileIn( __METHOD__ );
                self::initialize();
                
                // Split requested modules into two groups, modules and missing
@@ -239,22 +262,27 @@ class ResourceLoader {
                        }
                }
 
-               // If a version wasn't specified we need a shorter expiry time for updates to propagate to clients quickly
+               // If a version wasn't specified we need a shorter expiry time for updates to 
+               // propagate to clients quickly
                if ( is_null( $context->getVersion() ) ) {
                        $maxage = $wgResourceLoaderUnversionedClientMaxage;
                        $smaxage = $wgResourceLoaderUnversionedServerMaxage;
                }
-               // If a version was specified we can use a longer expiry time since changing version numbers causes cache misses
+               // If a version was specified we can use a longer expiry time since changing 
+               // version numbers causes cache misses
                else {
                        $maxage = $wgResourceLoaderVersionedClientMaxage;
                        $smaxage = $wgResourceLoaderVersionedServerMaxage;
                }
 
-               // To send Last-Modified and support If-Modified-Since, we need to detect the last modified time
+               // To send Last-Modified and support If-Modified-Since, we need to detect 
+               // the last modified time
+               wfProfileIn( __METHOD__.'-getModifiedTime' );
                $mtime = 1;
                foreach ( $modules as $name ) {
                        $mtime = max( $mtime, self::$modules[$name]->getModifiedTime( $context ) );
                }
+               wfProfileOut( __METHOD__.'-getModifiedTime' );
 
                header( 'Content-Type: ' . ( $context->getOnly() === 'styles' ? 'text/css' : 'text/javascript' ) );
                header( 'Last-Modified: ' . wfTimestamp( TS_RFC2822, $mtime ) );
@@ -266,6 +294,7 @@ class ResourceLoader {
                if ( $ims !== false && $mtime >= wfTimestamp( TS_UNIX, $ims ) ) {
                        header( 'HTTP/1.0 304 Not Modified' );
                        header( 'Status: 304 Not Modified' );
+                       wfProfileOut( __METHOD__ );
                        return;
                }
 
@@ -278,18 +307,20 @@ class ResourceLoader {
 
                // Generate output
                foreach ( $modules as $name ) {
+                       wfProfileIn( __METHOD__ . '-' . $name );
                        // Scripts
                        $scripts = '';
 
                        if ( $context->shouldIncludeScripts() ) {
-                               $scripts .= self::$modules[$name]->getScript( $context );
+                               $scripts .= self::$modules[$name]->getScript( $context ) . "\n";
                        }
 
                        // Styles
                        $styles = array();
 
                        if (
-                               $context->shouldIncludeStyles() && ( count( $styles = self::$modules[$name]->getStyles( $context ) ) )
+                               $context->shouldIncludeStyles() 
+                               && ( count( $styles = self::$modules[$name]->getStyles( $context ) ) )
                        ) {
                                foreach ( $styles as $media => $style ) {
                                        if ( self::$modules[$name]->getFlip( $context ) ) {
@@ -330,6 +361,7 @@ class ResourceLoader {
                                }
                                echo "mediaWiki.loader.implement( '$name', function() {{$scripts}},\n$styles,\n$messages );\n";
                        }
+                       wfProfileOut( __METHOD__ . '-' . $name );
                }
 
                // Update the status of script-only modules
@@ -359,5 +391,6 @@ class ResourceLoader {
                                echo self::filter( 'minify-js', ob_get_clean() );
                        }
                }
+               wfProfileOut( __METHOD__ );
        }
-}
\ No newline at end of file
+}

diff --git a/includes/ResourceLoaderContext.php b/includes/ResourceLoaderContext.php
index 269712a..e8737a5 100644 (file)
--- a/includes/ResourceLoaderContext.php
+++ b/includes/ResourceLoaderContext.php
@@ -20,7 +20,8 @@
  */
 
 /**
- * Object passed around to modules which contains information about the state of a specific loader request
+ * Object passed around to modules which contains information about the state 
+ * of a specific loader request
  */
 class ResourceLoaderContext {
        /* Protected Members */
@@ -115,9 +116,12 @@ class ResourceLoaderContext {
        }
 
        public function getHash() {
-               return isset( $this->hash ) ?
-                       $this->hash : $this->hash = implode( '|', array(
-                               $this->language, $this->direction, $this->skin, $this->user, $this->debug, $this->only, $this->version
+               if ( isset( $this->hash ) ) {
+                       $this->hash = implode( '|', array(
+                               $this->language, $this->direction, $this->skin, $this->user, 
+                               $this->debug, $this->only, $this->version
                        ) );
+               }
+               return $this->hash;
        }
-}
\ No newline at end of file
+}

diff --git a/includes/ResourceLoaderModule.php b/includes/ResourceLoaderModule.php
index 10c7d25..f883c6f 100644 (file)
--- a/includes/ResourceLoaderModule.php
+++ b/includes/ResourceLoaderModule.php
@@ -380,7 +380,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                // Only store if modified
                if ( $files !== $this->getFileDependencies( $context->getSkin() ) ) {
                        $encFiles = FormatJson::encode( $files );
-                       $dbw = wfGetDb( DB_MASTER );
+                       $dbw = wfGetDB( DB_MASTER );
                        $dbw->replace( 'module_deps',
                                array( array( 'md_module', 'md_skin' ) ), array(
                                        'md_module' => $this->getName(),
@@ -430,6 +430,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                if ( isset( $this->modifiedTime[$context->getHash()] ) ) {
                        return $this->modifiedTime[$context->getHash()];
                }
+               wfProfileIn( __METHOD__ );
                
                // Sort of nasty way we can get a flat list of files depended on by all styles
                $styles = array();
@@ -454,13 +455,16 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        $this->loaders,
                        $this->getFileDependencies( $context->getSkin() )
                );
+               wfProfileIn( __METHOD__.'-filemtime' );
                $filesMtime = max( array_map( 'filemtime', array_map( array( __CLASS__, 'remapFilename' ), $files ) ) );
+               wfProfileOut( __METHOD__.'-filemtime' );
                // Only get the message timestamp if there are messages in the module
                $msgBlobMtime = 0;
                if ( count( $this->messages ) ) {
                        // Get the mtime of the message blob
-                       // TODO: This timestamp is queried a lot and queried separately for each module. Maybe it should be put in memcached?
-                       $dbr = wfGetDb( DB_SLAVE );
+                       // TODO: This timestamp is queried a lot and queried separately for each module. 
+                       // Maybe it should be put in memcached?
+                       $dbr = wfGetDB( DB_SLAVE );
                        $msgBlobMtime = $dbr->selectField( 'msg_resource', 'mr_timestamp', array(
                                        'mr_resource' => $this->getName(),
                                        'mr_lang' => $context->getLanguage()
@@ -469,6 +473,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        $msgBlobMtime = $msgBlobMtime ? wfTimestamp( TS_UNIX, $msgBlobMtime ) : 0;
                }
                $this->modifiedTime[$context->getHash()] = max( $filesMtime, $msgBlobMtime );
+               wfProfileOut( __METHOD__ );
                return $this->modifiedTime[$context->getHash()];
        }
 
@@ -576,7 +581,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                $deps = $wgMemc->get( $key );
 
                if ( !$deps ) {
-                       $dbr = wfGetDb( DB_SLAVE );
+                       $dbr = wfGetDB( DB_SLAVE );
                        $deps = $dbr->selectField( 'module_deps', 'md_deps', array(
                                        'md_module' => $this->getName(),
                                        'md_skin' => $skin,
@@ -601,7 +606,12 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
         * @return String: concatenated contents of $files
         */
        protected static function concatScripts( $files ) {
-               return implode( "\n", array_map( 'file_get_contents', array_map( array( __CLASS__, 'remapFilename' ), array_unique( (array) $files ) ) ) );
+               return implode( "\n", 
+                       array_map( 
+                               'file_get_contents', 
+                               array_map( 
+                                       array( __CLASS__, 'remapFilename' ), 
+                                       array_unique( (array) $files ) ) ) );
        }
 
        protected static function organizeFilesByOption( $files, $option, $default ) {
@@ -636,7 +646,10 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                $styles = self::organizeFilesByOption( $styles, 'media', 'all' );
                foreach ( $styles as $media => $files ) {
                        $styles[$media] =
-                               implode( "\n", array_map( array( __CLASS__, 'remapStyle' ), array_unique( (array) $files ) ) );
+                               implode( "\n", 
+                                       array_map( 
+                                               array( __CLASS__, 'remapStyle' ), 
+                                               array_unique( (array) $files ) ) );
                }
                return $styles;
        }
@@ -841,7 +854,11 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule {
 
        public function getScript( ResourceLoaderContext $context ) {
                $user = User::newFromName( $context->getUser() );
-               $options = FormatJson::encode( $user instanceof User ? $user->getOptions() : User::getDefaultOptions() );
+               if ( $user instanceof User ) {
+                       $options = FormatJson::encode( $user->getOptions() );
+               } else {
+                       $options = FormatJson::encode( User::getDefaultOptions() );
+               }
                return "mediaWiki.user.options.set( $options );";
        }
 
@@ -894,9 +911,11 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
        /* Protected Methods */
        
        protected function getConfig( $context ) {
-               global $wgLoadScript, $wgScript, $wgStylePath, $wgScriptExtension, $wgArticlePath, $wgScriptPath, $wgServer,
-                       $wgContLang, $wgBreakFrames, $wgVariantArticlePath, $wgActionPaths, $wgUseAjax, $wgVersion,
-                       $wgEnableAPI, $wgEnableWriteAPI, $wgDBname, $wgEnableMWSuggest, $wgSitename, $wgFileExtensions;
+               global $wgLoadScript, $wgScript, $wgStylePath, $wgScriptExtension, 
+                       $wgArticlePath, $wgScriptPath, $wgServer, $wgContLang, $wgBreakFrames, 
+                       $wgVariantArticlePath, $wgActionPaths, $wgUseAjax, $wgVersion, 
+                       $wgEnableAPI, $wgEnableWriteAPI, $wgDBname, $wgEnableMWSuggest, 
+                       $wgSitename, $wgFileExtensions;
 
                // Pre-process information
                $separatorTransTable = $wgContLang->separatorTransformTable();
@@ -965,7 +984,12 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        // Build configuration
                        $config = FormatJson::encode( $this->getConfig( $context ) );
                        // Add a well-known start-up function
-                       $scripts .= "window.startUp = function() { $registration mediaWiki.config.set( $config ); };";
+                       $scripts .= <<<JAVASCRIPT
+window.startUp = function() {
+       $registration
+       mediaWiki.config.set( $config ); 
+};
+JAVASCRIPT;
                        // Build load query for jquery and mediawiki modules
                        $query = array(
                                'modules' => implode( '|', array( 'jquery', 'mediawiki' ) ),
@@ -983,9 +1007,9 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        // Build HTML code for loading jquery and mediawiki modules
                        $loadScript = Html::linkedScript( $wgLoadScript . '?' . wfArrayToCGI( $query ) );
                        // Add code to add jquery and mediawiki loading code; only if the current client is compatible
-                       $scripts .= "if ( isCompatible() ) { document.write( '$loadScript' ); }";
+                       $scripts .= "if ( isCompatible() ) { document.write( " . FormatJson::encode( $loadScript ) . "); }\n";
                        // Delete the compatible function - it's not needed anymore
-                       $scripts .= "delete window['isCompatible'];";
+                       $scripts .= "delete window['isCompatible'];\n";
                }
 
                return $scripts;