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
This commit is contained in:
jdlrobson 2018-04-09 18:22:13 -07:00
parent b1b0070dc7
commit a01d8be82c
4 changed files with 41 additions and 7 deletions

View file

@ -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 );

View file

@ -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 ] );

View file

@ -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

View file

@ -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' );
}
}