Skins: getDefaultStyles can now define render blocking CSS

This optimisation attempts to minimise loading the styles in places
they are not needed.

The logic is kept inside Skin::getDefaultModules to avoid fragmentation
of where modules get defined.

Update ApiParse to avoid repetition of code.

Bug: T42792
Bug: T42812
Change-Id: I59f02a7bab3baa9d43f6bc2ef1f549d9d31d8456

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 56df0f0..0b6e616 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2331,6 +2331,23 @@ class OutputPage extends ContextSource {
                }
        }
 
+       /**
+        * Transfer styles and JavaScript modules from skin.
+        *
+        * @param Skin $sk to load modules for
+        */
+       public function loadSkinModules( $sk ) {
+               foreach ( $sk->getDefaultModules() as $group => $modules ) {
+                       if ( $group === 'styles' ) {
+                               foreach ( $modules as $key => $moduleMembers ) {
+                                       $this->addModuleStyles( $moduleMembers );
+                               }
+                       } else {
+                               $this->addModules( $modules );
+                       }
+               }
+       }
+
        /**
         * Finally, all the text has been munged and accumulated into
         * the object, let's actually output it:
@@ -2424,9 +2441,7 @@ class OutputPage extends ContextSource {
                        }
 
                        $sk = $this->getSkin();
-                       foreach ( $sk->getDefaultModules() as $group ) {
-                               $this->addModules( $group );
-                       }
+                       $this->loadSkinModules( $sk );
 
                        MWDebug::addModules( $this );
 

diff --git a/includes/api/ApiParse.php b/includes/api/ApiParse.php
index 05b4289..096122d 100644 (file)
--- a/includes/api/ApiParse.php
+++ b/includes/api/ApiParse.php
@@ -323,9 +323,7 @@ class ApiParse extends ApiBase {
                                // Based on OutputPage::headElement()
                                $skin->setupSkinUserCss( $outputPage );
                                // Based on OutputPage::output()
-                               foreach ( $skin->getDefaultModules() as $group ) {
-                                       $outputPage->addModules( $group );
-                               }
+                               $outputPage->loadSkinModules( $skin );
                        }
 
                        Hooks::run( 'ApiParseMakeOutputPage', [ $this, $outputPage ] );

diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index f3276e8..9c4ac50 100644 (file)
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -166,7 +166,8 @@ abstract class Skin extends ContextSource {
         * It is recommended that skins wishing to override call parent::getDefaultModules()
         * and substitute out any modules they wish to change by using a key to look them up
         *
-        * For style modules, use setupSkinUserCss() instead.
+        * Any modules defined with the 'styles' key will be added as render blocking CSS via
+        * Output::addModuleStyles. Similarly, each key should refer to a list of modules
         *
         * @return array Array of modules with helper keys for easy overriding
         */
@@ -175,6 +176,10 @@ abstract class Skin extends ContextSource {
                $config = $this->getConfig();
                $user = $out->getUser();
                $modules = [
+                       // Styles key sets render blocking styles
+                       // Unlike other keys in this definition it is an associative array
+                       // where each key is the group name and points to a list of modules
+                       'styles' => [],
                        // modules not specific to any specific skin or page
                        'core' => [
                                // Enforce various default modules for all pages and all skins

diff --git a/tests/phpunit/includes/skins/SkinTest.php b/tests/phpunit/includes/skins/SkinTest.php
new file mode 100644 (file)
index 0000000..41ef2b7
--- /dev/null
+++ b/tests/phpunit/includes/skins/SkinTest.php
@@ -0,0 +1,16 @@
+<?php
+class SkinTest extends MediaWikiTestCase {
+
+       /**
+        * @covers Skin::getDefaultModules
+        */
+       public function testGetDefaultModules() {
+               $skin = $this->getMockBuilder( Skin::class )
+                       ->setMethods( [ 'outputPage', 'setupSkinUserCss' ] )
+                       ->getMock();
+
+               $modules = $skin->getDefaultModules();
+               $this->assertTrue( isset( $modules['core'] ), 'core key is set by default' );
+               $this->assertTrue( isset( $modules['styles'] ), 'style key is set by default' );
+       }
+}