[ASTraitCollection] Remove traitCollectionContext from ASTraitCollection; add containerWindowSize

Passing around a pointer was leading to crashes as the ASVC was the sole owner of the context. There are cases where the VC would dealloc while its subnodes were laying out. This could lead to the subnodes accessing a garbage pointer.
This commit is contained in:
ricky 2016-07-07 09:23:37 -07:00
parent c62a4d3e79
commit 637c4f3a9f
7 changed files with 28 additions and 112 deletions

View File

@ -27,18 +27,6 @@ typedef ASTraitCollection * _Nonnull (^ASDisplayTraitsForTraitWindowSizeBlock)(C
@property (nonatomic, strong, readonly) DisplayNodeType node;
/**
* An optional context to pass along with an ASTraitCollection.
* This can be used to pass any internal state to all subnodes via the ASTraitCollection that is not
* included in UITraitCollection. This could range from more fine-tuned size classes to a class of
* constants that is based upon the new trait collection.
*
* Be aware that internally this context is held by a C struct which cannot retain the pointer. Therefore
* ASVC keeps a strong reference to the context to make sure that it stays alive. If you change this value
* it will propagate the change to the subnodes.
*/
@property (nonatomic, strong) id _Nullable traitCollectionContext;
/**
* Set this block to customize the ASDisplayTraits returned when the VC transitions to the given traitCollection.
*/

View File

@ -58,17 +58,6 @@
return self;
}
- (void)dealloc
{
if (_traitCollectionContext != nil) {
// The setter will iterate through the VC's subnodes and replace the traitCollectionContext in their ASEnvironmentTraitCollection with nil.
// Since the VC holds the only strong reference to this context and we are in the process of destroying
// the VC, all the references in the subnodes will be unsafe unless we nil them out. More than likely all the subnodes will be dealloc'ed
// as part of the VC being dealloc'ed, but this is just to make extra sure.
self.traitCollectionContext = nil;
}
}
- (void)loadView
{
ASDisplayNodeAssertTrue(!_node.layerBacked);
@ -199,27 +188,15 @@ ASVisibilityDepthImplementation;
#pragma mark - ASEnvironmentTraitCollection
- (void)setTraitCollectionContext:(id)traitCollectionContext
{
if (_traitCollectionContext != traitCollectionContext) {
// nil out the displayContext in the subnodes so they aren't hanging around with a dealloc'ed pointer don't set
// the new context yet as this will cause ASEnvironmentTraitCollectionIsEqualToASEnvironmentTraitCollection to fail
ASEnvironmentTraitCollectionUpdateDisplayContext(self.node, nil);
_traitCollectionContext = traitCollectionContext;
}
}
- (ASEnvironmentTraitCollection)environmentTraitCollectionForUITraitCollection:(UITraitCollection *)traitCollection
{
if (self.overrideDisplayTraitsWithTraitCollection) {
ASTraitCollection *asyncTraitCollection = self.overrideDisplayTraitsWithTraitCollection(traitCollection);
self.traitCollectionContext = asyncTraitCollection.traitCollectionContext;
return [asyncTraitCollection environmentTraitCollection];
}
ASEnvironmentTraitCollection asyncTraitCollection = ASEnvironmentTraitCollectionFromUITraitCollection(traitCollection);
asyncTraitCollection.displayContext = self.traitCollectionContext;
asyncTraitCollection.containerWindowSize = self.view.frame.size;
return asyncTraitCollection;
}
@ -227,9 +204,9 @@ ASVisibilityDepthImplementation;
{
if (self.overrideDisplayTraitsWithWindowSize) {
ASTraitCollection *traitCollection = self.overrideDisplayTraitsWithWindowSize(windowSize);
self.traitCollectionContext = traitCollection.traitCollectionContext;
return [traitCollection environmentTraitCollection];
}
self.node.environmentTraitCollection.containerWindowSize = windowSize;
return self.node.environmentTraitCollection;
}
@ -255,17 +232,13 @@ ASVisibilityDepthImplementation;
[super traitCollectionDidChange:previousTraitCollection];
ASEnvironmentTraitCollection environmentTraitCollection = [self environmentTraitCollectionForUITraitCollection:self.traitCollection];
environmentTraitCollection.containerWindowSize = self.view.bounds.size;
[self progagateNewEnvironmentTraitCollection:environmentTraitCollection];
}
- (void)willTransitionToTraitCollection:(UITraitCollection *)newCollection withTransitionCoordinator:(id<UIViewControllerTransitionCoordinator>)coordinator
{
[super willTransitionToTraitCollection:newCollection withTransitionCoordinator:coordinator];
ASEnvironmentTraitCollection environmentTraitCollection = [self environmentTraitCollectionForUITraitCollection:newCollection];
[self progagateNewEnvironmentTraitCollection:environmentTraitCollection];
}
// Note: We don't override willTransitionToTraitCollection:withTransitionCoordinator: because viewWillTransitionToSize:withTransitionCoordinator: will also called
// called in all these cases. However, there are cases where viewWillTransitionToSize:withTransitionCoordinator: but willTransitionToTraitCollection:withTransitionCoordinator:
// is not.
- (void)viewWillTransitionToSize:(CGSize)size withTransitionCoordinator:(id<UIViewControllerTransitionCoordinator>)coordinator
{
[super viewWillTransitionToSize:size withTransitionCoordinator:coordinator];

View File

@ -69,25 +69,9 @@ typedef struct ASEnvironmentTraitCollection {
UIUserInterfaceIdiom userInterfaceIdiom;
UIUserInterfaceSizeClass verticalSizeClass;
UIForceTouchCapability forceTouchCapability;
// WARNING:
// This pointer is in a C struct and therefore not managed by ARC. It is
// an unsafe unretained pointer, so when you dereference it you better be
// sure that it is valid.
//
// Use displayContext when you wish to pass view context specific data along with the
// display traits to subnodes. This should be a piece of data owned by an
// ASViewController, which will ensure that the data is still valid when laying out
// its subviews. When the VC is dealloc'ed, the displayContext it created will also
// be dealloced but any subnodes that are hanging around (why would they be?) will now
// have a displayContext that points to a bad pointer.
//
// As an added precaution ASDisplayTraitsClearDisplayContext is called from ASVC's desctructor
// which will propagate a nil displayContext to its subnodes.
id __unsafe_unretained displayContext;
} ASEnvironmentTraitCollection;
extern void ASEnvironmentTraitCollectionUpdateDisplayContext(id<ASEnvironment> rootEnvironment, id _Nullable context);
CGSize containerWindowSize;
} ASEnvironmentTraitCollection;
extern ASEnvironmentTraitCollection ASEnvironmentTraitCollectionFromUITraitCollection(UITraitCollection *traitCollection);
extern BOOL ASEnvironmentTraitCollectionIsEqualToASEnvironmentTraitCollection(ASEnvironmentTraitCollection lhs, ASEnvironmentTraitCollection rhs);
@ -165,14 +149,11 @@ ASDISPLAYNODE_EXTERN_C_END
if (ASEnvironmentTraitCollectionIsEqualToASEnvironmentTraitCollection(currentTraits, oldTraits) == NO) {\
/* Must dispatch to main for self.view && [self.view.dataController completedNodes]*/ \
ASPerformBlockOnMainThread(^{\
BOOL needsLayout = (oldTraits.displayContext == currentTraits.displayContext) || currentTraits.displayContext != nil;\
NSArray<NSArray <ASCellNode *> *> *completedNodes = [self.view.dataController completedNodes];\
for (NSArray *sectionArray in completedNodes) {\
for (ASCellNode *cellNode in sectionArray) {\
ASEnvironmentStatePropagateDown(cellNode, currentTraits);\
if (needsLayout) {\
[cellNode setNeedsLayout];\
}\
[cellNode setNeedsLayout];\
}\
}\
});\

View File

@ -26,23 +26,11 @@ ASEnvironmentHierarchyState _ASEnvironmentHierarchyStateMakeDefault()
};
}
extern void ASEnvironmentTraitCollectionUpdateDisplayContext(id<ASEnvironment> rootEnvironment, id context)
{
ASEnvironmentState envState = [rootEnvironment environmentState];
ASEnvironmentTraitCollection environmentTraitCollection = envState.environmentTraitCollection;
environmentTraitCollection.displayContext = context;
envState.environmentTraitCollection = environmentTraitCollection;
[rootEnvironment setEnvironmentState:envState];
for (id<ASEnvironment> child in [rootEnvironment children]) {
ASEnvironmentStatePropagateDown(child, environmentTraitCollection);
}
}
ASEnvironmentTraitCollection _ASEnvironmentTraitCollectionMakeDefault()
{
return (ASEnvironmentTraitCollection) {
// Default values can be defined in here
.containerWindowSize = CGSizeZero,
};
}
@ -69,7 +57,7 @@ BOOL ASEnvironmentTraitCollectionIsEqualToASEnvironmentTraitCollection(ASEnviron
lhs.displayScale == rhs.displayScale &&
lhs.userInterfaceIdiom == rhs.userInterfaceIdiom &&
lhs.forceTouchCapability == rhs.forceTouchCapability &&
lhs.displayContext == rhs.displayContext;
CGSizeEqualToSize(lhs.containerWindowSize, rhs.containerWindowSize);
}
ASEnvironmentState ASEnvironmentStateMakeDefault()

