Move shutdown functions into the shutdown handler class.

This patch no longer applies, and it looks like something completely breaks all the tests.

Tests failing mostly to do with

01:54:48 Fatal error: Uncaught Error: Class 'Drupal\Core\Utility\ShutdownHandler' not found in /var/www/html/core/modules/simpletest/src/TestBase.php:1132

Looks nice start, error handling only needs polishing

1. +++ b/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php
   @@ -633,7 +634,7 @@ protected function prepareEnvironment() {
        // from both environments and the testing environment will try to call the
        // handlers defined by the original one.
   -    $callbacks = &drupal_register_shutdown_function();
   +    $callbacks = &ShutdownHandler::register();
        $this->originalShutdownCallbacks = $callbacks;
        $callbacks = [];
   

   This by reference stuff feels icky, wondering if we should have explicit get/set methods now that it is a class?

2. +++ b/core/lib/Drupal/Core/Utility/ShutdownHandler.php
   @@ -0,0 +1,105 @@
   + */
   ...
   +
   +  /**
   +   * Array of shutdown functions to be executed.
   +   *
   +   * We cannot use drupal_static() here because the static cache is reset during
   +   * batch processing, which breaks batch handling.
   +   *
   +   * @var callable[]
   +   */
   +  protected static $callbacks = [];
   

   the drupal_static() documentation feels a bit off now, but not sure what else to say. Maybe the class could explain why it can't be a service, and then it is implied that it has to be static?

3. +++ b/core/lib/Drupal/Core/Utility/ShutdownHandler.php
   @@ -0,0 +1,105 @@
   +   * @param ...
   +   *   Additional arguments to pass to the shutdown function.
   +   *
   +   * @return array
   +   *   Array of shutdown functions to be executed.
   +   *
   +   * @see register_shutdown_function()
   +   * @ingroup php_wrappers
   +   */
   +  public static function &register(callable $callback = NULL) {
   

   can we define this using the ... variable length thing now?

4. +++ b/core/lib/Drupal/Core/Utility/ShutdownHandler.php
   @@ -0,0 +1,105 @@
   +   *
   +   * @param \Exception|\Throwable $exception
   +   *   The exception object that was thrown.
   +   *
   +   * @see ShutdownHandler::shutdown()
   +   */
   +  protected static function handleException($exception) {
   

   can we type hint this now that we require php7?

5. +++ b/core/tests/Drupal/KernelTests/Core/Utility/ShutdownLegacyTest.php
   @@ -0,0 +1,24 @@
   +
   +/**
   + * Class ShutdownLegacyTest.
   + *
   + * @package Drupal\KernelTests\Core\Utility
   + * @group legacy
   + */
   +class ShutdownLegacyTest extends KernelTestBase {
   

   docblock needs an update.

6. +++ b/core/tests/Drupal/Tests/BrowserTestBase.php
   @@ -462,8 +463,8 @@ protected function tearDown() {
    
        // Restore original shutdown callbacks.
   -    if (function_exists('drupal_register_shutdown_function')) {
   -      $callbacks = &drupal_register_shutdown_function();
   +    if (method_exists(ShutdownHandler::class, 'register')) {
   +      $callbacks = &ShutdownHandler::register();
          $callbacks = $this->originalShutdownCallbacks;
        }
   

   Do we really still need this check? When cold it possibly not exist?

Applied the MR and searched for drupal_register_shutdown_function and seems all instances have been replaced.

Only question is the formatting of the trigger_errors. Not sure it matters but wanted to flag for committers.

I've added some comments to the MR.

feedback appears to have been addressed. Left a note in IS that an api change is being proposed.

Nice to see a reduction in the 'includes'!

I'm triaging RTBC issues. I read the IS, the comments, the change record and the MR. I didn't see any unanswered questions.

When setting an issue to RTBC the comment should explain what one did to determine the issue was ready for commit to core. For example, did you see something you liked or disliked, what core gates did you check, etc.

I looked at the comments raised by alexpott in the MR and see that they have been marked 'resolved' by the person who made the changes. I do not see any comment from another person that those changes have been reviewed. Therefor, I am setting this back to NR.

I made minor changes to the change record.

Some small changes. Rebased as the current MR didn't have the pipeline code yet.

Seems to have code failure, have not re-reviewed yet.

Still appears to have failures in MR.

Changes look good to me, and feedback addressed.

+1 RTBC but per new approach for #needs-review-queue-initiative going to leave in NR for a few days for additional eyes.

Going to go ahead and mark it, been a few days.

Checking the RTBC queue.

Ah, this is the issue where I commented about the same person making changes in response to comments in MR and them setting those comments to resolved. At the time I didn't realize there are limitations on who can resolve comments.

There have been some changes to the MR but mostly just keeping it up to date.

Leaving at RTBC.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

This will need to make changes due to #3405976: Transaction autocommit during shutdown relies on unreliable object destruction order (xdebug 3.3+ enabled) as that ended up adding a shutdown function.

I found a bunch of documentation changes to make. Some are due to recent changes in documentation standards, added long after work on this patch started. Some are kind of nitpicky. Some are due to needing better grammar. Please consider them.