Ensure services that are delayed and have requirements are scheduled correctly

src/Infrastructure/ServiceBasedPlugin.php

@@ -200,26 +200,7 @@ public function register_services() {
 				continue;
 			}
 
-			// Allow the services to delay their registration.
-			if ( is_a( $class, Delayed::class, true ) ) {
-				$registration_action = $class::get_registration_action();
-
-				if ( \did_action( $registration_action ) ) {
-					$this->register_service( $id, $class );
-				} else {
-					\add_action(
-						$registration_action,
-						function () use ( $id, $class ) {
-							$this->register_service( $id, $class );
-						}
-					);
-				}
-
-				next( $services );
-				continue;
-			}
-
-			$this->register_service( $id, $class );
+			$this->schedule_potential_service_registration( $id, $class );
 
 			next( $services );
 		}
@@ -273,7 +254,7 @@ function () use ( $id, $class, $services ) {
 							return;
 						}
 
-						$this->register_service( $id, $class );
+						$this->schedule_potential_service_registration( $id, $class );
 					},
 					PHP_INT_MAX
 				);
@@ -389,12 +370,44 @@ protected function get_identifier_from_fqcn( $fqcn ) {
 	}
 
 	/**
-	 * Register a single service.
+	 * Schedule the potential registration of a single service.
+	 *
+	 * This takes into account whether the service registration needs to be delayed or not.
+	 *
+	 * @param string $id ID of the service to register.
+	 * @param string $class Class of the service to register.
+	 */
+	protected function schedule_potential_service_registration( $id, $class ) {
+		if ( is_a( $class, Delayed::class, true ) ) {
+			$registration_action = $class::get_registration_action();
+
+			if ( \did_action( $registration_action ) ) {
+				$this->maybe_register_service( $id, $class );
+			} else {
+				\add_action(
+					$registration_action,
+					function () use ( $id, $class ) {
+						$this->maybe_register_service( $id, $class );
+					}
+				);
+			}
+		} else {
+			$this->maybe_register_service( $id, $class );
+		}
+	}
+
+	/**
+	 * Register a single service, provided its conditions are met.
 	 *
 	 * @param string $id ID of the service to register.
 	 * @param string $class Class of the service to register.
 	 */
-	protected function register_service( $id, $class ) {
+	protected function maybe_register_service( $id, $class ) {
+		// Ensure we don't register the same service more than once.
+		if ( $this->service_container->has( $id ) ) {
+			return;
+		}
+
 		// Only instantiate services that are actually needed.
 		if ( is_a( $class, Conditional::class, true )
 			&& ! $class::is_needed() ) {

tests/php/src/Admin/PolyfillsTest.php

@@ -14,8 +14,6 @@
 use AmpProject\AmpWP\Infrastructure\Registerable;
 use AmpProject\AmpWP\Infrastructure\Service;
 use AmpProject\AmpWP\Tests\TestCase;
-use WP_Scripts;
-use WP_Styles;
 
 /**
  * Tests for Polyfills class.
@@ -34,16 +32,32 @@ class PolyfillsTest extends TestCase {
 	private $instance;
 
 	/**
-	 * Setup.
+	 * Set up.
 	 *
 	 * @inheritdoc
 	 */
 	public function setUp() {
 		parent::setUp();
 
+		global $wp_scripts, $wp_styles;
+		$wp_scripts = null;
+		$wp_styles  = null;
+
 		$this->instance = new Polyfills();
 	}
 
+	/**
+	 * Tear down.
+	 *
+	 * @inheritdoc
+	 */
+	public function tearDown() {
+		parent::tearDown();
+		global $wp_scripts, $wp_styles;
+		$wp_scripts = null;
+		$wp_styles  = null;
+	}
+
 	public function test__construct() {
 		$this->assertInstanceOf( Polyfills::class, $this->instance );
 		$this->assertInstanceOf( Service::class, $this->instance );
@@ -66,22 +80,23 @@ public function test_get_requirements() {
 	 * @covers ::register_shimmed_styles
 	 */
 	public function test_registration() {
-		global $wp_scripts, $wp_styles;
-		$this->instance->register();
+		if ( function_exists( 'is_gutenberg_page' ) ) {
+			$this->assertFalse( is_gutenberg_page() );
+		}
+		if ( function_exists( 'get_current_screen' ) ) {
+			$screen = get_current_screen();
+			$this->assertTrue( empty( $screen->is_block_editor ) );
+		}
 
-		$wp_scripts = new WP_Scripts();
-		$wp_styles  = new WP_Styles();
-
-		/** This action is documented in includes/class-amp-theme-support.php */
-		do_action( 'amp_register_polyfills' );
+		$this->instance->register();
 
 		// These should pass in WP < 5.6.
 		$this->assertTrue( wp_script_is( 'lodash', 'registered' ) );
-		$this->assertStringContainsString( '_.noConflict();', $wp_scripts->print_inline_script( 'lodash', 'after', false ) );
+		$this->assertStringContainsString( '_.noConflict();', wp_scripts()->print_inline_script( 'lodash', 'after', false ) );
 
 		$this->assertTrue( wp_script_is( 'wp-api-fetch', 'registered' ) );
-		$this->assertStringContainsString( 'createRootURLMiddleware', $wp_scripts->print_inline_script( 'wp-api-fetch', 'after', false ) );
-		$this->assertStringContainsString( 'createNonceMiddleware', $wp_scripts->print_inline_script( 'wp-api-fetch', 'after', false ) );
+		$this->assertStringContainsString( 'createRootURLMiddleware', wp_scripts()->print_inline_script( 'wp-api-fetch', 'after', false ) );
+		$this->assertStringContainsString( 'createNonceMiddleware', wp_scripts()->print_inline_script( 'wp-api-fetch', 'after', false ) );
 
 		$this->assertTrue( wp_script_is( 'wp-hooks', 'registered' ) );
 		$this->assertTrue( wp_script_is( 'wp-i18n', 'registered' ) );
@@ -90,7 +105,5 @@ public function test_registration() {
 		$this->assertTrue( wp_script_is( 'wp-url', 'registered' ) );
 
 		$this->assertTrue( wp_style_is( 'wp-components', 'registered' ) );
-
-		unset( $wp_scripts, $wp_styles );
 	}
 }

tests/php/src/MobileRedirectionTest.php

@@ -692,7 +692,7 @@ private function register_paired_browsing_service() {
 		add_filter( 'amp_dev_mode_enabled', '__return_true' );
 		$this->assertTrue( call_user_func( [ $service_class, 'is_needed' ] ) );
 
-		$this->call_private_method( $this->plugin, 'register_service', [ $service_id, $service_classes[ $service_id ] ] );
+		$this->call_private_method( $this->plugin, 'maybe_register_service', [ $service_id, $service_classes[ $service_id ] ] );
 		$this->assertTrue( $this->container->has( $service_id ) );
 	}