Introduce SkinFactory

Modeled similar to ConfigFactory, this lets skins
register themselves via a callback, allowing for
proper dependency injection.

Loading via $wgValidSkinNames is still supported,
but considered "legacy", not deprecated though.

Skin::newFromKey is now deprecated (and had only
one caller in an extension, which I'll update
afterwards).

Change-Id: I1960483f87c2ef55c994545239b728fa376f17f4

11 files changed, 305 insertions, 87 deletions

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index 872e89b7cc3c..c535ca0c30bd 100644
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -36,7 +36,6 @@ $wgAutoloadLocalClasses = array(
 	'AuthPlugin' => 'includes/AuthPlugin.php',
 	'AuthPluginUser' => 'includes/AuthPlugin.php',
 	'Autopromote' => 'includes/Autopromote.php',
-	'BaseTemplate' => 'includes/SkinTemplate.php',
 	'Block' => 'includes/Block.php',
 	'Category' => 'includes/Category.php',
 	'Categoryfinder' => 'includes/Categoryfinder.php',
@@ -120,7 +119,6 @@ $wgAutoloadLocalClasses = array(
 	'MagicWordArray' => 'includes/MagicWord.php',
 	'MailAddress' => 'includes/UserMailer.php',
 	'MediaWiki' => 'includes/MediaWiki.php',
-	'MediaWikiI18N' => 'includes/SkinTemplate.php',
 	'MediaWikiVersionFetcher' => 'includes/MediaWikiVersionFetcher.php',
 	'Message' => 'includes/Message.php',
 	'MessageBlobStore' => 'includes/MessageBlobStore.php',
@@ -143,7 +141,6 @@ $wgAutoloadLocalClasses = array(
 	'PreferencesForm' => 'includes/Preferences.php',
 	'PrefixSearch' => 'includes/PrefixSearch.php',
 	'ProtectionForm' => 'includes/ProtectionForm.php',
-	'QuickTemplate' => 'includes/SkinTemplate.php',
 	'RawMessage' => 'includes/Message.php',
 	'ReverseChronologicalPager' => 'includes/Pager.php',
 	'RevisionItem' => 'includes/RevisionList.php',
@@ -156,10 +153,6 @@ $wgAutoloadLocalClasses = array(
 	'SiteConfiguration' => 'includes/SiteConfiguration.php',
 	'SiteStats' => 'includes/SiteStats.php',
 	'SiteStatsInit' => 'includes/SiteStats.php',
-	'Skin' => 'includes/Skin.php',
-	'SkinTemplate' => 'includes/SkinTemplate.php',
-	'SkinFallback' => 'includes/SkinFallback.php',
-	'SkinFallbackTemplate' => 'includes/SkinFallbackTemplate.php',
 	'SquidPurgeClient' => 'includes/SquidPurgeClient.php',
 	'SquidPurgeClientPool' => 'includes/SquidPurgeClient.php',
 	'StatCounter' => 'includes/StatCounter.php',
@@ -933,6 +926,17 @@ $wgAutoloadLocalClasses = array(
 	'Sites' => 'includes/site/SiteSQLStore.php',
 	'SiteStore' => 'includes/site/SiteStore.php',
 
+	# includes/skins
+	'BaseTemplate' => 'includes/skins/SkinTemplate.php',
+	'MediaWikiI18N' => 'includes/skins/SkinTemplate.php',
+	'QuickTemplate' => 'includes/skins/SkinTemplate.php',
+	'Skin' => 'includes/skins/Skin.php',
+	'SkinException' => 'includes/skins/SkinException.php',
+	'SkinFactory' => 'includes/skins/SkinFactory.php',
+	'SkinFallback' => 'includes/skins/SkinFallback.php',
+	'SkinFallbackTemplate' => 'includes/skins/SkinFallbackTemplate.php',
+	'SkinTemplate' => 'includes/skins/SkinTemplate.php',
+
 	# includes/specialpage
 	'ChangesListSpecialPage' => 'includes/specialpage/ChangesListSpecialPage.php',
 	'FormSpecialPage' => 'includes/specialpage/FormSpecialPage.php',
diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php
index b847163086a6..8ab1122e832b 100644
--- a/includes/context/RequestContext.php
+++ b/includes/context/RequestContext.php
@@ -352,12 +352,19 @@ class RequestContext implements IContextSource {
 
 			$skin = null;
 			wfRunHooks( 'RequestContextCreateSkin', array( $this, &$skin ) );
+			$fallback = $this->getConfig()->get( 'FallbackSkin' );
+			$factory = SkinFactory::getDefaultInstance();
 
 			// If the hook worked try to set a skin from it
 			if ( $skin instanceof Skin ) {
 				$this->skin = $skin;
 			} elseif ( is_string( $skin ) ) {
-				$this->skin = Skin::newFromKey( $skin );
+				try {
+					$this->skin = $factory->makeSkin( $skin );
+				} catch ( SkinException $e ) {
+					$this->skin = $factory->makeSkin( $fallback );
+				}
+
 			}
 
 			// If this is still null (the hook didn't run or didn't work)
@@ -372,7 +379,12 @@ class RequestContext implements IContextSource {
 					$userSkin = $this->getConfig()->get( 'DefaultSkin' );
 				}
 
-				$this->skin = Skin::newFromKey( $userSkin );
+				try {
+					$this->skin = $factory->makeSkin( $userSkin );
+				} catch ( SkinException $e ) {
+					$this->skin = $factory->makeSkin( $fallback );
+				}
+
 			}
 
 			// After all that set a context on whatever skin got created
diff --git a/includes/Skin.php b/includes/skins/Skin.php
index a59d5678a3fa..6ba1efd99247 100644
--- a/includes/Skin.php
+++ b/includes/skins/Skin.php
@@ -47,60 +47,7 @@ abstract class Skin extends ContextSource {
 	 * @return array Associative array of strings
 	 */
 	static function getSkinNames() {
-		global $wgValidSkinNames;
-		static $skinsInitialised = false;
-
-		if ( !$skinsInitialised || !count( $wgValidSkinNames ) ) {
-			# Get a list of available skins
-			# Build using the regular expression '^(.*).php$'
-			# Array keys are all lower case, array value keep the case used by filename
-			#
-			wfProfileIn( __METHOD__ . '-init' );
-
-			global $wgStyleDirectory;
-
-			$skinDir = dir( $wgStyleDirectory );
-
-			if ( $skinDir !== false && $skinDir !== null ) {
-				# while code from www.php.net
-				while ( false !== ( $file = $skinDir->read() ) ) {
-					// Skip non-PHP files, hidden files, and '.dep' includes
-					$matches = array();
-
-					if ( preg_match( '/^([^.]*)\.php$/', $file, $matches ) ) {
-						$aSkin = $matches[1];
-
-						// Explicitly disallow loading core skins via the autodiscovery mechanism.
-						//
-						// They should be loaded already (in a non-autodicovery way), but old files might still
-						// exist on the server because our MW version upgrade process is widely documented as
-						// requiring just copying over all files, without removing old ones.
-						//
-						// This is one of the reasons we should have never used autodiscovery in the first
-						// place. This hack can be safely removed when autodiscovery is gone.
-						if ( in_array( $aSkin, array( 'CologneBlue', 'Modern', 'MonoBook', 'Vector' ) ) ) {
-							wfLogWarning(
-								"An old copy of the $aSkin skin was found in your skins/ directory. " .
-								"You should remove it to avoid problems in the future." .
-								"See https://www.mediawiki.org/wiki/Manual:Skin_autodiscovery for details."
-							);
-							continue;
-						}
-
-						wfLogWarning(
-							"A skin using autodiscovery mechanism, $aSkin, was found in your skins/ directory. " .
-							"The mechanism will be removed in MediaWiki 1.25 and the skin will no longer be recognized. " .
-							"See https://www.mediawiki.org/wiki/Manual:Skin_autodiscovery for information how to fix this."
-						);
-						$wgValidSkinNames[strtolower( $aSkin )] = $aSkin;
-					}
-				}
-				$skinDir->close();
-			}
-			$skinsInitialised = true;
-			wfProfileOut( __METHOD__ . '-init' );
-		}
-		return $wgValidSkinNames;
+		return SkinFactory::getDefaultInstance()->getSkinNames();
 	}
 
 	/**
@@ -197,34 +144,20 @@ abstract class Skin extends ContextSource {
 	 * Factory method for loading a skin of a given type
 	 * @param string $key 'monobook', 'vector', etc.
 	 * @return Skin
+	 * @deprecated Use SkinFactory instead
 	 */
 	static function &newFromKey( $key ) {
-		global $wgStyleDirectory, $wgFallbackSkin;
+		wfDeprecated( __METHOD__, '1.24' );
+		global $wgFallbackSkin;
 
 		$key = Skin::normalizeKey( $key );
-
-		$skinNames = Skin::getSkinNames();
-		$skinName = $skinNames[$key];
-		$className = "Skin{$skinName}";
-
-		# Grab the skin class and initialise it.
-		if ( !class_exists( $className ) ) {
-
-			require_once "{$wgStyleDirectory}/{$skinName}.php";
-
-			# Check if we got if not fallback to default skin
-			if ( !class_exists( $className ) ) {
-				# DO NOT die if the class isn't found. This breaks maintenance
-				# scripts and can cause a user account to be unrecoverable
-				# except by SQL manipulation if a previously valid skin name
-				# is no longer valid.
-				wfDebug( "Skin class does not exist: $className\n" );
-
-				$fallback = $skinNames[Skin::normalizeKey( $wgFallbackSkin )];
-				$className = "Skin{$fallback}";
-			}
+		$factory = SkinFactory::getDefaultInstance();
+		try {
+			$skin = $factory->makeSkin( $key );
+		} catch ( SkinException $e ) {
+			$skin = $factory->makeSkin( $wgFallbackSkin );
 		}
-		$skin = new $className( $key );
+
 		return $skin;
 	}
 
diff --git a/includes/skins/SkinException.php b/includes/skins/SkinException.php
new file mode 100644
index 000000000000..31ff1437e850
--- /dev/null
+++ b/includes/skins/SkinException.php
@@ -0,0 +1,29 @@
+<?php
+/**
+ * Copyright 2014
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Exceptions for skin-related failures
+ *
+ * @since 1.24
+ */
+class SkinException extends MWException {
+}
diff --git a/includes/skins/SkinFactory.php b/includes/skins/SkinFactory.php
new file mode 100644
index 000000000000..51aced9c69f6
--- /dev/null
+++ b/includes/skins/SkinFactory.php
@@ -0,0 +1,202 @@
+<?php
+
+/**
+ * Copyright 2014
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Factory class to create Skin objects
+ *
+ * @since 1.24
+ */
+class SkinFactory {
+
+	/**
+	 * Map of name => callback
+	 * @var array
+	 */
+	private $factoryFunctions = array();
+	/**
+	 * Map of name => human readable name
+	 * @var array
+	 */
+	private $displayNames = array();
+
+	/**
+	 * @var SkinFactory
+	 */
+	private static $self;
+
+	public static function getDefaultInstance() {
+		if ( !self::$self ) {
+			self::$self = new self;
+		}
+
+		return self::$self;
+	}
+
+	/**
+	 * Register a new Skin factory function
+	 * Will override if it's already registered
+	 * @param string $name
+	 * @param string $displayName
+	 * @param callable $callback That takes the skin name as an argument
+	 * @throws InvalidArgumentException If an invalid callback is provided
+	 */
+	public function register( $name, $displayName, $callback ) {
+		if ( !is_callable( $callback ) ) {
+			throw new InvalidArgumentException( 'Invalid callback provided' );
+		}
+		$this->factoryFunctions[$name] = $callback;
+		$this->displayNames[$name] = $displayName;
+	}
+
+	/**
+	 * @return array
+	 */
+	private function getLegacySkinNames() {
+		global $wgValidSkinNames;
+		static $skinsInitialised = false;
+
+		if ( !$skinsInitialised || !count( $wgValidSkinNames ) ) {
+			# Get a list of available skins
+			# Build using the regular expression '^(.*).php$'
+			# Array keys are all lower case, array value keep the case used by filename
+			#
+			wfProfileIn( __METHOD__ . '-init' );
+
+			global $wgStyleDirectory;
+
+			$skinDir = dir( $wgStyleDirectory );
+
+			if ( $skinDir !== false && $skinDir !== null ) {
+				# while code from www.php.net
+				while ( false !== ( $file = $skinDir->read() ) ) {
+					// Skip non-PHP files, hidden files, and '.dep' includes
+					$matches = array();
+
+					if ( preg_match( '/^([^.]*)\.php$/', $file, $matches ) ) {
+						$aSkin = $matches[1];
+
+						// Explicitly disallow loading core skins via the autodiscovery mechanism.
+						//
+						// They should be loaded already (in a non-autodicovery way), but old files might still
+						// exist on the server because our MW version upgrade process is widely documented as
+						// requiring just copying over all files, without removing old ones.
+						//
+						// This is one of the reasons we should have never used autodiscovery in the first
+						// place. This hack can be safely removed when autodiscovery is gone.
+						if ( in_array( $aSkin, array( 'CologneBlue', 'Modern', 'MonoBook', 'Vector' ) ) ) {
+							wfLogWarning(
+								"An old copy of the $aSkin skin was found in your skins/ directory. " .
+								"You should remove it to avoid problems in the future." .
+								"See https://www.mediawiki.org/wiki/Manual:Skin_autodiscovery for details."
+							);
+							continue;
+						}
+
+						wfLogWarning(
+							"A skin using autodiscovery mechanism, $aSkin, was found in your skins/ directory. " .
+							"The mechanism will be removed in MediaWiki 1.25 and the skin will no longer be recognized. " .
+							"See https://www.mediawiki.org/wiki/Manual:Skin_autodiscovery for information how to fix this."
+						);
+						$wgValidSkinNames[strtolower( $aSkin )] = $aSkin;
+					}
+				}
+				$skinDir->close();
+			}
+			$skinsInitialised = true;
+			wfProfileOut( __METHOD__ . '-init' );
+		}
+		return $wgValidSkinNames;
+
+	}
+
+	/**
+	 * Returns an associative array of:
+	 *  skin name => human readable name
+	 *
+	 * @return array
+	 */
+	public function getSkinNames() {
+		return array_merge(
+			$this->getLegacySkinNames(),
+			$this->displayNames
+		);
+	}
+
+	/**
+	 * Get a legacy skin which uses $wgValidSkinNames
+	 * or autoloading
+	 *
+	 * @param string $name
+	 * @return Skin|bool false if the skin couldn't be constructed
+	 */
+	private function getLegacySkin( $name ) {
+		$skinNames = $this->getLegacySkinNames();
+		if ( !isset( $skinNames[$name] ) ) {
+			return false;
+		}
+		$skinName = $skinNames[$name];
+		$className = "Skin{$skinName}";
+
+		# Grab the skin class and initialise it.
+		if ( !class_exists( $className ) ) {
+			global $wgStyleDirectory;
+			require_once "{$wgStyleDirectory}/{$skinName}.php";
+
+			# Check if we got it
+			if ( !class_exists( $className ) ) {
+				# DO NOT die if the class isn't found. This breaks maintenance
+				# scripts and can cause a user account to be unrecoverable
+				# except by SQL manipulation if a previously valid skin name
+				# is no longer valid.
+				return false;
+			}
+		}
+		$skin = new $className( $name );
+		return $skin;
+
+	}
+
+	/**
+	 * Create a given Skin using the registered callback for $name.
+	 * @param string $name Name of the skin you want
+	 * @throws SkinException If a factory function isn't registered for $name
+	 * @throws UnexpectedValueException If the factory function returns a non-Skin object
+	 * @return Skin
+	 */
+	public function makeSkin( $name ) {
+		if ( !isset( $this->factoryFunctions[$name] ) ) {
+			// Check the legacy method of skin loading
+			$legacy = $this->getLegacySkin( $name );
+			if ( $legacy ) {
+				return $legacy;
+			}
+			throw new SkinException( "No registered builder available for $name." );
+		}
+		$skin = call_user_func( $this->factoryFunctions[$name], $name );
+		if ( $skin instanceof Skin ) {
+			return $skin;
+		} else {
+			throw new UnexpectedValueException( "The builder for $name returned a non-Skin object." );
+		}
+	}
+}
diff --git a/includes/SkinFallback.php b/includes/skins/SkinFallback.php
index 6055473b440a..6055473b440a 100644
--- a/includes/SkinFallback.php
+++ b/includes/skins/SkinFallback.php
diff --git a/includes/SkinFallbackTemplate.php b/includes/skins/SkinFallbackTemplate.php
index 0703b0eee3a1..0703b0eee3a1 100644
--- a/includes/SkinFallbackTemplate.php
+++ b/includes/skins/SkinFallbackTemplate.php
diff --git a/includes/SkinTemplate.php b/includes/skins/SkinTemplate.php
index 867d8ec605a4..867d8ec605a4 100644
--- a/includes/SkinTemplate.php
+++ b/includes/skins/SkinTemplate.php
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index e866386244a8..8532f5cd2879 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -195,7 +195,7 @@ mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"
 		$method = $class->getMethod( 'makeResourceLoaderLink' );
 		$method->setAccessible( true );
 		$ctx = new RequestContext();
-		$ctx->setSkin( Skin::newFromKey( 'vector' ) );
+		$ctx->setSkin( SkinFactory::getDefaultInstance()->makeSkin( 'fallback' ) );
 		$ctx->setLanguage( 'en' );
 		$out = new OutputPage( $ctx );
 		$rl = $out->getResourceLoader();
diff --git a/tests/phpunit/includes/skins/SkinFactoryTest.php b/tests/phpunit/includes/skins/SkinFactoryTest.php
new file mode 100644
index 000000000000..4497cbabec6e
--- /dev/null
+++ b/tests/phpunit/includes/skins/SkinFactoryTest.php
@@ -0,0 +1,38 @@
+<?php
+
+class SkinFactoryTest extends MediaWikiTestCase {
+
+	/**
+	 * @covers SkinFactory::register
+	 */
+	public function testRegister() {
+		$factory = new SkinFactory();
+		$factory->register( 'fallback', 'Fallback', function() {
+			return new SkinFallback();
+		} );
+		$this->assertTrue( true ); // No exception thrown
+		$this->setExpectedException( 'InvalidArgumentException' );
+		$factory->register( 'invalid', 'Invalid', 'Invalid callback' );
+	}
+
+	/**
+	 * @covers SkinFactory::makeSkin
+	 */
+	public function testMakeSkinWithNoBuilders() {
+		$factory = new SkinFactory();
+		$this->setExpectedException( 'SkinException' );
+		$factory->makeSkin( 'nobuilderregistered' );
+	}
+
+	/**
+	 * @covers SkinFactory::makeSkin
+	 */
+	public function testMakeSkinWithInvalidCallback() {
+		$factory = new SkinFactory();
+		$factory->register( 'unittest', 'Unittest', function () {
+			return true; // Not a Skin object
+		} );
+		$this->setExpectedException( 'UnexpectedValueException' );
+		$factory->makeSkin( 'unittest' );
+	}
+}
diff --git a/tests/phpunit/includes/SkinTemplateTest.php b/tests/phpunit/includes/skins/SkinTemplateTest.php
index baa995d4e653..baa995d4e653 100644
--- a/tests/phpunit/includes/SkinTemplateTest.php
+++ b/tests/phpunit/includes/skins/SkinTemplateTest.php