From 004f1edb37b491b026568998d5c166c93ba4e4e9 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Fri, 17 Sep 2010 11:45:49 +0000 Subject: [PATCH] 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. --- includes/MessageBlobStore.php | 6 ++- includes/OutputPage.php | 8 +-- includes/ResourceLoader.php | 83 +++++++++++++++++++++--------- includes/ResourceLoaderContext.php | 14 +++-- includes/ResourceLoaderModule.php | 50 +++++++++++++----- 5 files changed, 113 insertions(+), 48 deletions(-) diff --git a/includes/MessageBlobStore.php b/includes/MessageBlobStore.php index 0c23143a10..da9ce1b9db 100644 --- 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 cec102a598..18ebfe2e8d 100644 --- 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 889fbbbff2..555d3413bb 100644 --- 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 269712aa52..e8737a5a73 100644 --- 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 10c7d25c18..f883c6f68e 100644 --- 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 .= << 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; -- 2.20.1