Only apply output buffer filter when in final phase

[westonruter]

As suggested by @nextend in a follow-up to #61212:

We use output buffering in our 900k+ plugin and based on my experience I suggest the following changes:

<?php
function _gutenberg_buffer_template_output( string $passthrough ): string {
	ob_start(
		static function ( string $output, ?int $phase): string {

			if ($phase & PHP_OUTPUT_HANDLER_FINAL) {
				/**
			 	* Filters the template output buffer prior to sending to the client.
			 	*
			 	* @param string $output Output buffer.
				 * @return string Filtered output buffer.
				 */
				return (string) apply_filters( 'gutenberg_template_output_buffer', $output );
			}

			return $output;
		}
	);
	return $passthrough;
}

As I remember these are related when ob_clean and ob_flush called during the output buffer is open. It skips the processing of the output when its dropped.

[westonruter]

@nextend I'm having a hard time triggering a scenario in which PHP_OUTPUT_HANDLER_FINAL is not passed. I'm marking this as a draft so this doesn't get merged prior to validating the need for it.

[westonruter]

I tried adding a test case in WordPress/performance#1317 but I'm not having success.

[nextend]

Here is an example that can trigger the $phase argument to not be PHP_OUTPUT_HANDLER_FINAL:

https://bitbucket.org/rolandsoos/workspace/snippets/aq7bnz

The content of the first echo will be dropped by the ob_clean() call. The current implementation in #60280 would apply the filter for this dropped message.

[westonruter]

@nextend Thanks for providing that test case. Sorry for the delay in responding. To be honest I've been having a hard time wrapping my brain around this. I've iterated on the corresponding PR in the WordPress/performance repo and I'm getting closer but I'm still perplexed why it isn't behaving as expected. Could you take a look? https://github.com/WordPress/performance/pull/1317/files

[nextend]

@westonruter I think the missing piece is to call ob_end_flush(); before calling ob_get_clean();.

On line 91, a new output buffer is created. This output buffer is necessary as it allows you to access the output in the same way the PHP runtime does when outputting the default output buffer.

In the original code, the following happened:

• System [1] output buffer <- This will be sent to the browser.
  • Line 91 -> [2] output buffer
    • od_buffer_output -> [3] output buffer
    • This part is tricky: Calling ob_get_contents() at this point does not trigger the ob callback, as there might be more text to add to the buffer after calling ob_get_contents. Example: https://gist.github.com/nextend/ac5c90de0e2d628fba02355819998a34
    • If you check the source code of PHP, ob_get_clean has two parts: first, it calls ob_get_contents, and then it discards the output buffer. This is where a common misunderstanding occurs. In the first example, I showed why ob_get_contents does not trigger the ob callback, and since it is used inside ob_get_clean, there will be no ob callback call. Reference: https://github.com/php/php-src/blob/48916cec3383e5ac83bd9486d5445f847a36b922/main/output.c#L1408

In the new code:

• System [1] output buffer <- This will be sent to the browser.
  • Line 91 -> [2] output buffer
    • od_buffer_output -> [3] output buffer
    • ob_end_flush -> Output buffer [3] is closed -> ob callback gets called -> Result of the callback is flushed into [2] buffer
  • Now we can access the real output with ob_get_clean, which gives us the content of the [2] buffer (and then discards the output).

[westonruter]

@nextend Thank you. I iterated on it a bit more with your change applied and I think it's in a good shape: WordPress/performance#1317