View File

@ -18,21 +18,7 @@
@property (nonatomic, assign, readonly) UIUserInterfaceIdiom userInterfaceIdiom;
@property (nonatomic, assign, readonly) UIUserInterfaceSizeClass verticalSizeClass;
@property (nonatomic, assign, readonly) UIForceTouchCapability forceTouchCapability;
/**
* An optional context to pass along with an ASTraitCollection.
* This can be used to pass any internal state to all subnodes via the ASTraitCollection that is not
* included in UITraitCollection. This could range from more fine-tuned size classes to a class of
* constants that is based upon the new trait collection.
*
* Be aware that internally this context is held by a C struct which cannot retain the pointer.
* ASTraitCollection is generally a very short-lived class, existing only to provide a non-struct API
* to trait collections. When an ASTraitCollection is returned via one of ASViewController's 2
* custom trait collection creation blocks, traitCollectionContext is assigned to the VC's traitCollectionContext.
* This makes sure that the VC is the owner of the context and ASEnvironmentTraitCollections will not
* have a reference to a dangling pointer.
*/
@property (nonatomic, strong, readonly) id traitCollectionContext;
@property (nonatomic, assign, readonly) CGSize containerWindowSize;
+ (ASTraitCollection *)traitCollectionWithASEnvironmentTraitCollection:(ASEnvironmentTraitCollection)traits;

