Prevent layout cache corruption in edge case

[scottsb]

Description

When layout is loaded from the cache, it ought to repopulate the list of applied layout handles to prevent any chance of a layout handle being reapplied later, layering on top of the XML loaded from cache. This pull request adds that missing functionality.

The bug caused by the status quo is triggered when layout is loaded early before the page actually begins to render and before all layout handles may be applied. (Layout is loaded early, for example, when $resultPage->getConfig()->getTitle() is called, as happens in multiple core controllers, including checkout.) If this initial layout load matches a cache key but the latter layout load (with all layout handles assembled) does not, then when the latter layout load occurs, the guard condition which attempts to block duplicate layout handle application (in _merge of Magento\Framework\View\Model\Layout\Merge) does not get triggered, since the layout handles loaded from cache were not populated.

This duplicate XML generally does not cause issues, because earlier nodes are replaced by later nodes via the _overrideElementWorkaround method in \Magento\Framework\View\Layout\ScheduledStructure\Helper, and since the duplicate nodes are the same as the originals, there's no problem. However, because this method removes the entire tree (including descendants) of the first instance of duplicated nodes, it can lead to nodes missing from the final structure, depending on the layout order sequence. (For example, if a parent node such as <container name="content" label="Main Content Area"/> ends up appearing after some element that was supposed to be inserted into that container, the child block never gets re-added.) This method's algorithm is a little clunky itself and could probably be refined, but by merging this fix, we avoid at least one scenario where this algorithm is even needed.

Manual testing scenarios

As far as I've seen this does not cause any issues in core alone, but it can cause issues when custom modules extend core in standard ways. This describes one concrete case where this can occur, but it is likely to occur in other cases as well.

1. Create a custom module:
   1. Add an afterExecute plugin on Magento\Checkout\Controller\Index\Index.
   2. In this plugin method, conditionally add a layout handle if the cart contains a particular product.
2. Enable and clear layout cache.
3. Add a product to the cart other than the one checked in the plugin condition.
4. Load checkout. (Checkout should appear correctly.)
5. Add the product to the cart checked in the plugin condition.
6. Load checkout again. (Without this fix, checkout will break, as the customer.customer.data block will not be loaded. With this fix, checkout will load correctly.)

[ladle3000]

@scottsb How can I add this fix to my M 2.2 installation? I'm hitting this exact error I believe when Magestore GiftCard Module is running on my store.

[scottsb]

@snoroozi You could create a preference in a local module for this class, overriding the load() method with the change from this PR. Or you might be able to use the PR patch with this patch application module (if the PR patch doesn't work, it probably would only need a little fine tuning).

[magento-engcom-team]

@scottsb thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

[magento-engcom-team]

Hi @scottsb. Thank you for your contribution.
Changes from your Pull Request will be available with the upcoming 2.2.6 release.

Please, consider to port this solution to 2.3 release line.
You may use Porting tool to port commits automatically.