Issue #2254865 by Wim Leers, lauriii, borisson_, JeroenT, alexpott, Fabianx:...

Issue #2254865 by Wim Leers, lauriii, borisson_, JeroenT, alexpott, Fabianx: toolbar_pre_render() runs on every page and is responsible for ~15ms/17000 function calls

core/modules/contextual/contextual.module

@@ -14,12 +14,20 @@
  * Implements hook_toolbar().
  */
 function contextual_toolbar() {
+  $items = [];
+  $items['contextual'] = [
+    '#cache' => [
+      'contexts' => [
+        'user.permissions',
+      ],
+    ],
+  ];
 
   if (!\Drupal::currentUser()->hasPermission('access contextual links')) {
-    return;
+    return $items;
   }
 
-  $tab['contextual'] = array(
+  $items['contextual'] += array(
     '#type' => 'toolbar_item',
     'tab' => array(
       '#type' => 'html_tag',
@@ -41,7 +49,7 @@ function contextual_toolbar() {
     ),
   );
 
-  return $tab;
+  return $items;
 }
 
 /**

core/modules/shortcut/shortcut.module

@@ -364,10 +364,22 @@ function shortcut_preprocess_page(&$variables) {
 function shortcut_toolbar() {
   $user = \Drupal::currentUser();
 
-  $items = array();
+  $items = [];
+  $items['shortcuts'] = [
+    '#cache' => [
+      'contexts' => [
+        // Cacheable per user, because each user can have their own shortcut
+        // set, even if they cannot create or select a shortcut set, because
+        // an administrator may have assigned a non-default shortcut set.
+        'user',
+      ],
+    ],
+  ];
+
   if ($user->hasPermission('access shortcuts')) {
     $links = shortcut_renderable_links();
     $shortcut_set = shortcut_current_displayed_set();
+    \Drupal::service('renderer')->addCacheableDependency($items['shortcuts'], $shortcut_set);
     $configure_link = NULL;
     if (shortcut_set_edit_access($shortcut_set)->isAllowed()) {
       $configure_link = array(
@@ -378,7 +390,7 @@ function shortcut_toolbar() {
       );
     }
     if (!empty($links) || !empty($configure_link)) {
-      $items['shortcuts'] = array(
+      $items['shortcuts'] += array(
         '#type' => 'toolbar_item',
         'tab' => array(
           '#type' => 'link',

core/modules/system/src/Tests/Cache/AssertPageCacheContextsAndTagsTrait.php

@@ -102,16 +102,22 @@ protected function assertCacheTags(array $expected_tags) {
    *
    * @param string[] $expected_contexts
    *   The expected cache contexts.
+   * @param string $message
+   *   (optional) A verbose message to output.
+   *
+   * @return
+   *   TRUE if the assertion succeeded, FALSE otherwise.
    */
-  protected function assertCacheContexts(array $expected_contexts) {
+  protected function assertCacheContexts(array $expected_contexts, $message = NULL) {
     $actual_contexts = $this->getCacheHeaderValues('X-Drupal-Cache-Contexts');
     sort($expected_contexts);
     sort($actual_contexts);
-    $this->assertIdentical($actual_contexts, $expected_contexts);
-    if ($actual_contexts !== $expected_contexts) {
+    $return = $this->assertIdentical($actual_contexts, $expected_contexts, $message);
+    if (!$return) {
       debug('Missing cache contexts: ' . implode(',', array_diff($actual_contexts, $expected_contexts)));
       debug('Unwanted cache contexts: ' . implode(',', array_diff($expected_contexts, $actual_contexts)));
     }
+    return $return;
   }
 
   /**

core/modules/toolbar/src/Tests/ToolbarCacheContextsTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\toolbar\Tests\ToolbarCacheContextsTest.
+ */
+
+namespace Drupal\toolbar\Tests;
+
+use Drupal\Core\Cache\Cache;
+use Drupal\simpletest\WebTestBase;
+use Drupal\system\Tests\Cache\AssertPageCacheContextsAndTagsTrait;
+
+
+/**
+ * Tests the cache contexts for toolbar.
+ *
+ * @group toolbar
+ */
+class ToolbarCacheContextsTest extends WebTestBase {
+
+  use AssertPageCacheContextsAndTagsTrait;
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = ['toolbar', 'test_page_test'];
+
+  /**
+   * An authenticated user to use for testing.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $adminUser;
+
+  /**
+   * An authenticated user to use for testing.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $adminUser2;
+
+  /**
+   * A list of default permissions for test users.
+   *
+   * @var array
+   */
+  protected $perms = [
+    'access toolbar',
+    'access administration pages',
+    'administer site configuration',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+
+    $this->adminUser = $this->drupalCreateUser($this->perms);
+    $this->adminUser2 = $this->drupalCreateUser($this->perms);
+  }
+
+  /**
+   * Tests toolbar cache contexts.
+   */
+  public function testToolbarCacheContextsCaller() {
+    // Test with default combination and permission to see toolbar.
+    $this->assertToolbarCacheContexts(['user'], 'Expected cache contexts found for default combination and permission to see toolbar.');
+
+    // Test without user toolbar tab. User module is a required module so we have to
+    // manually remove the user toolbar tab.
+    $this->installModules(['toolbar_disable_user_toolbar']);
+    $this->assertToolbarCacheContexts(['user.permissions'], 'Expected cache contexts found without user toolbar tab.');
+
+    // Test with the toolbar and contextual enabled.
+    $this->installModules(['contextual']);
+    $this->adminUser2 = $this->drupalCreateUser(array_merge($this->perms, ['access contextual links']));
+    $this->assertToolbarCacheContexts(['user.permissions'], 'Expected cache contexts found with contextual module enabled.');
+    \Drupal::service('module_installer')->uninstall(['contextual']);
+
+    // Test with the tour module enabled.
+    $this->installModules(['tour']);
+    $this->adminUser2 = $this->drupalCreateUser(array_merge($this->perms, ['access tour']));
+    $this->assertToolbarCacheContexts(['user.permissions'], 'Expected cache contexts found with tour module enabled.');
+    \Drupal::service('module_installer')->uninstall(['tour']);
+
+    // Test with shortcut module enabled.
+    $this->installModules(['shortcut']);
+    $this->adminUser2 = $this->drupalCreateUser(array_merge($this->perms, ['access shortcuts', 'administer shortcuts']));
+    $this->assertToolbarCacheContexts(['user'], 'Expected cache contexts found with shortcut module enabled.');
+  }
+
+  /**
+   * Tests that cache contexts are applied for both users.
+   *
+   * @param string[] $cache_contexts
+   *   Expected cache contexts for both users.
+   * @param string $message
+   *   (optional) A verbose message to output.
+   *
+   * @return
+   *   TRUE if the assertion succeeded, FALSE otherwise.
+   */
+  protected function assertToolbarCacheContexts(array $cache_contexts, $message = NULL) {
+    // Default cache contexts that should exist on all test cases.
+    $default_cache_contexts = [
+      'languages:language_interface',
+      'theme',
+    ];
+    $cache_contexts = Cache::mergeContexts($default_cache_contexts, $cache_contexts);
+
+    // Assert contexts for user1 which has only default permissions.
+    $this->drupalLogin($this->adminUser);
+    $this->drupalGet('test-page');
+    $return = $this->assertCacheContexts($cache_contexts);
+    $this->drupalLogout();
+
+    // Assert contexts for user2 which has some additional permissions.
+    $this->drupalLogin($this->adminUser2);
+    $this->drupalGet('test-page');
+    $return = $return && $this->assertCacheContexts($cache_contexts);
+
+    if ($return) {
+      $this->pass($message);
+    }
+    else {
+      $this->fail($message);
+    }
+    return $return;
+  }
+
+  /**
+   * Installs a given list of modules and rebuilds the cache.
+   *
+   * @param string[] $module_list
+   *   An array of module names.
+   */
+  protected function installModules(array $module_list) {
+    \Drupal::service('module_installer')->install($module_list);
+
+    // Installing modules updates the container and needs a router rebuild.
+    $this->container = \Drupal::getContainer();
+    $this->container->get('router.builder')->rebuildIfNeeded();
+  }
+
+
+}

core/modules/toolbar/tests/modules/toolbar_disable_user_toolbar/toolbar_disable_user_toolbar.info.yml

+name: 'Disable user toolbar'
+type: module
+description: 'Support module for testing toolbar without user toolbar'
+package: Testing
+version: VERSION
+core: 8.x

core/modules/toolbar/tests/modules/toolbar_disable_user_toolbar/toolbar_disable_user_toolbar.module

+<?php
+
+/**
+ * Implements hook_toolbar_alter().
+ */
+function toolbar_disable_user_toolbar_toolbar_alter(&$items) {
+  unset($items['user']);
+}

core/modules/toolbar/toolbar.module

@@ -59,6 +59,10 @@ function toolbar_page_top(array &$page_top) {
   $page_top['toolbar'] = array(
     '#type' => 'toolbar',
     '#access' => \Drupal::currentUser()->hasPermission('access toolbar'),
+    '#cache' => [
+      'keys' => ['toolbar'],
+      'contexts' => ['user.permissions'],
+    ],
   );
 }
 
@@ -97,17 +101,19 @@ function template_preprocess_toolbar(&$variables) {
       }
     }
 
-    $attributes = array();
-    // Pass the wrapper attributes along.
-    if (array_key_exists('#wrapper_attributes', $element[$key])) {
-      $attributes = $element[$key]['#wrapper_attributes'];
-    }
-
     // Add the tab.
-    $variables['tabs'][$key] = array(
-      'link' => $element[$key]['tab'],
-      'attributes' => new Attribute($attributes),
-    );
+    if (isset($element[$key]['tray'])) {
+      $attributes = array();
+      // Pass the wrapper attributes along.
+      if (array_key_exists('#wrapper_attributes', $element[$key])) {
+        $attributes = $element[$key]['#wrapper_attributes'];
+      }
+
+      $variables['tabs'][$key] = array(
+        'link' => $element[$key]['tab'],
+        'attributes' => new Attribute($attributes),
+      );
+    }
 
     // Add other non-tray, non-tab child elements to the remainder variable for
     // later rendering.

core/modules/tour/tour.module

@@ -32,11 +32,20 @@ function tour_help($route_name, RouteMatchInterface $route_match) {
  * Implements hook_toolbar().
  */
 function tour_toolbar() {
+  $items = [];
+  $items['tour'] = [
+    '#cache' => [
+      'contexts' => [
+        'user.permissions',
+      ],
+    ],
+  ];
+
   if (!\Drupal::currentUser()->hasPermission('access tour')) {
-    return;
+    return $items;
   }
 
-  $tab['tour'] = array(
+  $items['tour'] += array(
     '#type' => 'toolbar_item',
     'tab' => array(
       '#type' => 'html_tag',
@@ -59,7 +68,7 @@ function tour_toolbar() {
     ),
   );
 
-  return $tab;
+  return $items;
 }
 
 /**

core/modules/user/user.module

@@ -4,6 +4,7 @@
 use Drupal\Component\Utility\SafeMarkup;
 use Drupal\Component\Utility\Unicode;
 use Drupal\Core\Asset\AttachedAssetsInterface;
+use Drupal\Core\Cache\Cache;
 use Drupal\Core\Entity\Display\EntityViewDisplayInterface;
 use Drupal\Core\Field\BaseFieldDefinition;
 use Drupal\Core\Form\FormStateInterface;
@@ -1317,6 +1318,7 @@ function user_toolbar() {
   $user = \Drupal::currentUser();
 
   // Add logout & user account links or login link.
+  $links_cache_contexts = [];
   if ($user->isAuthenticated()) {
     $links = array(
       'account' => array(
@@ -1338,6 +1340,8 @@ function user_toolbar() {
         'url' => Url::fromRoute('user.logout'),
       ),
     );
+    // The "Edit user account" link is per-user.
+    $links_cache_contexts[] = 'user';
   }
   else {
      $links = array(
@@ -1358,10 +1362,21 @@ function user_toolbar() {
         'title' => t('My account'),
         'class' => array('toolbar-icon', 'toolbar-icon-user'),
       ),
+      '#cache' => [
+        'contexts' => [
+          // Cacheable per user, because the current user's name is shown.
+          'user',
+        ],
+      ],
     ),
     'tray' => array(
       '#heading' => t('User account actions'),
       'user_links' => array(
+        '#cache' => [
+          // Cacheable per "authenticated or not", because the links to
+          // display depend on that.
+          'contexts' => Cache::mergeContexts(array('user.roles:authenticated'), $links_cache_contexts),
+        ],
         '#theme' => 'links__toolbar_user',
         '#links' => $links,
         '#attributes' => array(