New runloop queue to coalesce Interface state update calls. (#788)

* fix SIMULATE_WEB_RESPONSE not imported #449 * Coalesce interface state updates to ASCATransactionQueue before CATransaction commit. This will avoid duplicate interface state delegate calls caused by view repeatly added/removed to/from hierarchy during controller animation transition. * fix tests for new run loop queue * Support for disabling ASCATransactionQueue * Fix didExitHierarchy to use ASCATransactionQueue. * merge range managed and none range managed for didExitHierarchy * Revert "merge range managed and none range managed for didExitHierarchy" This reverts commit f807efa. * merge range managed and none range managed for didExitHierarchy * remove metadata * abstract queue to impl class methods * Add tests * Fix test fail because of shared object. * guard _pendingInterfaceState access with lock * name refactor * Refactor from comments https://github.com/TextureGroup/Texture/pull/788/\#pullrequestreview-94849919 * Apply InterfaceState immediately after ASCATranactionQueue is processed and before next runloop started. * refactor * no op to start CI build * remove unused var and kick off tests * change lisence * remove code for weak ref * add change log and adjust license
TextureGroup · Feb 13, 2018 · 2618c50 · 2618c50
1 parent 6f34691
commit 2618c50
Show file tree

Hide file tree

Showing 13 changed files with 445 additions and 66 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,6 @@
 ## master
 * Add your own contributions to the next release on the line below this with your name.
+- [ASRunloopQueue] Introduce new runloop queue(ASCATransactionQueue) to coalesce Interface state update calls for view controller transitions.
 - [ASRangeController] Fix stability of "minimum" rangeMode if the app has more than one layout before scrolling.
 - **Important** ASDisplayNode's cornerRadius is a new thread-safe bridged property that should be preferred over CALayer's. Use the latter at your own risk! [Huy Nguyen](https://github.com/nguyenhuy) [#749](https://github.com/TextureGroup/Texture/pull/749).
 - [ASCellNode] Adds mapping for UITableViewCell focusStyle [Alex Hill](https://github.com/alexhillc) [#727](https://github.com/TextureGroup/Texture/pull/727)

diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -63,7 +63,7 @@
 // We have to forward declare the protocol as this place otherwise it will not compile compiling with an Base SDK < iOS 10
 @protocol CALayerDelegate;
 
-@interface ASDisplayNode () <UIGestureRecognizerDelegate, CALayerDelegate, _ASDisplayLayerDelegate>
+@interface ASDisplayNode () <UIGestureRecognizerDelegate, CALayerDelegate, _ASDisplayLayerDelegate, ASCATransactionQueueObserving>
 
 /**
  * See ASDisplayNodeInternal.h for ivars
@@ -2739,7 +2739,7 @@ - (void)setHierarchyState:(ASHierarchyState)newState
   // Entered or exited range managed state.
   if ((newState & ASHierarchyStateRangeManaged) != (oldState & ASHierarchyStateRangeManaged)) {
     if (newState & ASHierarchyStateRangeManaged) {
-      [self enterInterfaceState:self.supernode.interfaceState];
+      [self enterInterfaceState:self.supernode.pendingInterfaceState];
     } else {
       // The case of exiting a range-managed state should be fairly rare.  Adding or removing the node
       // to a view hierarchy will cause its interfaceState to be either fully set or unset (all fields),
@@ -2782,30 +2782,34 @@ - (void)didExitHierarchy
   ASDisplayNodeAssert(_flags.isExitingHierarchy, @"You should never call -didExitHierarchy directly. Appearance is automatically managed by ASDisplayNode");
   ASDisplayNodeAssert(!_flags.isEnteringHierarchy, @"ASDisplayNode inconsistency. __enterHierarchy and __exitHierarchy are mutually exclusive");
   ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
-
-  if (![self supportsRangeManagedInterfaceState]) {
-    self.interfaceState = ASInterfaceStateNone;
-  } else {
-    // This case is important when tearing down hierarchies.  We must deliver a visibileStateDidChange:NO callback, as part our API guarantee that this method can be used for
-    // things like data analytics about user content viewing.  We cannot call the method in the dealloc as any incidental retain operations in client code would fail.
-    // Additionally, it may be that a Standard UIView which is containing us is moving between hierarchies, and we should not send the call if we will be re-added in the
-    // same runloop.  Strategy: strong reference (might be the last!), wait one runloop, and confirm we are still outside the hierarchy (both layer-backed and view-backed).
-    // TODO: This approach could be optimized by only performing the dispatch for root elements + recursively apply the interface state change. This would require a closer
-    // integration with _ASDisplayLayer to ensure that the superlayer pointer has been cleared by this stage (to check if we are root or not), or a different delegate call.
-
-    if (ASInterfaceStateIncludesVisible(self.interfaceState)) {
-      dispatch_async(dispatch_get_main_queue(), ^{
-        // This block intentionally retains self.
-        __instanceLock__.lock();
-          unsigned isInHierarchy = _flags.isInHierarchy;
-          BOOL isVisible = ASInterfaceStateIncludesVisible(_interfaceState);
-          ASInterfaceState newState = (_interfaceState & ~ASInterfaceStateVisible);
-        __instanceLock__.unlock();
-
-        if (!isInHierarchy && isVisible) {
-          self.interfaceState = newState;
+
+  // This case is important when tearing down hierarchies.  We must deliver a visibileStateDidChange:NO callback, as part our API guarantee that this method can be used for
+  // things like data analytics about user content viewing.  We cannot call the method in the dealloc as any incidental retain operations in client code would fail.
+  // Additionally, it may be that a Standard UIView which is containing us is moving between hierarchies, and we should not send the call if we will be re-added in the
+  // same runloop.  Strategy: strong reference (might be the last!), wait one runloop, and confirm we are still outside the hierarchy (both layer-backed and view-backed).
+  // TODO: This approach could be optimized by only performing the dispatch for root elements + recursively apply the interface state change. This would require a closer
+  // integration with _ASDisplayLayer to ensure that the superlayer pointer has been cleared by this stage (to check if we are root or not), or a different delegate call.
+  if (ASInterfaceStateIncludesVisible(_pendingInterfaceState)) {
+    void(^exitVisibleInterfaceState)(void) = ^{
+      // This block intentionally retains self.
+      __instanceLock__.lock();
+      unsigned isStillInHierarchy = _flags.isInHierarchy;
+      BOOL isVisible = ASInterfaceStateIncludesVisible(_pendingInterfaceState);
+      ASInterfaceState newState = (_pendingInterfaceState & ~ASInterfaceStateVisible);
+      __instanceLock__.unlock();
+
+      if (!isStillInHierarchy && isVisible) {
+        if (![self supportsRangeManagedInterfaceState]) {
+          newState = ASInterfaceStateNone;
         }
-      });
+        self.interfaceState = newState;
+      }
+    };
+
+    if ([[ASCATransactionQueue sharedQueue] disabled]) {
+      dispatch_async(dispatch_get_main_queue(), exitVisibleInterfaceState);
+    } else {
+      exitVisibleInterfaceState();
     }
   }
 }
@@ -2866,25 +2870,53 @@ - (ASInterfaceState)interfaceState
 }
 
 - (void)setInterfaceState:(ASInterfaceState)newState
+{
+  if ([[ASCATransactionQueue sharedQueue] disabled]) {
+    [self applyPendingInterfaceState:newState];
+  } else {
+    ASDN::MutexLocker l(__instanceLock__);
+    if (_pendingInterfaceState != newState) {
+      _pendingInterfaceState = newState;
+      [[ASCATransactionQueue sharedQueue] enqueue:self];
+    }
+  }
+}
+
+- (ASInterfaceState)pendingInterfaceState
+{
+  ASDN::MutexLocker l(__instanceLock__);
+  return _pendingInterfaceState;
+}
+
+- (void)applyPendingInterfaceState:(ASInterfaceState)newPendingState
 {
   //This method is currently called on the main thread. The assert has been added here because all of the
   //did(Enter|Exit)(Display|Visible|Preload)State methods currently guarantee calling on main.
   ASDisplayNodeAssertMainThread();
-  // It should never be possible for a node to be visible but not be allowed / expected to display.
-  ASDisplayNodeAssertFalse(ASInterfaceStateIncludesVisible(newState) && !ASInterfaceStateIncludesDisplay(newState));
+
   // This method manages __instanceLock__ itself, to ensure the lock is not held while didEnter/Exit(.*)State methods are called, thus avoid potential deadlocks
   ASDisplayNodeAssertLockUnownedByCurrentThread(__instanceLock__);
 
   ASInterfaceState oldState = ASInterfaceStateNone;
+  ASInterfaceState newState = ASInterfaceStateNone;
   {
     ASDN::MutexLocker l(__instanceLock__);
-    if (_interfaceState == newState) {
-      return;
+    // newPendingState will not be used when ASCATransactionQueue is enabled
+    // and use _pendingInterfaceState instead for interfaceState update.
+    if ([[ASCATransactionQueue sharedQueue] disabled]) {
+      _pendingInterfaceState = newPendingState;
     }
     oldState = _interfaceState;
+    newState = _pendingInterfaceState;
+    if (newState == oldState) {
+      return;
+    }
     _interfaceState = newState;
   }
 
+  // It should never be possible for a node to be visible but not be allowed / expected to display.
+  ASDisplayNodeAssertFalse(ASInterfaceStateIncludesVisible(newState) && !ASInterfaceStateIncludesDisplay(newState));
+
   // TODO: Trigger asynchronous measurement if it is not already cached or being calculated.
   // if ((newState & ASInterfaceStateMeasureLayout) != (oldState & ASInterfaceStateMeasureLayout)) {
   // }
@@ -2981,6 +3013,12 @@ - (void)setInterfaceState:(ASInterfaceState)newState
   [self interfaceStateDidChange:newState fromState:oldState];
 }
 
+- (void)prepareForCATransactionCommit
+{
+  // Apply _pendingInterfaceState actual _interfaceState, note that ASInterfaceStateNone is not used.
+  [self applyPendingInterfaceState:ASInterfaceStateNone];
+}
+
 - (void)interfaceStateDidChange:(ASInterfaceState)newState fromState:(ASInterfaceState)oldState
 {
   // Subclass hook

diff --git a/Source/ASRunLoopQueue.h b/Source/ASRunLoopQueue.h
@@ -20,8 +20,15 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+@protocol ASCATransactionQueueObserving <NSObject>
+- (void)prepareForCATransactionCommit;
+@end
+
+@interface ASAbstractRunLoopQueue : NSObject
+@end
+
 AS_SUBCLASSING_RESTRICTED
-@interface ASRunLoopQueue<ObjectType> : NSObject <NSLocking>
+@interface ASRunLoopQueue<ObjectType> : ASAbstractRunLoopQueue <NSLocking>
 
 /**
  * Create a new queue with the given run loop and handler.
@@ -41,13 +48,37 @@ AS_SUBCLASSING_RESTRICTED
 
 - (void)enqueue:(ObjectType)object;
 
-@property (nonatomic, readonly) BOOL isEmpty;
+@property (atomic, readonly) BOOL isEmpty;
 
 @property (nonatomic, assign) NSUInteger batchSize;           // Default == 1.
 @property (nonatomic, assign) BOOL ensureExclusiveMembership; // Default == YES.  Set-like behavior.
 
 @end
 
+AS_SUBCLASSING_RESTRICTED
+@interface ASCATransactionQueue : ASAbstractRunLoopQueue
+
+@property (atomic, readonly) BOOL isEmpty;
+@property (atomic, readonly) BOOL disabled;
+/**
+ * The queue to run on main run loop before CATransaction commit.
+ *
+ * @discussion this queue will run after ASRunLoopQueue and before CATransaction commit
+ * to get last chance of updating/coalesce info like interface state.
+ * Each node will only be called once per transaction commit to reflect interface change.
+ */
+@property (class, atomic, readonly) ASCATransactionQueue *sharedQueue;
+
+- (void)enqueue:(id<ASCATransactionQueueObserving>)object;
+
+/**
+ * @abstract Apply a node's interfaceState immediately rather than adding to the queue.
+ */
+- (void)disable;
+
+@end
+
+
 AS_SUBCLASSING_RESTRICTED
 @interface ASDeallocQueue : NSObject