core/modules/big_pipe/big_pipe.module | 55 ++++++++++++---------- core/modules/big_pipe/big_pipe.services.yml | 5 ++ .../big_pipe/src/Controller/BigPipeController.php | 12 +++-- .../big_pipe/src/PageCache/DenyBigPipeResponse.php | 24 ++++++++++ core/modules/big_pipe/src/Render/BigPipe.php | 16 ++----- .../src/Render/Placeholder/BigPipeStrategy.php | 19 +++++++- .../page_cache/src/StackMiddleware/PageCache.php | 8 ++-- 7 files changed, 93 insertions(+), 46 deletions(-) diff --git a/core/modules/big_pipe/big_pipe.module b/core/modules/big_pipe/big_pipe.module index a91c739..d484f86 100644 --- a/core/modules/big_pipe/big_pipe.module +++ b/core/modules/big_pipe/big_pipe.module @@ -39,34 +39,41 @@ function big_pipe_page_attachments(array &$page) { } $request = \Drupal::request(); + // BigPipe only uses JS when there is an actual session, so only add the no-JS + // detection then. + // @see \Drupal\big_pipe\Render\Placeholder\BigPipeStrategy. + $session_exists = \Drupal::service('session_configuration')->hasSession($request); + $page['#cache']['contexts'][] = 'session.exists'; // Only do the no-JS detection while we don't know if there's no JS support: // avoid endless redirect loops. $has_big_pipe_nojs_cookie = $request->cookies->has(BigPipeStrategy::NOJS_COOKIE); $page['#cache']['contexts'][] = 'cookies:' . BigPipeStrategy::NOJS_COOKIE; - if (!$has_big_pipe_nojs_cookie) { - // Let server set the BigPipe no-JS cookie. - $page['#attached']['html_head'][] = [ - [ - // Redirect through a 'Refresh' meta tag if JavaScript is disabled. - '#tag' => 'meta', - '#noscript' => TRUE, - '#attributes' => [ - 'http-equiv' => 'Refresh', - // @todo: Switch to Url::fromRoute() once https://www.drupal.org/node/2589967 is resolved. - 'content' => '0; URL=' . Url::fromUri('internal:/big_pipe/no-js', ['query' => \Drupal::service('redirect.destination')->getAsArray()])->toString(), + if ($session_exists) { + if (!$has_big_pipe_nojs_cookie) { + // Let server set the BigPipe no-JS cookie. + $page['#attached']['html_head'][] = [ + [ + // Redirect through a 'Refresh' meta tag if JavaScript is disabled. + '#tag' => 'meta', + '#noscript' => TRUE, + '#attributes' => [ + 'http-equiv' => 'Refresh', + // @todo: Switch to Url::fromRoute() once https://www.drupal.org/node/2589967 is resolved. + 'content' => '0; URL=' . Url::fromUri('internal:/big_pipe/no-js', ['query' => \Drupal::service('redirect.destination')->getAsArray()])->toString(), + ], ], - ], - 'big_pipe_detect_nojs', - ]; - } - else { - // Let client delete the BigPipe no-JS cookie. - $page['#attached']['html_head'][] = [ - [ - '#tag' => 'script', - '#value' => 'document.cookie = "' . BigPipeStrategy::NOJS_COOKIE . '=1; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"', - ], - 'big_pipe_detect_js', - ]; + 'big_pipe_detect_nojs', + ]; + } + else { + // Let client delete the BigPipe no-JS cookie. + $page['#attached']['html_head'][] = [ + [ + '#tag' => 'script', + '#value' => 'document.cookie = "' . BigPipeStrategy::NOJS_COOKIE . '=1; path=/; expires=Thu, 01 Jan 1970 00:00:00 GMT"', + ], + 'big_pipe_detect_js', + ]; + } } } diff --git a/core/modules/big_pipe/big_pipe.services.yml b/core/modules/big_pipe/big_pipe.services.yml index 5235037..82a0c73 100644 --- a/core/modules/big_pipe/big_pipe.services.yml +++ b/core/modules/big_pipe/big_pipe.services.yml @@ -23,3 +23,8 @@ services: class: Drupal\big_pipe\EventSubscriber\NoBigPipeRouteAlterSubscriber tags: - { name: event_subscriber } + + big_pipe.page_cache: + class: Drupal\big_pipe\PageCache\DenyBigPipeResponse + tags: + - { name: page_cache_response_policy } diff --git a/core/modules/big_pipe/src/Controller/BigPipeController.php b/core/modules/big_pipe/src/Controller/BigPipeController.php index e7c9da5..7d1ae3e 100644 --- a/core/modules/big_pipe/src/Controller/BigPipeController.php +++ b/core/modules/big_pipe/src/Controller/BigPipeController.php @@ -37,10 +37,12 @@ public function setNoJsCookie(Request $request) { // This controller may only be accessed when the browser does not support // JavaScript. It is accessed automatically when that's the case thanks to // big_pipe_page_attachments(). When this controller is executed, deny - // access when the no-JS cookie is already set: this indicates a redirect - // loop, since the cookie was already set, yet the user is executing this - // controller. - if ($request->cookies->has(BigPipeStrategy::NOJS_COOKIE)) { + // access when either: + // - the no-JS cookie is already set: this indicates a redirect loop, since + // the cookie was already set, yet the user is executing this controller; + // - there is no session, in which case BigPipe never uses JS anyway, so it + // is pointless to set this cookie. + if ($request->cookies->has(BigPipeStrategy::NOJS_COOKIE) || $request->getSession() === NULL) { throw new AccessDeniedHttpException(); } @@ -51,7 +53,7 @@ public function setNoJsCookie(Request $request) { $response = new LocalRedirectResponse($request->query->get('destination')); // Set cookie without httpOnly, so that JavaScript can delete it. $response->headers->setCookie(new Cookie(BigPipeStrategy::NOJS_COOKIE, TRUE, 0, '/', NULL, FALSE, FALSE)); - $response->addCacheableDependency((new CacheableMetadata())->addCacheContexts(['cookies:' . BigPipeStrategy::NOJS_COOKIE])); + $response->addCacheableDependency((new CacheableMetadata())->addCacheContexts(['cookies:' . BigPipeStrategy::NOJS_COOKIE, 'session.exists'])); return $response; } diff --git a/core/modules/big_pipe/src/PageCache/DenyBigPipeResponse.php b/core/modules/big_pipe/src/PageCache/DenyBigPipeResponse.php new file mode 100644 index 0000000..9fd5e2c --- /dev/null +++ b/core/modules/big_pipe/src/PageCache/DenyBigPipeResponse.php @@ -0,0 +1,24 @@ +', $content, 2); - $pre_body_output_page_cache = $this->sendPreBody($pre_body, $nojs_placeholders, $cumulative_assets); - $placeholder_output_page_cache = $this->sendPlaceholders($placeholders, $this->getPlaceholderOrder($pre_body), $cumulative_assets); - $post_body_output_page_cache = $this->sendPostBody($post_body); + $page_cache_response = $this->sendPreBody($pre_body, $nojs_placeholders, $cumulative_assets); + $this->sendPlaceholders($placeholders, $this->getPlaceholderOrder($pre_body), $cumulative_assets); + $page_cache_response .= $this->sendPostBody($post_body); // @todo this is hacky as hell - $response_for_page_cache = $pre_body_output_page_cache . $placeholder_output_page_cache . $post_body_output_page_cache; if (\Drupal::currentUser()->isAnonymous()) { - $this->set($this->requestStack->getMasterRequest(), new Response($response_for_page_cache), -1, []); + $this->set($this->requestStack->getMasterRequest(), new Response($page_cache_response ), -1, []); } if ($has_session) { @@ -377,11 +376,9 @@ protected function sendNoJsPlaceholders($html, $no_js_placeholders, AttachedAsse * simplify debugging. */ protected function sendPlaceholders(array $placeholders, array $placeholder_order, AttachedAssetsInterface $cumulative_assets) { - $sent_output = ''; - // Return early if there are no BigPipe placeholders to send. if (empty($placeholders)) { - return $sent_output; + return; } // Send the start signal. @@ -389,7 +386,6 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde print static::START_SIGNAL; print "\n"; flush(); - $sent_output .= "\n" . static::START_SIGNAL . "\n"; // A BigPipe response consists of a HTML response plus multiple embedded // AJAX responses. To process the attachments of those AJAX responses, we @@ -463,7 +459,6 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde EOF; print $output; flush(); - $sent_output .= $output; // Another placeholder was rendered and sent, track the set of asset // libraries sent so far. Any new settings are already sent; we don't need @@ -478,7 +473,6 @@ protected function sendPlaceholders(array $placeholders, array $placeholder_orde print static::STOP_SIGNAL; print "\n"; flush(); - $sent_output .= "\n" . static::START_SIGNAL . "\n"; return $sent_output; } diff --git a/core/modules/big_pipe/src/Render/Placeholder/BigPipeStrategy.php b/core/modules/big_pipe/src/Render/Placeholder/BigPipeStrategy.php index 6013bb5..f022ec0 100644 --- a/core/modules/big_pipe/src/Render/Placeholder/BigPipeStrategy.php +++ b/core/modules/big_pipe/src/Render/Placeholder/BigPipeStrategy.php @@ -117,7 +117,8 @@ public function processPlaceholders(array $placeholders) { return []; } - return $this->doProcessPlaceholders($placeholders); + $has_session = $this->sessionConfiguration->hasSession($request); + return $this->doProcessPlaceholders($placeholders, $has_session); } /** @@ -125,13 +126,21 @@ public function processPlaceholders(array $placeholders) { * * @param array $placeholders * The placeholders to process. + * @param bool $has_session * * @return array * The BigPipe placeholders. */ - protected function doProcessPlaceholders(array $placeholders) { + protected function doProcessPlaceholders(array $placeholders, $has_session) { $overridden_placeholders = []; foreach ($placeholders as $placeholder => $placeholder_elements) { + // When using BigPipe without a session, i.e. for anonymous users, never + // use JavaScript. This allows BigPipe to accelerate Page Cache misses. + if (!$has_session) { + $overridden_placeholders[$placeholder] = static::createBigPipeNoJsPlaceholder($placeholder, $placeholder_elements, FALSE); + continue; + } + // BigPipe uses JavaScript and the DOM to find the placeholder to replace. // This means finding the placeholder to replace must be efficient. Most // placeholders are HTML, which we can find efficiently thanks to the @@ -182,6 +191,9 @@ protected static function createBigPipeJsPlaceholder($original_placeholder, arra '#markup' => '
', '#cache' => [ 'max-age' => 0, + 'contexts' => [ + 'session.exists', + ], ], '#attached' => [ 'library' => [ @@ -224,6 +236,9 @@ protected static function createBigPipeNoJsPlaceholder($original_placeholder, ar '#markup' => $big_pipe_placeholder, '#cache' => [ 'max-age' => 0, + 'contexts' => [ + 'session.exists', + ], ], '#attached' => [ 'big_pipe_nojs_placeholders' => [ diff --git a/core/modules/page_cache/src/StackMiddleware/PageCache.php b/core/modules/page_cache/src/StackMiddleware/PageCache.php index 2491b5c..6d35331 100644 --- a/core/modules/page_cache/src/StackMiddleware/PageCache.php +++ b/core/modules/page_cache/src/StackMiddleware/PageCache.php @@ -239,14 +239,14 @@ protected function fetch(Request $request, $type = self::MASTER_REQUEST, $catch // Therefore exclude them, even for subclasses that implement // CacheableResponseInterface. if ($response instanceof BinaryFileResponse || $response instanceof StreamedResponse) { - // @todo supporting BigPipe means that we return early here, which means - // that none of the code below runs. Thus: - // - page cache response policies won't work for BigPipe (which I guess can be okay since we need to duplicate that part of the page cache code in BigPipe anyway, because we need to assemble the final non-BigPipe response from it and then store it in Page Cache - // - for the same reason, we need to handle all the other logic below: 4xx responses cache tags + expiration date return $response; } // Allow policy rules to further restrict which responses to cache. + // @todo supporting BigPipe means that we return early here, which means + // that none of the code below runs. Thus: + // - page cache response policies won't work for BigPipe (which I guess can be okay since we need to duplicate that part of the page cache code in BigPipe anyway, because we need to assemble the final non-BigPipe response from it and then store it in Page Cache + // - for the same reason, we need to handle all the other logic below: 4xx responses cache tags + expiration date if ($this->responsePolicy->check($response, $request) === ResponsePolicyInterface::DENY) { return $response; }