From 0e994a90ffed30844ed82cb077d0c3c942767ee7 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 26 Mar 2018 22:34:36 -0700 Subject: [PATCH] Add initial Server Timing response headers See #990 --- includes/class-amp-autoloader.php | 1 + includes/class-amp-response-headers.php | 88 +++++++++++++++++++ includes/class-amp-theme-support.php | 66 ++++---------- .../templates/class-amp-content-sanitizer.php | 3 + tests/test-class-amp-theme-support.php | 34 +++---- 5 files changed, 128 insertions(+), 64 deletions(-) create mode 100644 includes/class-amp-response-headers.php diff --git a/includes/class-amp-autoloader.php b/includes/class-amp-autoloader.php index b98afe871be..dbc01f6feea 100644 --- a/includes/class-amp-autoloader.php +++ b/includes/class-amp-autoloader.php @@ -30,6 +30,7 @@ class AMP_Autoloader { */ private static $_classmap = array( 'AMP_Theme_Support' => 'includes/class-amp-theme-support', + 'AMP_Response_Headers' => 'includes/class-amp-response-headers', 'AMP_Comment_Walker' => 'includes/class-amp-comment-walker', 'AMP_Template_Customizer' => 'includes/admin/class-amp-customizer', 'AMP_Post_Meta_Box' => 'includes/admin/class-amp-post-meta-box', diff --git a/includes/class-amp-response-headers.php b/includes/class-amp-response-headers.php new file mode 100644 index 00000000000..d8a5e62d635 --- /dev/null +++ b/includes/class-amp-response-headers.php @@ -0,0 +1,88 @@ + true, + 'status_code' => null, + ), + $args + ); + + self::$headers_sent[] = array_merge( compact( 'name', 'value' ), $args ); + if ( headers_sent() ) { + return false; + } + + header( + sprintf( '%s: %s', $name, $value ), + $args['replace'], + $args['status_code'] + ); + return true; + } + + /** + * Send Server-Timing header. + * + * @since 1.0 + * @todo What is the ordering in Chrome dev tools? What are the colors about? + * @todo Is there a better name standardization? + * @todo Is there a way to indicate nested server timings, so an outer method's own time can be seen separately from the inner method's time? + * + * @param string $name Name. + * @param float $duration Duration. If negative, will be added to microtime( true ). Optional. + * @param string $description Description. Optional. + * @return bool Return value of send_header call. + */ + public static function send_server_timing( $name, $duration = null, $description = null ) { + $value = $name; + if ( isset( $description ) ) { + $value .= sprintf( ';desc=%s', wp_json_encode( $description ) ); + } + if ( isset( $duration ) ) { + if ( $duration < 0 ) { + $duration = microtime( true ) + $duration; + } + $value .= sprintf( ';dur=%f', $duration * 1000 ); + } + return self::send_header( 'Server-Timing', $value, array( 'replace' => false ) ); + } +} diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 6373d2e6764..59d08e5a6d4 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -68,22 +68,25 @@ class AMP_Theme_Support { public static $purged_amp_query_vars = array(); /** - * Headers sent (or attempted to be sent). + * Start time when init was called. * - * @since 0.7 - * @see AMP_Theme_Support::send_header() - * @var array[] + * @since 1.0 + * @var float */ - public static $headers_sent = array(); + public static $init_start_time; /** * Initialize. + * + * @since 0.7 */ public static function init() { if ( ! current_theme_supports( 'amp' ) ) { return; } + self::$init_start_time = microtime( true ); + self::purge_amp_query_vars(); self::handle_xhr_request(); self::add_temporary_discussion_restrictions(); @@ -318,45 +321,6 @@ public static function purge_amp_query_vars() { } } - /** - * Send an HTTP response header. - * - * This largely exists to facilitate unit testing but it also provides a better interface for sending headers. - * - * @since 0.7.0 - * - * @param string $name Header name. - * @param string $value Header value. - * @param array $args { - * Args to header(). - * - * @type bool $replace Whether to replace a header previously sent. Default true. - * @type int $status_code Status code to send with the sent header. - * } - * @return bool Whether the header was sent. - */ - public static function send_header( $name, $value, $args = array() ) { - $args = array_merge( - array( - 'replace' => true, - 'status_code' => null, - ), - $args - ); - - self::$headers_sent[] = array_merge( compact( 'name', 'value' ), $args ); - if ( headers_sent() ) { - return false; - } - - header( - sprintf( '%s: %s', $name, $value ), - $args['replace'], - $args['status_code'] - ); - return true; - } - /** * Hook into a POST form submissions, such as the comment form or some other form submission. * @@ -377,7 +341,7 @@ public static function handle_xhr_request() { // Send AMP response header. $origin = wp_validate_redirect( wp_sanitize_redirect( esc_url_raw( self::$purged_amp_query_vars['__amp_source_origin'] ) ) ); if ( $origin ) { - self::send_header( 'AMP-Access-Control-Allow-Source-Origin', $origin, array( 'replace' => true ) ); + AMP_Response_Headers::send_header( 'AMP-Access-Control-Allow-Source-Origin', $origin, array( 'replace' => true ) ); } // Intercept POST requests which redirect. @@ -526,8 +490,8 @@ public static function intercept_post_request_redirect( $location ) { $absolute_location .= '#' . $parsed_location['fragment']; } - self::send_header( 'AMP-Redirect-To', $absolute_location ); - self::send_header( 'Access-Control-Expose-Headers', 'AMP-Redirect-To' ); + AMP_Response_Headers::send_header( 'AMP-Redirect-To', $absolute_location ); + AMP_Response_Headers::send_header( 'Access-Control-Expose-Headers', 'AMP-Redirect-To' ); wp_send_json_success(); } @@ -1000,6 +964,7 @@ public static function start_output_buffering() { * @see AMP_Theme_Support::start_output_buffering() */ public static function finish_output_buffering() { + AMP_Response_Headers::send_server_timing( 'amp_output_buffer', -self::$init_start_time, 'AMP Output Buffer' ); echo self::prepare_response( ob_get_clean() ); // WPCS: xss ok. } @@ -1067,6 +1032,8 @@ public static function prepare_response( $response, $args = array() ) { $args ); + $dom_parse_start = microtime( true ); + /* * Make sure that is present in output prior to parsing. * Note that the meta charset is supposed to appear within the first 1024 bytes. @@ -1098,8 +1065,11 @@ public static function prepare_response( $response, $args = array() ) { $dom->documentElement->setAttribute( 'amp', '' ); } + AMP_Response_Headers::send_server_timing( 'amp_dom_parse', -$dom_parse_start, 'AMP DOM Parse' ); + $assets = AMP_Content_Sanitizer::sanitize_document( $dom, self::$sanitizer_classes, $args ); + $dom_serialize_start = microtime( true ); self::ensure_required_markup( $dom ); // @todo If 'utf-8' is not the blog charset, then we'll need to do some character encoding conversation or "entityification". @@ -1145,6 +1115,8 @@ public static function prepare_response( $response, $args = array() ) { ); } + AMP_Response_Headers::send_server_timing( 'amp_dom_serialize', -$dom_serialize_start, 'AMP DOM Serialize' ); + return $response; } diff --git a/includes/templates/class-amp-content-sanitizer.php b/includes/templates/class-amp-content-sanitizer.php index d7b75c28ec3..7c24ee563e0 100644 --- a/includes/templates/class-amp-content-sanitizer.php +++ b/includes/templates/class-amp-content-sanitizer.php @@ -63,6 +63,7 @@ public static function sanitize_document( &$dom, $sanitizer_classes, $args ) { $return_styles = ! empty( $args['return_styles'] ); unset( $args['return_styles'] ); foreach ( $sanitizer_classes as $sanitizer_class => $sanitizer_args ) { + $sanitize_class_start = microtime( true ); if ( ! class_exists( $sanitizer_class ) ) { /* translators: %s is sanitizer class */ _doing_it_wrong( __METHOD__, sprintf( esc_html__( 'Sanitizer (%s) class does not exist', 'amp' ), esc_html( $sanitizer_class ) ), '0.4.1' ); @@ -90,6 +91,8 @@ public static function sanitize_document( &$dom, $sanitizer_classes, $args ) { } else { $stylesheets = array_merge( $stylesheets, $sanitizer->get_stylesheets() ); } + + AMP_Response_Headers::send_server_timing( 'amp_sanitize', -$sanitize_class_start, $sanitizer_class ); } return compact( 'scripts', 'styles', 'stylesheets' ); diff --git a/tests/test-class-amp-theme-support.php b/tests/test-class-amp-theme-support.php index 439807a584d..a4cb1762865 100644 --- a/tests/test-class-amp-theme-support.php +++ b/tests/test-class-amp-theme-support.php @@ -27,7 +27,7 @@ public function tearDown() { $_SERVER['QUERY_STRING'] = ''; unset( $_SERVER['REQUEST_URI'] ); unset( $_SERVER['REQUEST_METHOD'] ); - AMP_Theme_Support::$headers_sent = array(); + AMP_Response_Headers::$headers_sent = array(); } /** @@ -349,7 +349,7 @@ public function test_purge_amp_query_vars() { public function test_handle_xhr_request() { AMP_Theme_Support::purge_amp_query_vars(); AMP_Theme_Support::handle_xhr_request(); - $this->assertEmpty( AMP_Theme_Support::$headers_sent ); + $this->assertEmpty( AMP_Response_Headers::$headers_sent ); $_GET['_wp_amp_action_xhr_converted'] = '1'; @@ -358,14 +358,14 @@ public function test_handle_xhr_request() { $_SERVER['REQUEST_METHOD'] = 'POST'; AMP_Theme_Support::purge_amp_query_vars(); AMP_Theme_Support::handle_xhr_request(); - $this->assertEmpty( AMP_Theme_Support::$headers_sent ); + $this->assertEmpty( AMP_Response_Headers::$headers_sent ); // Try home source origin. $_GET['__amp_source_origin'] = home_url(); $_SERVER['REQUEST_METHOD'] = 'POST'; AMP_Theme_Support::purge_amp_query_vars(); AMP_Theme_Support::handle_xhr_request(); - $this->assertCount( 1, AMP_Theme_Support::$headers_sent ); + $this->assertCount( 1, AMP_Response_Headers::$headers_sent ); $this->assertEquals( array( 'name' => 'AMP-Access-Control-Allow-Source-Origin', @@ -373,7 +373,7 @@ public function test_handle_xhr_request() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent[0] + AMP_Response_Headers::$headers_sent[0] ); $this->assertEquals( PHP_INT_MAX, has_filter( 'wp_redirect', array( 'AMP_Theme_Support', 'intercept_post_request_redirect' ) ) ); $this->assertEquals( PHP_INT_MAX, has_filter( 'comment_post_redirect', array( 'AMP_Theme_Support', 'filter_comment_post_redirect' ) ) ); @@ -476,7 +476,7 @@ public function test_intercept_post_request_redirect() { } ); // Test redirecting to full URL with HTTPS protocol. - AMP_Theme_Support::$headers_sent = array(); + AMP_Response_Headers::$headers_sent = array(); ob_start(); AMP_Theme_Support::intercept_post_request_redirect( $url ); $this->assertEquals( '{"success":true}', ob_get_clean() ); @@ -487,7 +487,7 @@ public function test_intercept_post_request_redirect() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent + AMP_Response_Headers::$headers_sent ); $this->assertContains( array( @@ -496,11 +496,11 @@ public function test_intercept_post_request_redirect() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent + AMP_Response_Headers::$headers_sent ); // Test redirecting to non-HTTPS URL. - AMP_Theme_Support::$headers_sent = array(); + AMP_Response_Headers::$headers_sent = array(); ob_start(); $url = home_url( '/', 'http' ); AMP_Theme_Support::intercept_post_request_redirect( $url ); @@ -512,7 +512,7 @@ public function test_intercept_post_request_redirect() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent + AMP_Response_Headers::$headers_sent ); $this->assertContains( array( @@ -521,11 +521,11 @@ public function test_intercept_post_request_redirect() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent + AMP_Response_Headers::$headers_sent ); // Test redirecting to host-less location. - AMP_Theme_Support::$headers_sent = array(); + AMP_Response_Headers::$headers_sent = array(); ob_start(); AMP_Theme_Support::intercept_post_request_redirect( '/new-location/' ); $this->assertEquals( '{"success":true}', ob_get_clean() ); @@ -536,11 +536,11 @@ public function test_intercept_post_request_redirect() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent + AMP_Response_Headers::$headers_sent ); // Test redirecting to scheme-less location. - AMP_Theme_Support::$headers_sent = array(); + AMP_Response_Headers::$headers_sent = array(); ob_start(); $url = home_url( '/new-location/' ); AMP_Theme_Support::intercept_post_request_redirect( substr( $url, strpos( $url, ':' ) + 1 ) ); @@ -552,11 +552,11 @@ public function test_intercept_post_request_redirect() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent + AMP_Response_Headers::$headers_sent ); // Test redirecting to empty location. - AMP_Theme_Support::$headers_sent = array(); + AMP_Response_Headers::$headers_sent = array(); ob_start(); AMP_Theme_Support::intercept_post_request_redirect( '' ); $this->assertEquals( '{"success":true}', ob_get_clean() ); @@ -567,7 +567,7 @@ public function test_intercept_post_request_redirect() { 'replace' => true, 'status_code' => null, ), - AMP_Theme_Support::$headers_sent + AMP_Response_Headers::$headers_sent ); }