Contexts not mapped in time to use them in BlockInterface::access()

If things worked as expected, we should also be able to remove the getRuntimeContexts/applyContextMapping calls from BlockViewBuilder.
But \Drupal\block\BlockViewBuilder::lazyBuilder() purposefully does a fresh entity_load() :(

This patch makes sense to me. Two things jump out, though:

1. +++ b/core/modules/block/src/BlockAccessControlHandler.php
   @@ -127,7 +127,16 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
   +          if ($entity->getPlugin() instanceof ContextAwarePluginInterface) {
   +            $contexts = $this->contextRepository->getRuntimeContexts(array_values($entity->getPlugin()->getContextMapping()));
   +            $this->contextHandler->applyContextMapping($entity->getPlugin(), $contexts);
   +          }
   +          $access = $entity->getPlugin()->access($account, TRUE);
   

   Nit: $entity->getPlugin() is called several times. Maybe call it once and then re-use the return value? That might make it a bit easier to read.

2. +++ b/core/modules/block/src/BlockAccessControlHandler.php
   @@ -127,7 +127,16 @@ protected function checkAccess(EntityInterface $entity, $operation, AccountInter
   +        catch (ContextException $e) {
   +          $access = AccessResult::forbidden();
   +        }
   

   If it's reasonably easy to do, can the failure be logged? Either that, or don't catch the exception at all -- to me, this looks as if we may be swallowing potentially useful debugging info.

If we don't catch it, the page won't render. And we don't want to log it because missing contexts on conditions is treated a access/visibility = false in core.

Eclipse

Okay, I didn't know that. Makes sense. Thanks @EclipseGc and @dsnopek for clearing that up. Maybe we can add a comment to that effect, just so it's not a WTF for others?

Looks good to me. We've discussed the exception catching at length in #2620126: No way to handle 'optional' contexts without try/catch, this is the way it works now, as bad as it (IMHO) is.

Quickly discussed in IRC that we now do the assignment twice. But we can't change that, since we re-load those blocks again due to the lazy builder approach, sadly.

Huge ++ from me on this.

Eclipse

#2620126: No way to handle 'optional' contexts without try/catch was major and was closed as a duplicate, so setting this to major.

Patch looks good. Particularly the max-age zero bit.

I know @catch has has some reservations about this patch - I'm going to assign it to @catch to make sure that they're accounted for.

I still really dislike having to do the try/catch around the exception like this, it's not an exceptional (excuse pun) situation like Symfony HTTP exceptions (which we had an equivalent of with special DRUPAL_NOT_FOUND and drupal_not_found() return values and request short-circuiting), or form handling which similarly needs to short-circuit the request logic - this is just a method that gets some stuff and returns some stuff back or not.

The patch here is OK as a minimal fix in 8.0.x though, so committed/pushed to that and 8.1.x - and re-opening #2620126: No way to handle 'optional' contexts without try/catch.

Committed/pushed to 8.1.x and cherry-picked to 8.0.x. Thanks!