From 54f9e1f159ae747f81fc7a5886da77fde8025adf Mon Sep 17 00:00:00 2001
From: Ori Livneh <ori@wikimedia.org>
Date: Thu, 10 Dec 2015 14:58:11 -0800
Subject: [PATCH] Timing::measure(): handle missing marks better

Currently Timing::measure() does not check that the requested start and end
marks exist, causing it to return bogus values without any indication that
something has gone wrong. Fix this by logging and error and returning false
in case either the start or end markers do not exist. To make it possible to
log, make Timing implement the LoggerAware interface.

Change-Id: I75af5273e9a8a52b31d0af1de206b0d8a4c82fbc
---
 includes/context/RequestContext.php |  6 +++++-
 includes/libs/Timing.php            | 33 ++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/includes/context/RequestContext.php b/includes/context/RequestContext.php
index 4f8e65d01b..a0073aaa8c 100644
--- a/includes/context/RequestContext.php
+++ b/includes/context/RequestContext.php
@@ -22,6 +22,8 @@
  * @file
  */
 
+use MediaWiki\Logger\LoggerFactory;
+
 /**
  * Group all the pieces relevant to the context of a request into one instance
  */
@@ -151,7 +153,9 @@ class RequestContext implements IContextSource, MutableContext {
 	 */
 	public function getTiming() {
 		if ( $this->timing === null ) {
-			$this->timing = new Timing();
+			$this->timing = new Timing( array(
+				'logger' => LoggerFactory::getInstance( 'Timing' )
+			) );
 		}
 		return $this->timing;
 	}
diff --git a/includes/libs/Timing.php b/includes/libs/Timing.php
index 653227e1b4..00ceda3e78 100644
--- a/includes/libs/Timing.php
+++ b/includes/libs/Timing.php
@@ -18,6 +18,10 @@
  * @file
  */
 
+use Psr\Log\LoggerAwareInterface;
+use Psr\Log\LoggerInterface;
+use Psr\Log\NullLogger;
+
 /**
  * An interface to help developers measure the performance of their applications.
  * This interface closely matches the W3C's User Timing specification.
@@ -38,13 +42,27 @@
  *
  * @since 1.27
  */
-class Timing {
+class Timing implements LoggerAwareInterface {
 
 	/** @var array[] */
 	private $entries = array();
 
-	public function __construct() {
+	/** @var LoggerInterface */
+	protected $logger;
+
+	public function __construct( array $params = array() ) {
 		$this->clearMarks();
+		$this->setLogger( isset( $params['logger'] ) ? $params['logger'] : new NullLogger() );
+	}
+
+	/**
+	 * Sets a logger instance on the object.
+	 *
+	 * @param LoggerInterface $logger
+	 * @return null
+	 */
+	public function setLogger( LoggerInterface $logger ) {
+		$this->logger = $logger;
 	}
 
 	/**
@@ -102,14 +120,23 @@ class Timing {
 	 * @param string $measureName
 	 * @param string $startMark
 	 * @param string $endMark
-	 * @return array The measure that has been created.
+	 * @return array|bool The measure that has been created, or false if either
+	 *  the start mark or the end mark do not exist.
 	 */
 	public function measure( $measureName, $startMark = 'requestStart', $endMark = null ) {
 		$start = $this->getEntryByName( $startMark );
+		if ( $start === null ) {
+			$this->logger->error( __METHOD__ . ": The mark '$startMark' does not exist" );
+			return false;
+		}
 		$startTime = $start['startTime'];
 
 		if ( $endMark ) {
 			$end = $this->getEntryByName( $endMark );
+			if ( $end === null ) {
+				$this->logger->error( __METHOD__ . ": The mark '$endMark' does not exist" );
+				return false;
+			}
 			$endTime = $end['startTime'];
 		} else {
 			$endTime = microtime( true );
-- 
2.20.1