Skip to content

Commit

Permalink
Remove failed attempt to use non-erasable output buffer
Browse files Browse the repository at this point in the history
On some hosting configurations, the WP object cache is not available in the callback at shutdown.

Also fix some new unit test assertion failures.
  • Loading branch information
westonruter committed Apr 28, 2018
1 parent 056e90a commit 261c1ea
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 35 deletions.
35 changes: 3 additions & 32 deletions includes/class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,7 @@ public static function add_hooks() {
* Start output buffering at very low priority for sake of plugins and themes that use template_redirect
* instead of template_include.
*/
add_action( 'template_redirect', function() {
AMP_Theme_Support::start_output_buffering( array(
'cancelable' => class_exists( 'WP_UnitTestCase' ),
) );
} );
add_action( 'template_redirect', array( __CLASS__, 'start_output_buffering' ), 0 );

// Commenting hooks.
add_filter( 'wp_list_comments_args', array( __CLASS__, 'set_comments_walker' ), PHP_INT_MAX );
Expand Down Expand Up @@ -967,21 +963,8 @@ public static function dequeue_customize_preview_scripts() {
*
* @since 0.7
* @see AMP_Theme_Support::finish_output_buffering()
*
* @param array $args {
* Args.
*
* @type bool $cancelable Whether the output buffer can be flushed/removed/cleaned.
* }
*/
public static function start_output_buffering( $args = array() ) {
$args = array_merge(
array(
'cancelable' => true, // Must be cancelable during unit tests.
),
$args
);

public static function start_output_buffering() {
/*
* Disable the New Relic Browser agent on AMP responses.
* This prevents th New Relic from causing invalid AMP responses due the NREUM script it injects after the meta charset:
Expand All @@ -993,19 +976,7 @@ public static function start_output_buffering( $args = array() ) {
newrelic_disable_autorum();
}

$chunk_size = 0; // Default value.
if ( defined( 'PHP_OUTPUT_HANDLER_STDFLAGS' ) ) {
$erase_flags = $args['cancelable'] ? PHP_OUTPUT_HANDLER_STDFLAGS : 0; // phpcs:ignore PHPCompatibility.PHP.NewConstants.php_output_handler_stdflagsFound
} else {
$erase_flags = ! $args['cancelable'];
}
ob_start( array( __CLASS__, 'finish_output_buffering' ), $chunk_size, $erase_flags );

/*
* Remove shutdown flushing which will cause PHP notice since buffer is not flushable.
* This was only needed in PHP 5.2 anyway, which this plugin does not support.
*/
remove_action( 'shutdown', 'wp_ob_end_flush_all', 1 );
ob_start( array( __CLASS__, 'finish_output_buffering' ) );
}

/**
Expand Down
12 changes: 9 additions & 3 deletions tests/test-class-amp-theme-support.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public function tearDown() {
$_SERVER['QUERY_STRING'] = '';
unset( $_SERVER['REQUEST_URI'] );
unset( $_SERVER['REQUEST_METHOD'] );
unset( $GLOBALS['content_width'] );
if ( isset( $GLOBALS['wp_customize'] ) ) {
$GLOBALS['wp_customize']->stop_previewing_theme();
}
Expand Down Expand Up @@ -605,16 +606,19 @@ public function test_register_widgets() {
* Test register_content_embed_handlers.
*
* @covers AMP_Theme_Support::register_content_embed_handlers()
* @global int $content_width
*/
public function test_register_content_embed_handlers() {
global $content_width;
$content_width = 1234;
$embed_handlers = AMP_Theme_Support::register_content_embed_handlers();
foreach ( $embed_handlers as $embed_handler ) {
$this->assertTrue( is_subclass_of( $embed_handler, 'AMP_Base_Embed_Handler' ) );
$reflection = new ReflectionObject( $embed_handler );
$args = $reflection->getProperty( 'args' );
$args->setAccessible( true );
$property = $args->getValue( $embed_handler );
$this->assertEquals( AMP_Post_Template::CONTENT_MAX_WIDTH, $property['content_max_width'] );
$this->assertEquals( $content_width, $property['content_max_width'] );
}
}

Expand Down Expand Up @@ -892,7 +896,6 @@ function newrelic_disable_autorum() {
$ob_level = ob_get_level();
AMP_Theme_Support::start_output_buffering();

$this->assertEquals( 0, has_action( 'shutdown', array( self::TESTED_CLASS, 'finish_output_buffering' ) ) );
$this->assertTrue( ob_get_level() > $ob_level );

// End output buffer.
Expand Down Expand Up @@ -987,7 +990,10 @@ public function test_prepare_response() {
echo '<!-- wp_print_scripts -->';
} );
add_filter( 'script_loader_tag', function( $tag, $handle ) {
return preg_replace( '/(?<=<script)/', " handle='$handle' ", $tag );
if ( ! wp_scripts()->get_data( $handle, 'conditional' ) ) {
$tag = preg_replace( '/(?<=<script)/', " handle='$handle' ", $tag );
}
return $tag;
}, 10, 2 );
add_action( 'wp_footer', function() {
wp_print_scripts( 'amp-mathml' );
Expand Down

0 comments on commit 261c1ea

Please sign in to comment.