Issue #2939208 by alexpott, tim.plunkett: Services registered with...

Issue #2939208 by alexpott, tim.plunkett: Services registered with ContainerBuilder::setDefinition() are removed from container causing a BC break

core/lib/Drupal/Core/DependencyInjection/ContainerBuilder.php

@@ -88,10 +88,7 @@ public function register($id, $class = null) {
     if (strtolower($id) !== $id) {
       throw new \InvalidArgumentException("Service ID names must be lowercase: $id");
     }
-    $definition = parent::register($id, $class);
-    // As of Symfony 3.4 all services are private by default.
-    $definition->setPublic(TRUE);
-    return $definition;
+    return parent::register($id, $class);
   }
 
   /**
@@ -104,6 +101,22 @@ public function setAlias($alias, $id) {
     return $alias;
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function setDefinition($id, Definition $definition) {
+    $definition = parent::setDefinition($id, $definition);
+    // As of Symfony 3.4 all definitions are private by default.
+    // \Symfony\Component\DependencyInjection\Compiler\ResolvePrivatesPassOnly
+    // removes services marked as private from the container even if they are
+    // also marked as public. Drupal requires services that are public to
+    // remain in the container and not be removed.
+    if ($definition->isPublic()) {
+      $definition->setPrivate(FALSE);
+    }
+    return $definition;
+  }
+
   /**
    * {@inheritdoc}
    */

core/lib/Drupal/Core/DependencyInjection/YamlFileLoader.php

@@ -158,8 +158,6 @@ private function parseDefinition($id, $service, $file)
             $definition = new ChildDefinition($service['parent']);
         } else {
             $definition = new Definition();
-            // As of Symfony 3.4 all services are private by default.
-            $definition->setPublic(TRUE);
         }
 
         if (isset($service['class'])) {

core/tests/Drupal/KernelTests/KernelTestBaseTest.php

@@ -137,6 +137,10 @@ public function testRegister() {
     $this->assertInstanceOf('Symfony\Component\HttpFoundation\Request', $new_request);
     $this->assertSame($new_request, \Drupal::request());
     $this->assertSame($request, $new_request);
+
+    // Ensure getting the router.route_provider does not trigger a deprecation
+    // message that errors.
+    $this->container->get('router.route_provider');
   }
 
   /**

core/tests/Drupal/Tests/Core/DependencyInjection/ContainerBuilderTest.php

@@ -5,6 +5,7 @@
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Tests\UnitTestCase;
 use Drupal\Tests\Core\DependencyInjection\Fixture\BarClass;
+use Symfony\Component\DependencyInjection\Definition;
 
 /**
  * @coversDefaultClass \Drupal\Core\DependencyInjection\ContainerBuilder
@@ -70,6 +71,34 @@ public function testRegister() {
     $this->assertTrue($service->isPublic());
   }
 
+  /**
+   * @covers ::setDefinition
+   */
+  public function testSetDefinition() {
+    // Test a service with defaults.
+    $container = new ContainerBuilder();
+    $definition = new Definition();
+    $service = $container->setDefinition('foo', $definition);
+    $this->assertTrue($service->isPublic());
+    $this->assertFalse($service->isPrivate());
+
+    // Test a service with public set to false.
+    $definition = new Definition();
+    $definition->setPublic(FALSE);
+    $service = $container->setDefinition('foo', $definition);
+    $this->assertFalse($service->isPublic());
+    $this->assertFalse($service->isPrivate());
+
+    // Test a service with private set to true. Drupal does not support this.
+    // We only support using setPublic() to make things not available outside
+    // the container.
+    $definition = new Definition();
+    $definition->setPrivate(TRUE);
+    $service = $container->setDefinition('foo', $definition);
+    $this->assertTrue($service->isPublic());
+    $this->assertFalse($service->isPrivate());
+  }
+
   /**
    * @covers ::setAlias
    */