resourceloader: cache minified user and site modules

* Add support for a '/* @nomin */' annotation in ResourceLoader. If present in
  JavaScript or CSS, the code will not be minified or cached. This allows
  modules like the ResourceLoaderUserTokensModule to declare themselves unfit
  for minification / caching without requiring a complicated refactor.
* Make ResourceLoader::filter() static, at the cost of not having minifier
  errors in the ResourceLoader log bucket. (They will continue to be logged as
  exceptions, however).

Change-Id: Ic1d802ee20565e61046bfbd8fd209bc56a4cbd6c

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 31b3651..002b4ea 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -87,6 +87,9 @@ class ResourceLoader implements LoggerAwareInterface {
         */
        private $logger;
 
         */
        private $logger;
 

+       /** @var string JavaScript / CSS pragma to disable minification. **/
+       const FILTER_NOMIN = ' /* @nomin */ ';
+

        /**
         * Load information stored in the database about modules.
         *
        /**
         * Load information stored in the database about modules.
         *


@@ -180,79 +183,57 @@ class ResourceLoader implements LoggerAwareInterface {
         *
         * @param string $filter Name of filter to run
         * @param string $data Text to filter, such as JavaScript or CSS text
         *
         * @param string $filter Name of filter to run
         * @param string $data Text to filter, such as JavaScript or CSS text

-        * @param array $options For back-compat, can also be the boolean value for "cacheReport". Keys:
+        * @param array $options Keys:

         *  - (bool) cache: Whether to allow caching this data. Default: true.
         *  - (bool) cache: Whether to allow caching this data. Default: true.

-        *  - (bool) cacheReport: Whether to include the "cache key" report comment. Default: false.

         * @return string Filtered data, or a comment containing an error message
         */
         * @return string Filtered data, or a comment containing an error message
         */

-       public function filter( $filter, $data, $options = array() ) {
-               // Back-compat
-               if ( is_bool( $options ) ) {
-                       $options = array( 'cacheReport' => $options );
-               }
-               // Defaults
-               $options += array( 'cache' => true, 'cacheReport' => false );
-               $stats = RequestContext::getMain()->getStats();
-
-               // Don't filter empty content
-               if ( trim( $data ) === '' ) {
+       public static function filter( $filter, $data, Array $options = array() ) {
+               if ( strpos( $data, ResourceLoader::FILTER_NOMIN ) !== false ) {

                        return $data;
                }
 
                        return $data;
                }
 

-               if ( !in_array( $filter, array( 'minify-js', 'minify-css' ) ) ) {
-                       $this->logger->warning( 'Invalid filter {filter}', array(
-                               'filter' => $filter
-                       ) );
-                       return $data;
+               if ( isset( $options['cache'] ) && $options['cache'] === false ) {
+                       return self::applyFilter( $filter, $data );

                }
 
                }
 

-               if ( !$options['cache'] ) {
-                       $result = self::applyFilter( $filter, $data, $this->config );
+               $stats = RequestContext::getMain()->getStats();
+               $cache = ObjectCache::newAccelerator( CACHE_ANYTHING );
+
+               $key = wfGlobalCacheKey(
+                       'resourceloader',
+                       'filter',
+                       $filter,
+                       self::$filterCacheVersion, md5( $data )
+               );
+
+               $result = $cache->get( $key );
+               if ( $result === false ) {
+                       $stats->increment( "resourceloader_cache.$filter.miss" );
+                       $result = self::applyFilter( $filter, $data );
+                       $cache->set( $key, $result, 24 * 3600 );

                } else {
                } else {

-                       $key = wfGlobalCacheKey(
-                               'resourceloader',
-                               'filter',
-                               $filter,
-                               self::$filterCacheVersion, md5( $data )
-                       );
-                       $cache = wfGetCache( wfIsHHVM() ? CACHE_ACCEL : CACHE_ANYTHING );
-                       $cacheEntry = $cache->get( $key );
-                       if ( is_string( $cacheEntry ) ) {
-                               $stats->increment( "resourceloader_cache.$filter.hit" );
-                               return $cacheEntry;
-                       }
-                       $result = '';
-                       try {
-                               $statStart = microtime( true );
-                               $result = self::applyFilter( $filter, $data, $this->config );
-                               $statTiming = microtime( true ) - $statStart;
-                               $stats->increment( "resourceloader_cache.$filter.miss" );
-                               $stats->timing( "resourceloader_cache.$filter.timing", 1000 * $statTiming );
-                               if ( $options['cacheReport'] ) {
-                                       $result .= "\n/* cache key: $key */";
-                               }
-                               // Set a TTL since HHVM's APC doesn't have any limitation or eviction logic.
-                               $cache->set( $key, $result, 24 * 3600 );
-                       } catch ( Exception $e ) {
-                               MWExceptionHandler::logException( $e );
-                               $this->logger->warning( 'Minification failed: {exception}', array(
-                                       'exception' => $e
-                               ) );
-                               $this->errors[] = self::formatExceptionNoComment( $e );
-                       }
+                       $stats->increment( "resourceloader_cache.$filter.hit" );
+               }
+               if ( $result === null ) {
+                       // Cached failure
+                       $result = $data;

                }
 
                return $result;
        }
 
                }
 
                return $result;
        }
 

-       private static function applyFilter( $filter, $data, Config $config ) {
-               switch ( $filter ) {
-                       case 'minify-js':
-                               return JavaScriptMinifier::minify( $data );
-                       case 'minify-css':
-                               return CSSMin::minify( $data );
+       private static function applyFilter( $filter, $data ) {
+               $data = trim( $data );
+               if ( $data ) {
+                       try {
+                               $data = ( $filter === 'minify-css' )
+                                       ? CSSMin::minify( $data )
+                                       : JavaScriptMinifier::minify( $data );
+                       } catch ( Exception $e ) {
+                               MWExceptionHandler::logException( $e );
+                               return null;
+                       }

                }
                }

-

                return $data;
        }
 
                return $data;
        }
 


@@ -1040,11 +1021,7 @@ MESSAGE;
                                }
 
                                if ( !$context->getDebug() ) {
                                }
 
                                if ( !$context->getDebug() ) {

-                                       // Don't cache private modules. This is especially important in the case
-                                       // of modules which change every time they are built, like the embedded
-                                       // user.tokens module (bug T84960).
-                                       $filterOptions = array( 'cache' => ( $module->getGroup() !== 'private' ) );
-                                       $strContent = $this->filter( $filter, $strContent, $filterOptions );
+                                       $strContent = self::filter( $filter, $strContent );

                                }
 
                                $out .= $strContent;
                                }
 
                                $out .= $strContent;


