Simplify block caching and allow to cache the whole page, including access checks

1. +++ b/src/Plugin/DisplayVariant/BlockDisplayVariant.php
   @@ -126,15 +127,13 @@ class BlockDisplayVariant extends VariantBase implements ContextAwareVariantInte
   +    // @todo Make the whole display a pre_render callback to fully benefit from
   +    //   render caching and avoid to re-calculate contexts and access for all
   +    //   blocks.
   

   Sounds great :)

2. +++ b/src/Plugin/DisplayVariant/BlockDisplayVariant.php
   @@ -142,7 +141,9 @@ class BlockDisplayVariant extends VariantBase implements ContextAwareVariantInte
   +    $cacheability = CacheableMetadata::createFromRenderArray($build['regions'])
   +      ->addCacheableDependency($page);
   
   @@ -160,12 +161,12 @@ class BlockDisplayVariant extends VariantBase implements ContextAwareVariantInte
   +        $cacheability->addCacheableDependency($access);
   
   @@ -183,39 +184,25 @@ class BlockDisplayVariant extends VariantBase implements ContextAwareVariantInte
   +        $cacheability->addCacheableDependency($block);
   

   Why have a $cacheability variable if you can just apply it to the underlying render array?

   Every one of these should become:

   $this->renderer->addCacheableDependency($build, $dependency);
   

   What am I missing?

3. +++ b/tests/src/Unit/BlockDisplayVariantTest.php
   @@ -200,15 +80,17 @@ class BlockDisplayVariantTest extends UnitTestCase {
   +    $block2->getCacheContexts()->shouldNotBeCalled();
   +    $block2->getCacheMaxAge()->shouldNotBeCalled();
   +    $block2->getCacheTags()->shouldNotBeCalled();
   

   <3

   +1000 for prophesize()!

4. +++ b/tests/src/Unit/BlockDisplayVariantTest.php
   @@ -257,35 +145,32 @@ class BlockDisplayVariantTest extends UnitTestCase {
   +    // metadata is accessible blocks is merged into to avoid cache redirects
   

   s/is/of/

   "merged into" -> into what?

1. +++ b/src/Plugin/DisplayVariant/BlockDisplayVariant.php
   @@ -126,15 +127,13 @@ class BlockDisplayVariant extends VariantBase implements ContextAwareVariantInte
   +    // @todo Make the whole display a pre_render callback to fully benefit from
   

   Nit: By "the whole display", you really mean "this entire function", right? Perhaps the @todo really belongs in build()'s docs?

2. +++ b/src/Plugin/DisplayVariant/BlockDisplayVariant.php
   @@ -142,7 +141,9 @@ class BlockDisplayVariant extends VariantBase implements ContextAwareVariantInte
   +    $cacheability = CacheableMetadata::createFromRenderArray($build['regions'])
   

   s/$cacheability/$page_cacheability/, to make it clearer what you're calculating the cacheability of?

Would be nice to get this in so I can work on that. As commented there, this would give me a 20% performance improvement for a specific page in a project.

Sounds awesome!

I haven't tested this but a quick read through the patch looks great! I think this will also simplify the patch on #2511570: Remove Drupal\page_manager\Plugin\PageAwareVariantInterface. So, RTBC+1!

Conflicted with #2550815: BlockDisplayVariant::buildBlock() must check emptyness with Element::isEmpty(), can I get a reroll?

Thanks!