From ba40a63c0ee23daec4886a3e7da62b061639f19c Mon Sep 17 00:00:00 2001
From: Florian <florian.schmidt.stargatewissen@gmail.com>
Date: Sat, 2 Apr 2016 16:39:50 +0200
Subject: [PATCH] Don't construct SpecialPages twice

If the special page object was already created for the request, there's
no need to create the object again. Save the created result (object, null)
in a global static array and return the value if the realName was already
created.

Bug: T123995
Change-Id: I70bf0e93e45f4b0597deaef717f5eb87c66f0a71
---
 includes/specialpage/SpecialPageFactory.php    |  7 +++++++
 .../specialpage/SpecialPageFactoryTest.php     | 18 ++++++++----------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/includes/specialpage/SpecialPageFactory.php b/includes/specialpage/SpecialPageFactory.php
index 8ce480e123..725c4fc581 100644
--- a/includes/specialpage/SpecialPageFactory.php
+++ b/includes/specialpage/SpecialPageFactory.php
@@ -182,6 +182,7 @@ class SpecialPageFactory {
 
 	private static $list;
 	private static $aliases;
+	private static $pageObjectCache = [];
 
 	/**
 	 * Reset the internal list of special pages. Useful when changing $wgSpecialPages after
@@ -190,6 +191,7 @@ class SpecialPageFactory {
 	public static function resetList() {
 		self::$list = null;
 		self::$aliases = null;
+		self::$pageObjectCache = [];
 	}
 
 	/**
@@ -373,6 +375,10 @@ class SpecialPageFactory {
 	public static function getPage( $name ) {
 		list( $realName, /*...*/ ) = self::resolveAlias( $name );
 
+		if ( isset( self::$pageObjectCache[$realName] ) ) {
+			return self::$pageObjectCache[$realName];
+		}
+
 		$specialPageList = self::getPageList();
 
 		if ( isset( $specialPageList[$realName] ) ) {
@@ -400,6 +406,7 @@ class SpecialPageFactory {
 				$page = null;
 			}
 
+			self::$pageObjectCache[$realName] = $page;
 			if ( $page instanceof SpecialPage ) {
 				return $page;
 			} else {
diff --git a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
index 534cf9baf7..3d407fbc32 100644
--- a/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
+++ b/tests/phpunit/includes/specialpage/SpecialPageFactoryTest.php
@@ -55,19 +55,17 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
 		$specialPageTestHelper = new SpecialPageTestHelper();
 
 		return [
-			'class name' => [ 'SpecialAllPages', false ],
+			'class name' => [ 'SpecialAllPages' ],
 			'closure' => [ function () {
 				return new SpecialAllPages();
-			}, false ],
-			'function' => [ [ $this, 'newSpecialAllPages' ], false ],
-			'callback string' => [ 'SpecialPageTestHelper::newSpecialAllPages', false ],
+			} ],
+			'function' => [ [ $this, 'newSpecialAllPages' ] ],
+			'callback string' => [ 'SpecialPageTestHelper::newSpecialAllPages' ],
 			'callback with object' => [
-				[ $specialPageTestHelper, 'newSpecialAllPages' ],
-				false
+				[ $specialPageTestHelper, 'newSpecialAllPages' ]
 			],
 			'callback array' => [
-				[ 'SpecialPageTestHelper', 'newSpecialAllPages' ],
-				false
+				[ 'SpecialPageTestHelper', 'newSpecialAllPages' ]
 			]
 		];
 	}
@@ -76,7 +74,7 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
 	 * @covers SpecialPageFactory::getPage
 	 * @dataProvider specialPageProvider
 	 */
-	public function testGetPage( $spec, $shouldReuseInstance ) {
+	public function testGetPage( $spec ) {
 		$this->mergeMwGlobalArrayValue( 'wgSpecialPages', [ 'testdummy' => $spec ] );
 		SpecialPageFactory::resetList();
 
@@ -84,7 +82,7 @@ class SpecialPageFactoryTest extends MediaWikiTestCase {
 		$this->assertInstanceOf( 'SpecialPage', $page );
 
 		$page2 = SpecialPageFactory::getPage( 'testdummy' );
-		$this->assertEquals( $shouldReuseInstance, $page2 === $page, "Should re-use instance:" );
+		$this->assertEquals( true, $page2 === $page, "Should re-use instance:" );
 	}
 
 	/**
-- 
2.20.1