@@ -1077,7 +1054,7 @@ MESSAGE;
                        if ( count( $states ) ) {
                                $stateScript = self::makeLoaderStateScript( $states );
                                if ( !$context->getDebug() ) {
                        if ( count( $states ) ) {
                                $stateScript = self::makeLoaderStateScript( $states );
                                if ( !$context->getDebug() ) {

-                                       $stateScript = $this->filter( 'minify-js', $stateScript );
+                                       $stateScript = self::filter( 'minify-js', $stateScript );

                                }
                                $out .= $stateScript;
                        }
                                }
                                $out .= $stateScript;
                        }


@@ -1118,8 +1095,7 @@ MESSAGE;
                                // Minify manually because the general makeModuleResponse() minification won't be
                                // effective here due to the script being a string instead of a function. (T107377)
                                if ( !ResourceLoader::inDebugMode() ) {
                                // Minify manually because the general makeModuleResponse() minification won't be
                                // effective here due to the script being a string instead of a function. (T107377)
                                if ( !ResourceLoader::inDebugMode() ) {

-                                       $scripts = self::applyFilter( 'minify-js', $scripts,
-                                               ConfigFactory::getDefaultInstance()->makeConfig( 'main' ) );
+                                       $scripts = self::filter( 'minify-js', $scripts );

                                }
                        } else {
                                $scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" );
                                }
                        } else {
                                $scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" );


@@ -1416,13 +1392,11 @@ MESSAGE;
         * @return string
         */
        public static function makeConfigSetScript( array $configuration ) {
         * @return string
         */
        public static function makeConfigSetScript( array $configuration ) {

-               if ( ResourceLoader::inDebugMode() ) {
-                       return Xml::encodeJsCall( 'mw.config.set', array( $configuration ), true );
-               }
-
-               $config = RequestContext::getMain()->getConfig();
-               $js = Xml::encodeJsCall( 'mw.config.set', array( $configuration ), false );
-               return self::applyFilter( 'minify-js', $js, $config );
+               return Xml::encodeJsCall(
+                       'mw.config.set',
+                       array( $configuration ),
+                       ResourceLoader::inDebugMode()
+               ) . ResourceLoader::FILTER_NOMIN;

        }
 
        /**
        }
 
        /**

diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php
index 7f7aa76..4ddfe55 100644 (file)
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -619,11 +619,11 @@ abstract class ResourceLoaderModule {
                                                                foreach ( $style as $cssText ) {
                                                                        if ( is_string( $cssText ) ) {
                                                                                $stylePairs[$media][] =
                                                                foreach ( $style as $cssText ) {
                                                                        if ( is_string( $cssText ) ) {
                                                                                $stylePairs[$media][] =

-                                                                                       $rl->filter( 'minify-css', $cssText );
+                                                                                       ResourceLoader::filter( 'minify-css', $cssText );

                                                                        }
                                                                }
                                                        } elseif ( is_string( $style ) ) {
                                                                        }
                                                                }
                                                        } elseif ( is_string( $style ) ) {

-                                                               $stylePairs[$media] = $rl->filter( 'minify-css', $style );
+                                                               $stylePairs[$media] = ResourceLoader::filter( 'minify-css', $style );

                                                        }
                                                }
                                        }
                                                        }
                                                }
                                        }

diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php
index e5f3fb8..1857d23 100644 (file)
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -221,13 +221,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
 
                        $skipFunction = $module->getSkipFunction();
                        if ( $skipFunction !== null && !ResourceLoader::inDebugMode() ) {
 
                        $skipFunction = $module->getSkipFunction();
                        if ( $skipFunction !== null && !ResourceLoader::inDebugMode() ) {

-                               $skipFunction = $resourceLoader->filter( 'minify-js',
-                                       $skipFunction,
-                                       // There will potentially be lots of these little strings in the registrations
-                                       // manifest, we don't want to blow up the startup module with
-                                       // "/* cache key: ... */" all over it.
-                                       /* cacheReport = */ false
-                               );
+                               $skipFunction = ResourceLoader::filter( 'minify-js', $skipFunction );

                        }
 
                        $registryData[$name] = array(
                        }
 
                        $registryData[$name] = array(

diff --git a/includes/resourceloader/ResourceLoaderUserTokensModule.php b/includes/resourceloader/ResourceLoaderUserTokensModule.php
index ccd1dfd..28f1b9a 100644 (file)
--- a/includes/resourceloader/ResourceLoaderUserTokensModule.php
+++ b/includes/resourceloader/ResourceLoaderUserTokensModule.php
@@ -51,14 +51,20 @@ class ResourceLoaderUserTokensModule extends ResourceLoaderModule {
        }
 
        /**
        }
 
        /**

+        * Generate the JavaScript content of this module.
+        *
+        * Add '@nomin' annotation to prevent the module's contents from getting
+        * cached (T84960).
+        *

         * @param ResourceLoaderContext $context
         * @return string
         */
        public function getScript( ResourceLoaderContext $context ) {
         * @param ResourceLoaderContext $context
         * @return string
         */
        public function getScript( ResourceLoaderContext $context ) {

-               return Xml::encodeJsCall( 'mw.user.tokens.set',
+               return Xml::encodeJsCall(
+                       'mw.user.tokens.set',

                        array( $this->contextUserTokens( $context ) ),
                        ResourceLoader::inDebugMode()
                        array( $this->contextUserTokens( $context ) ),
                        ResourceLoader::inDebugMode()

-               );
+               ) . ResourceLoader::FILTER_NOMIN;

        }
 
        /**
        }
 
        /**