resourceloader: Don't cache minification of user.tokens

As of b1e4006b4, the tokens are different on every request.
Caching these is completely useless because the cache entry is
simply unreachable and is extra overhead on every request for
logged-in users to save content to Memcached.

Whether they should be minified at all and whether they perhaps
shouldn't change on every request is a separate matter.

Bug: T84960
Change-Id: I6016e4b01e44e0acbfd6d49f7d99555e2290c9cb

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index a3a5a27..ba9fcba 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2981,8 +2981,10 @@ class OutputPage extends ContextSource {
                // Load embeddable private modules before any loader links
                // This needs to be TYPE_COMBINED so these modules are properly wrapped
                // in mw.loader.implement() calls and deferred until mw.user is available
-               $embedScripts = array( 'user.options', 'user.tokens' );
+               $embedScripts = array( 'user.options' );
                $links[] = $this->makeResourceLoaderLink( $embedScripts, ResourceLoaderModule::TYPE_COMBINED );
+               // Separate user.tokens as otherwise caching will be allowed (T84960)
+               $links[] = $this->makeResourceLoaderLink( 'user.tokens', ResourceLoaderModule::TYPE_COMBINED );
 
                // Scripts and messages "only" requests marked for top inclusion
                $links[] = $this->makeResourceLoaderLink(

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 5a9465e..b4b5a2e 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -169,58 +169,69 @@ class ResourceLoader {
         *
         * @param string $filter Name of filter to run
         * @param string $data Text to filter, such as JavaScript or CSS text
-        * @param string $cacheReport Whether to include the cache key report
+        * @param array $options For back-compat, can also be the boolean value for "cacheReport". Keys:
+        *  - (bool) cache: Whether to allow caching this data. Default: true.
+        *  - (bool) cacheReport: Whether to include the "cache key" report comment. Default: true.
         * @return string Filtered data, or a comment containing an error message
         */
-       public function filter( $filter, $data, $cacheReport = true ) {
+       public function filter( $filter, $data, $options = array() ) {
+               // Back-compat
+               if ( is_bool( $options ) ) {
+                       $options = array( 'cacheReport' => $options );
+               }
+               // Defaults
+               $options += array( 'cache' => true, 'cacheReport' => true );
 
-               // For empty/whitespace-only data or for unknown filters, don't perform
-               // any caching or processing
-               if ( trim( $data ) === '' || !in_array( $filter, array( 'minify-js', 'minify-css' ) ) ) {
+               // Don't filter empty content
+               if ( trim( $data ) === '' ) {
                        return $data;
                }
 
-               // Try for cache hit
-               // Use CACHE_ANYTHING since filtering is very slow compared to DB queries
-               $key = wfMemcKey( 'resourceloader', 'filter', $filter, self::$filterCacheVersion, md5( $data ) );
-               $cache = wfGetCache( CACHE_ANYTHING );
-               $cacheEntry = $cache->get( $key );
-               if ( is_string( $cacheEntry ) ) {
-                       wfIncrStats( "rl-$filter-cache-hits" );
-                       return $cacheEntry;
+               if ( !in_array( $filter, array( 'minify-js', 'minify-css' ) ) ) {
+                       wfDebugLog( 'resourceloader', __METHOD__ . ": Invalid filter: $filter" );
+                       return $data;
                }
 
-               $result = '';
-               // Run the filter - we've already verified one of these will work
-               try {
-                       wfIncrStats( "rl-$filter-cache-misses" );
+               if ( !$options['cache'] ) {
+                       $result = $this->applyFilter( $filter, $data );
+               } else {
+                       // Use CACHE_ANYTHING since filtering is very slow compared to DB queries
+                       $key = wfMemcKey( 'resourceloader', 'filter', $filter, self::$filterCacheVersion, md5( $data ) );
+                       $cache = wfGetCache( CACHE_ANYTHING );
+                       $cacheEntry = $cache->get( $key );
+                       if ( is_string( $cacheEntry ) ) {
+                               wfIncrStats( "rl-$filter-cache-hits" );
+                               return $cacheEntry;
+                       }
+                       $result = '';
+                       try {
+                               wfIncrStats( "rl-$filter-cache-misses" );
+                               $result = $this->applyFilter( $filter, $data );
+                               if ( $options['cacheReport'] ) {
+                                       $result .= "\n/* cache key: $key */";
+                               }
+                               $cache->set( $key, $result );
+                       } catch ( Exception $e ) {
+                               MWExceptionHandler::logException( $e );
+                               wfDebugLog( 'resourceloader', __METHOD__ . ": minification failed: $e" );
+                               $this->errors[] = self::formatExceptionNoComment( $e );
+                       }
+               }
+
+               return $result;
+       }
+
+       private function applyFilter( $filter, $data ) {
                        switch ( $filter ) {
                                case 'minify-js':
-                                       $result = JavaScriptMinifier::minify( $data,
+                                       return JavaScriptMinifier::minify( $data,
                                                $this->config->get( 'ResourceLoaderMinifierStatementsOnOwnLine' ),
                                                $this->config->get( 'ResourceLoaderMinifierMaxLineLength' )
                                        );
-                                       if ( $cacheReport ) {
-                                               $result .= "\n/* cache key: $key */";
-                                       }
-                                       break;
                                case 'minify-css':
-                                       $result = CSSMin::minify( $data );
-                                       if ( $cacheReport ) {
-                                               $result .= "\n/* cache key: $key */";
-                                       }
-                                       break;
+                                       return CSSMin::minify( $data );
                        }
-
-                       // Save filtered text to Memcached
-                       $cache->set( $key, $result );
-               } catch ( Exception $e ) {
-                       MWExceptionHandler::logException( $e );
-                       wfDebugLog( 'resourceloader', __METHOD__ . ": minification failed: $e" );
-                       $this->errors[] = self::formatExceptionNoComment( $e );
-               }
-
-               return $result;
+                       return $data;
        }
 
        /* Methods */
@@ -1078,11 +1089,19 @@ MESSAGE;
                        }
                }
 
+               $enableFilterCache = true;
+               if ( count( $modules ) === 1 && reset( $modules ) instanceof ResourceLoaderUserTokensModule ) {
+                       // If we're building the embedded user.tokens, don't cache (T84960)
+                       $enableFilterCache = false;
+               }
+
                if ( !$context->getDebug() ) {
                        if ( $context->getOnly() === 'styles' ) {
                                $out = $this->filter( 'minify-css', $out );
                        } else {
-                               $out = $this->filter( 'minify-js', $out );
+                               $out = $this->filter( 'minify-js', $out, array(
+                                       'cache' => $enableFilterCache
+                               ) );
                        }
                }
 

diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php
index 06054ee..6c078b0 100644 (file)
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -217,9 +217,9 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        if ( $skipFunction !== null && !ResourceLoader::inDebugMode() ) {
                                $skipFunction = $resourceLoader->filter( 'minify-js',
                                        $skipFunction,
-                                       // There will potentially be lots of these little string in the registrations
+                                       // 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 in non-debug mode.
+                                       // "/* cache key: ... */" all over it.
                                        /* cacheReport = */ false
                                );
                        }