From 01715f09d883c2b768d0eb711824244adb46bc7c Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 18 Jul 2017 19:44:27 +0000 Subject: [PATCH] [ASDisplayNode] Fix infinite layout loop (#455) * Fix infinite layout loop - The problem will occur if a node does either of the followings: 1. Keeps using a stale pending layout over a calculated layout 2. Doesn't update the next layout's version after calling _setNeedsLayoutFromAbove. * Update CHANGELOG --- CHANGELOG.md | 3 ++- Source/ASDisplayNode+Layout.mm | 13 +++++++++---- Source/ASDisplayNode.mm | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9bb2deddd..8b83bea11e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ - [ASCollectionView] Add delegate bridging and index space translation for missing UICollectionViewLayout properties. [Scott Goodson](https://github.com/appleguy) - [ASTextNode2] Add initial implementation for link handling. [Scott Goodson](https://github.com/appleguy) [#396](https://github.com/TextureGroup/Texture/pull/396) - [ASTextNode2] Provide compile flag to globally enable new implementation of ASTextNode: ASTEXTNODE_EXPERIMENT_GLOBAL_ENABLE. [Scott Goodson](https://github.com/appleguy) [#396](https://github.com/TextureGroup/Texture/pull/410) - - Add ASCollectionGalleryLayoutDelegate - an async collection layout that makes same-size collections (e.g photo galleries, pagers, etc) fast and lightweight! [Huy Nguyen](https://github.com/nguyenhuy/) [#76](https://github.com/TextureGroup/Texture/pull/76) [#451](https://github.com/TextureGroup/Texture/pull/451) +- Add ASCollectionGalleryLayoutDelegate - an async collection layout that makes same-size collections (e.g photo galleries, pagers, etc) fast and lightweight! [Huy Nguyen](https://github.com/nguyenhuy/) [#76](https://github.com/TextureGroup/Texture/pull/76) [#451](https://github.com/TextureGroup/Texture/pull/451) +- Fix an issue that causes infinite layout loop in ASDisplayNode after [#428](https://github.com/TextureGroup/Texture/pull/428) [Huy Nguyen](https://github.com/nguyenhuy) [#455](https://github.com/TextureGroup/Texture/pull/455) ##2.3.5 - Fix an issue where inserting/deleting sections could lead to inconsistent supplementary element behavior. [Adlai Holler](https://github.com/Adlai-Holler) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index 6b73c1bc5e..6d44b4a157 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -302,10 +302,10 @@ ASPrimitiveTraitCollectionDeprecatedImplementation } CGSize boundsSizeForLayout = ASCeilSizeValues(bounds.size); - + // Prefer _pendingDisplayNodeLayout over _calculatedDisplayNodeLayout (if exists, it's the newest) // If there is no _pending, check if _calculated is valid to reuse (avoiding recalculation below). - if (_pendingDisplayNodeLayout == nullptr) { + if (_pendingDisplayNodeLayout == nullptr || _pendingDisplayNodeLayout->version < _layoutVersion) { if (_calculatedDisplayNodeLayout->version >= _layoutVersion && (_calculatedDisplayNodeLayout->requestedLayoutFromAbove == YES || CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, boundsSizeForLayout))) { @@ -352,8 +352,10 @@ ASPrimitiveTraitCollectionDeprecatedImplementation ASLayout *layout = [self calculateLayoutThatFits:constrainedSize restrictedToSize:self.style.size relativeToParentSize:boundsSizeForLayout]; - nextLayout = std::make_shared(layout, constrainedSize, boundsSizeForLayout, version); + // Now that the constrained size of pending layout might have been reused, the layout is useless + // Release it and any orphaned subnodes it retains + _pendingDisplayNodeLayout = nullptr; } if (didCreateNewContext) { @@ -373,6 +375,9 @@ ASPrimitiveTraitCollectionDeprecatedImplementation // particular ASLayout object, and shouldn't loop asking again unless we have a different ASLayout. nextLayout->requestedLayoutFromAbove = YES; [self _setNeedsLayoutFromAbove]; + // Update the layout's version here because _setNeedsLayoutFromAbove calls __setNeedsLayout which in turn increases _layoutVersion + // Failing to do this will cause the layout to be invalid immediately + nextLayout->version = _layoutVersion; } // Prepare to transition to nextLayout @@ -956,7 +961,7 @@ ASPrimitiveTraitCollectionDeprecatedImplementation _unflattenedLayout = displayNodeLayout->layout; displayNodeLayout->layout = [_unflattenedLayout filteredNodeLayoutTree]; } - + _calculatedDisplayNodeLayout = displayNodeLayout; } diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index 84862316b6..2855fa91a8 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -920,7 +920,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) // This method will confirm that the layout is up to date (and update if needed). // Importantly, it will also APPLY the layout to all of our subnodes if (unless parent is transitioning). [self _locked_measureNodeWithBoundsIfNecessary:bounds]; - + [self _locked_layoutPlaceholderIfNecessary]; }