profiler: Centralise output responsibility from ProfilerOutputText to Profiler

Make it Profiler.php's responsibility to enforce this, based on the
existing signal from ProfilerOutput::logsToOutput().

The ProfilerOutputText class should not have to double-check this
a second time.

Long-term, I'd like even this check in Profiler::logDataPageOutputOnly
to be removed, because really the external caller of that should
know whether it is safe to output stuff or not rather than stashing
its own state inside Profiler::$allowOutput and then implicitly
reading it back out again later on. But, that's for another time.

Also:
* Remove use of deprecated Profiler::setTemplated while at it.
* Make 'visible' parameter explicit, as for other parameters.

Change-Id: Iaa3fc4ea25a059b90235d769db60c04b8f152f05

diff --git a/includes/profiler/Profiler.php b/includes/profiler/Profiler.php
index 0fcc97d..2eb5586 100644 (file)
--- a/includes/profiler/Profiler.php
+++ b/includes/profiler/Profiler.php
@@ -249,6 +249,10 @@ abstract class Profiler {
         * @since 1.26
         */
        public function logDataPageOutputOnly() {
+               if ( !$this->allowOutput ) {
+                       return;
+               }
+
                $outputs = [];
                foreach ( $this->getOutputs() as $output ) {
                        if ( $output->logsToOutput() ) {
@@ -291,7 +295,7 @@ abstract class Profiler {
         * @param bool $t
         */
        public function setTemplated( $t ) {
-               // wfDeprecated( __METHOD__, '1.34' );
+               wfDeprecated( __METHOD__, '1.34' );
                $this->allowOutput = ( $t === true );
        }
 
@@ -302,7 +306,7 @@ abstract class Profiler {
         * @return bool
         */
        public function getTemplated() {
-               // wfDeprecated( __METHOD__, '1.34' );
+               wfDeprecated( __METHOD__, '1.34' );
                return $this->getAllowOutput();
        }
 

diff --git a/includes/profiler/output/ProfilerOutput.php b/includes/profiler/output/ProfilerOutput.php
index fe27c04..bc14f4b 100644 (file)
--- a/includes/profiler/output/ProfilerOutput.php
+++ b/includes/profiler/output/ProfilerOutput.php
@@ -48,7 +48,7 @@ abstract class ProfilerOutput {
        }
 
        /**
-        * Does log() just send the data to the request/script output?
+        * May the log() try to write to standard output?
         * @return bool
         * @since 1.33
         */
@@ -57,7 +57,10 @@ abstract class ProfilerOutput {
        }
 
        /**
-        * Log MediaWiki-style profiling data
+        * Log MediaWiki-style profiling data.
+        *
+        * For classes that enable logsToOutput(), this must not
+        * be called unless Profiler::setAllowOutput is enabled.
         *
         * @param array $stats Result of Profiler::getFunctionStats()
         */

diff --git a/includes/profiler/output/ProfilerOutputText.php b/includes/profiler/output/ProfilerOutputText.php
index 95b5ff9..ee06b59 100644 (file)
--- a/includes/profiler/output/ProfilerOutputText.php
+++ b/includes/profiler/output/ProfilerOutputText.php
@@ -29,11 +29,15 @@
  */
 class ProfilerOutputText extends ProfilerOutput {
        /** @var float Min real time display threshold */
-       protected $thresholdMs;
+       private $thresholdMs;
+
+       /** @var bool Whether to use visible text or a comment (for HTML responses) */
+       private $visible;
 
        function __construct( Profiler $collector, array $params ) {
                parent::__construct( $collector, $params );
                $this->thresholdMs = $params['thresholdMs'] ?? 1.0;
+               $this->visible = $params['visible'] ?? false;
        }
 
        public function logsToOutput() {
@@ -41,39 +45,36 @@ class ProfilerOutputText extends ProfilerOutput {
        }
 
        public function log( array $stats ) {
-               if ( $this->collector->getTemplated() ) {
-                       $out = '';
+               $out = '';
 
-                       // Filter out really tiny entries
-                       $min = $this->thresholdMs;
-                       $stats = array_filter( $stats, function ( $a ) use ( $min ) {
-                               return $a['real'] > $min;
-                       } );
-                       // Sort descending by time elapsed
-                       usort( $stats, function ( $a, $b ) {
-                               return $b['real'] <=> $a['real'];
-                       } );
+               // Filter out really tiny entries
+               $min = $this->thresholdMs;
+               $stats = array_filter( $stats, function ( $a ) use ( $min ) {
+                       return $a['real'] > $min;
+               } );
+               // Sort descending by time elapsed
+               usort( $stats, function ( $a, $b ) {
+                       return $b['real'] <=> $a['real'];
+               } );
 
-                       array_walk( $stats,
-                               function ( $item ) use ( &$out ) {
-                                       $out .= sprintf( "%6.2f%% %3.3f %6d - %s\n",
-                                               $item['%real'], $item['real'], $item['calls'], $item['name'] );
-                               }
-                       );
+               array_walk( $stats,
+                       function ( $item ) use ( &$out ) {
+                               $out .= sprintf( "%6.2f%% %3.3f %6d - %s\n",
+                                       $item['%real'], $item['real'], $item['calls'], $item['name'] );
+                       }
+               );
 
-                       $contentType = $this->collector->getContentType();
-                       if ( wfIsCLI() ) {
+               $contentType = $this->collector->getContentType();
+               if ( wfIsCLI() ) {
+                       print "<!--\n{$out}\n-->\n";
+               } elseif ( $contentType === 'text/html' ) {
+                       if ( $this->visible ) {
+                               print "<pre>{$out}</pre>";
+                       } else {
                                print "<!--\n{$out}\n-->\n";
-                       } elseif ( $contentType === 'text/html' ) {
-                               $visible = $this->params['visible'] ?? false;
-                               if ( $visible ) {
-                                       print "<pre>{$out}</pre>";
-                               } else {
-                                       print "<!--\n{$out}\n-->\n";
-                               }
-                       } elseif ( $contentType === 'text/javascript' || $contentType === 'text/css' ) {
-                               print "\n/*\n{$out}*/\n";
                        }
+               } elseif ( $contentType === 'text/javascript' || $contentType === 'text/css' ) {
+                       print "\n/*\n{$out}*/\n";
                }
        }
 }

diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php
index f348135..aeca016 100644 (file)
--- a/includes/skins/SkinTemplate.php
+++ b/includes/skins/SkinTemplate.php
@@ -208,7 +208,7 @@ class SkinTemplate extends Skin {
         * Initialize various variables and generate the template
         */
        function outputPage() {
-               Profiler::instance()->setTemplated( true );
+               Profiler::instance()->setAllowOutput();
                $out = $this->getOutput();
 
                $this->initPage( $out );

diff --git a/load.php b/load.php
index 4d34e5d..6edb7ec 100644 (file)
--- a/load.php
+++ b/load.php
@@ -45,7 +45,7 @@ $context = new ResourceLoaderContext( $resourceLoader, $wgRequest );
 // Respond to ResourceLoader request
 $resourceLoader->respond( $context );
 
-Profiler::instance()->setTemplated( true );
+Profiler::instance()->setAllowOutput();
 
 $mediawiki = new MediaWiki();
 $mediawiki->doPostOutputShutdown( 'fast' );

diff --git a/maintenance/Maintenance.php b/maintenance/Maintenance.php
index c3644ee..130d1fb 100644 (file)
--- a/maintenance/Maintenance.php
+++ b/maintenance/Maintenance.php
@@ -831,7 +831,7 @@ abstract class Maintenance {
                                        + $wgProfiler
                                        + [ 'threshold' => $wgProfileLimit ]
                        );
-                       $profiler->setTemplated( true );
+                       $profiler->setAllowOutput();
                        Profiler::replaceStubInstance( $profiler );
                }