This is somewhat long and lacking in headings so here's the short: there's an undocumented variable 'menu_expanded' that adds a redundant layer of caching to the menu build process, which is causing a problem for test cases I am writing, and I believe it should be removed and that doing so will not break anything (except possibly for poorly behaved contrib modules).
I am having an issue with the 'menu_expanded' variable not being updated correctly during testing. Firstly I would like to say that I am greatly displeased that this variable exists and is not documented anywhere. Not anywhere. So I may as well explain what it is:
The 'menu_expanded' variable, as far as I can tell, is used as a cache for a database check. It is only ever read during menu_tree_page_data when creating the parameters to pass to menu_build_tree. Specifically, only menu names which are in the 'menu_expanded' variable are checked for 'expanded' menu items. The variable is created with the private function _menu_set_expanded_menus, which is called from _menu_clear_page_cache, which is called whenever any menu is rebuilt or modified. Because this may happen multiple times in a single request, _menu_clear_page_cache will only perform the cache clear once immediately, and if it is called again during the same page request it will add the cache clearing operation as a shutdown function.
Aside: I still haven't figured out why this breaks during testing, but it certainly has to do with that odd "first rebuild immediately, afterwards on shutdown" behavior. For whatever reason the variable is being set up correctly, then clobbered at some point, then not fixed until shutdown functions are called which during a test run may not be soon enough.
However, from my analysis this undocumented variable is completely useless. It is only used to build $tree_parameters to pass to menu_build_tree, but $tree_parameters is already cached. Any time the 'menu_expanded' variable would be refreshed is a time when the cache would be cleared anyway. So all that this variable does is add a redundant layer of caching that doesn't work right during test cases.
I understand that this may not be the sort of change being considered in Drupal 7 anymore. I do not know how the menu system works in Drupal 8 so I don't know if this is still an issue. Nonetheless I am suggesting that the 'menu_expanded' variable be removed. This will not break anything (except fragile hackish things, possibly), and could only affect performance immediately after a cache clear. This means removing the _menu_set_expanded_menus function and all references to it (on lines 3319 and 3325 of includes/menu.inc) and removing the variable from the conditional on line 1285.
Comments