View File

@ -21,7 +21,7 @@
horizontalSizeClass:(UIUserInterfaceSizeClass)horizontalSizeClass
verticalSizeClass:(UIUserInterfaceSizeClass)verticalSizeClass
forceTouchCapability:(UIForceTouchCapability)forceTouchCapability
traitCollectionContext:(id)traitCollectionContext
containerWindowSize:(CGSize)windowSize
{
self = [super init];
if (self) {
@ -30,7 +30,7 @@
_horizontalSizeClass = horizontalSizeClass;
_verticalSizeClass = verticalSizeClass;
_forceTouchCapability = forceTouchCapability;
_traitCollectionContext = traitCollectionContext;
_containerWindowSize = windowSize;
}
return self;
}
@ -40,29 +40,29 @@
horizontalSizeClass:(UIUserInterfaceSizeClass)horizontalSizeClass
verticalSizeClass:(UIUserInterfaceSizeClass)verticalSizeClass
forceTouchCapability:(UIForceTouchCapability)forceTouchCapability
traitCollectionContext:(id)traitCollectionContext
containerWindowSize:(CGSize)windowSize
{
return [[[self class] alloc] initWithDisplayScale:displayScale
userInterfaceIdiom:userInterfaceIdiom
horizontalSizeClass:horizontalSizeClass
verticalSizeClass:verticalSizeClass
forceTouchCapability:forceTouchCapability
traitCollectionContext:traitCollectionContext];
containerWindowSize:windowSize];
}
+ (ASTraitCollection *)traitCollectionWithASEnvironmentTraitCollection:(ASEnvironmentTraitCollection)traits
{
return [[[self class] alloc] initWithDisplayScale:traits.displayScale
userInterfaceIdiom:traits.userInterfaceIdiom
horizontalSizeClass:traits.horizontalSizeClass
verticalSizeClass:traits.verticalSizeClass
forceTouchCapability:traits.forceTouchCapability
traitCollectionContext:traits.displayContext];
return [[[self class] alloc] initWithDisplayScale:traits.displayScale
userInterfaceIdiom:traits.userInterfaceIdiom
horizontalSizeClass:traits.horizontalSizeClass
verticalSizeClass:traits.verticalSizeClass
forceTouchCapability:traits.forceTouchCapability
containerWindowSize:traits.containerWindowSize];
}
+ (ASTraitCollection *)traitCollectionWithUITraitCollection:(UITraitCollection *)traitCollection
traitCollectionContext:(id)traitCollectionContext
containerWindowSize:(CGSize)windowSize
{
ASTraitCollection *asyncTraitCollection = nil;
if (AS_AT_LEAST_IOS9) {
@ -71,7 +71,7 @@
horizontalSizeClass:traitCollection.horizontalSizeClass
verticalSizeClass:traitCollection.verticalSizeClass
forceTouchCapability:traitCollection.forceTouchCapability
traitCollectionContext:traitCollectionContext];
containerWindowSize:windowSize];
}
else if (AS_AT_LEAST_IOS8) {
asyncTraitCollection = [[[self class] alloc] initWithDisplayScale:traitCollection.displayScale
@ -79,7 +79,7 @@
horizontalSizeClass:traitCollection.horizontalSizeClass
verticalSizeClass:traitCollection.verticalSizeClass
forceTouchCapability:0
traitCollectionContext:traitCollectionContext];
containerWindowSize:windowSize];
} else {
asyncTraitCollection = [[[self class] alloc] init];
}
@ -95,7 +95,7 @@
.userInterfaceIdiom = self.userInterfaceIdiom,
.verticalSizeClass = self.verticalSizeClass,
.forceTouchCapability = self.forceTouchCapability,
.displayContext = self.traitCollectionContext,
.containerWindowSize = self.containerWindowSize,
};
}
@ -105,7 +105,7 @@
self.horizontalSizeClass == traitCollection.horizontalSizeClass &&
self.verticalSizeClass == traitCollection.verticalSizeClass &&
self.userInterfaceIdiom == traitCollection.userInterfaceIdiom &&
self.traitCollectionContext == traitCollection.traitCollectionContext &&
CGSizeEqualToSize(self.containerWindowSize, traitCollection.containerWindowSize) &&
self.forceTouchCapability == traitCollection.forceTouchCapability;
}

View File

@ -207,7 +207,7 @@ ASEnvironmentState ASEnvironmentMergeObjectAndState(ASEnvironmentState childEnvi
childTraitCollection.userInterfaceIdiom = parentTraitCollection.userInterfaceIdiom;
childTraitCollection.forceTouchCapability = parentTraitCollection.forceTouchCapability;
childTraitCollection.displayScale = parentTraitCollection.displayScale;
childTraitCollection.displayContext = parentTraitCollection.displayContext;
childTraitCollection.containerWindowSize = parentTraitCollection.containerWindowSize;
childEnvironmentState.environmentTraitCollection = childTraitCollection;
}