[ASNodeController] Add -nodeDidLayout callback. Allow switching retain behavior at runtime. (#470)

With these changes, I'd also like to propose that we move ASNodeController
out of Beta (renaming the files without +Beta). Let me know what you think!

Because we don't support ASNodeController directly in ASCV / ASTV, it is still
important to allow flipping the ownership in certain cases (in particular, for
root CellNodeController objects that should follow the lifecycle of the
ASCellNode rather than owning the ASCellNode).

CHANGELOG.md

@@ -1,6 +1,7 @@
 ## master
 
 * Add your own contributions to the next release on the line below this with your name.
+- [ASNodeController] Add -nodeDidLayout callback. Allow switching retain behavior at runtime. [Scott Goodson](https://github.com/appleguy)
 - [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)

Source/ASDisplayNode+Subclasses.h

@@ -96,6 +96,13 @@ NS_ASSUME_NONNULL_BEGIN
  */
 - (void)didExitPreloadState;
 
+/**
+ * @abstract Called when the node has completed applying the layout.
+ * @discussion Can be used for operations that are performed after layout has completed.
+ * @note This method is guaranteed to be called on main.
+ */
+- (void)nodeDidLayout;
+
 @end
 
 @interface ASDisplayNode (Subclassing) <ASInterfaceStateDelegate>

Source/ASDisplayNode.mm

@@ -916,7 +916,9 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
     if (_transitionID != ASLayoutElementContextInvalidTransitionID) {
       return;
     }
-    
+
+    as_activity_create_for_scope("-[ASDisplayNode __layout]");
+
     // 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];
@@ -1122,6 +1124,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
   ASDisplayNodeAssertMainThread();
   ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
   ASDisplayNodeAssertTrue(self.isNodeLoaded);
+  [_interfaceStateDelegate nodeDidLayout];
 }
 
 #pragma mark Layout Transition

Source/ASNodeController+Beta.h

@@ -18,18 +18,15 @@
 #import <AsyncDisplayKit/ASDisplayNode.h>
 #import <AsyncDisplayKit/ASDisplayNode+Subclasses.h> // for ASInterfaceState protocol
 
-// Until an ASNodeController can be provided in place of an ASCellNode, some apps may prefer to have
-// nodes keep their controllers alive (and a weak reference from controller to node)
-#define INVERT_NODE_CONTROLLER_OWNERSHIP 0
-
 /* ASNodeController is currently beta and open to change in the future */
 @interface ASNodeController<__covariant DisplayNodeType : ASDisplayNode *> : NSObject <ASInterfaceStateDelegate>
 
-#if INVERT_NODE_CONTROLLER_OWNERSHIP
-@property (nonatomic, weak) DisplayNodeType node;
-#else
-@property (nonatomic, strong) DisplayNodeType node;
-#endif
+@property (nonatomic, strong /* may be weak! */) DisplayNodeType node;
+
+// Until an ASNodeController can be provided in place of an ASCellNode, some apps may prefer to have
+// nodes keep their controllers alive (and a weak reference from controller to node)
+
+@property (nonatomic, assign) BOOL shouldInvertStrongReference;
 
 - (void)loadNode;
 
@@ -47,3 +44,9 @@
                       fromState:(ASInterfaceState)oldState ASDISPLAYNODE_REQUIRES_SUPER;
 
 @end
+
+@interface ASDisplayNode (ASNodeController)
+
+@property(nonatomic, readonly) ASNodeController *nodeController;
+
+@end

Source/ASNodeController+Beta.m

@@ -15,34 +15,28 @@
 //      http://www.apache.org/licenses/LICENSE-2.0
 //
 
-#import "ASNodeController+Beta.h"
-#import "ASDisplayNode+FrameworkPrivate.h"
+#import <AsyncDisplayKit/ASWeakProxy.h>
+#import <AsyncDisplayKit/ASNodeController+Beta.h>
+#import <AsyncDisplayKit/ASDisplayNode+FrameworkPrivate.h>
 
-#if INVERT_NODE_CONTROLLER_OWNERSHIP
+#define _node (_shouldInvertStrongReference ? _weakNode : _strongNode)
 
-@interface ASDisplayNode (ASNodeController)
-@property (nonatomic, strong) ASNodeController *asdkNodeController;
-@end
+@interface ASDisplayNode (ASNodeControllerOwnership)
 
-@implementation ASDisplayNode (ASNodeController)
+// This property exists for debugging purposes. Don't use __nodeController in production code.
+@property (nonatomic, readonly) ASNodeController *__nodeController;
 
-- (ASNodeController *)asdkNodeController
-{
-  return objc_getAssociatedObject(self, @selector(asdkNodeController));
-}
-
-- (void)setAsdkNodeController:(ASNodeController *)asdkNodeController
-{
-  objc_setAssociatedObject(self, @selector(asdkNodeController), asdkNodeController, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
-}
+// These setters are mutually exclusive. Setting one will clear the relationship of the other.
+- (void)__setNodeControllerStrong:(ASNodeController *)nodeController;
+- (void)__setNodeControllerWeak:(ASNodeController *)nodeController;
 
 @end
 
-#endif
-
 @implementation ASNodeController
-
-@synthesize node = _node;
+{
+  ASDisplayNode *_strongNode;
+  __weak ASDisplayNode *_weakNode;
+}
 
 - (instancetype)init
 {
@@ -66,16 +60,41 @@
   return _node;
 }
 
--(void)setNode:(ASDisplayNode *)node
+- (void)setupReferencesWithNode:(ASDisplayNode *)node
 {
-  _node = node;
-  _node.interfaceStateDelegate = self;
-#if INVERT_NODE_CONTROLLER_OWNERSHIP
-  _node.asdkNodeController = self;
-#endif
+  if (_shouldInvertStrongReference) {
+    // The node should own the controller; weak reference from controller to node.
+    _weakNode = node;
+    [node __setNodeControllerStrong:self];
+    _strongNode = nil;
+  } else {
+    // The controller should own the node; weak reference from node to controller.
+    _strongNode = node;
+    [node __setNodeControllerWeak:self];
+    _weakNode = nil;
+  }
+
+  node.interfaceStateDelegate = self;
+}
+
+- (void)setNode:(ASDisplayNode *)node
+{
+  [self setupReferencesWithNode:node];
+}
+
+- (void)setShouldInvertStrongReference:(BOOL)shouldInvertStrongReference
+{
+  if (_shouldInvertStrongReference != shouldInvertStrongReference) {
+    // Because the BOOL controls which ivar we access, get the node before toggling.
+    ASDisplayNode *node = _node;
+    _shouldInvertStrongReference = shouldInvertStrongReference;
+    [self setupReferencesWithNode:node];
+  }
 }
 
 // subclass overrides
+- (void)nodeDidLayout {}
+
 - (void)didEnterVisibleState {}
 - (void)didExitVisibleState  {}
 
@@ -89,3 +108,41 @@
                       fromState:(ASInterfaceState)oldState {}
 
 @end
+
+@implementation ASDisplayNode (ASNodeControllerOwnership)
+
+- (ASNodeController *)__nodeController
+{
+  ASNodeController *nodeController = nil;
+  id object = objc_getAssociatedObject(self, @selector(__nodeController));
+
+  if ([object isKindOfClass:[ASWeakProxy class]]) {
+    nodeController = (ASNodeController *)[(ASWeakProxy *)object target];
+  } else {
+    nodeController = (ASNodeController *)object;
+  }
+
+  return nodeController;
+}
+
+- (void)__setNodeControllerStrong:(ASNodeController *)nodeController
+{
+  objc_setAssociatedObject(self, @selector(__nodeController), nodeController, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+}
+
+- (void)__setNodeControllerWeak:(ASNodeController *)nodeController
+{
+  // Associated objects don't support weak references. Since assign can become a dangling pointer, use ASWeakProxy.
+  ASWeakProxy *nodeControllerProxy = [ASWeakProxy weakProxyWithTarget:nodeController];
+  objc_setAssociatedObject(self, @selector(__nodeController), nodeControllerProxy, OBJC_ASSOCIATION_RETAIN_NONATOMIC);
+}
+
+@end
+
+@implementation ASDisplayNode (ASNodeController)
+
+- (ASNodeController *)nodeController {
+  return self.__nodeController;
+}
+
+@end