Wrap transaction operation retain cycle fix in an experiment (#1438)

* Wrap transaction operation retain cycle fix in an experiment This is a follow on to #1429. * fix typo in experiment name. fix copyright in tests file.
TextureGroup · Apr 1, 2019 · 442317b · 442317b
1 parent 19e1a34
commit 442317b
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 13 deletions.
diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h
@@ -33,6 +33,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
   ASExperimentalTextDrawing = 1 << 12,                      // exp_text_drawing
   ASExperimentalFixRangeController = 1 << 13,               // exp_fix_range_controller
   ASExperimentalOOMBackgroundDeallocDisable = 1 << 14,      // exp_oom_bg_dealloc_disable
+  ASExperimentalTransactionOperationRetainCycle = 1 << 15,   // exp_transaction_operation_retain_cycle
   ASExperimentalFeatureAll = 0xFFFFFFFF
 };
 

diff --git a/Source/ASExperimentalFeatures.mm b/Source/ASExperimentalFeatures.mm
@@ -26,7 +26,8 @@
                                       @"exp_image_downloader_priority",
                                       @"exp_text_drawing",
                                       @"exp_fix_range_controller",
-                                      @"exp_oom_bg_dealloc_disable"]));
+                                      @"exp_oom_bg_dealloc_disable",
+                                      @"exp_transaction_operation_retain_cycle"]));
   if (flags == ASExperimentalFeatureAll) {
     return allNames;
   }

diff --git a/Source/Details/Transactions/_ASAsyncTransactionContainer.mm b/Source/Details/Transactions/_ASAsyncTransactionContainer.mm
@@ -10,6 +10,7 @@
 #import <AsyncDisplayKit/_ASAsyncTransactionContainer.h>
 #import <AsyncDisplayKit/_ASAsyncTransactionContainer+Private.h>
 
+#import <AsyncDisplayKit/ASConfigurationInternal.h>
 #import <AsyncDisplayKit/_ASAsyncTransaction.h>
 #import <AsyncDisplayKit/_ASAsyncTransactionGroup.h>
 
@@ -54,14 +55,25 @@ - (_ASAsyncTransaction *)asyncdisplaykit_asyncTransaction
       self.asyncdisplaykit_asyncLayerTransactions = transactions;
     }
     __weak CALayer *weakSelf = self;
-    transaction = [[_ASAsyncTransaction alloc] initWithCompletionBlock:^(_ASAsyncTransaction *completedTransaction, BOOL cancelled) {
-      __strong CALayer *self = weakSelf;
-      if (self == nil) {
-        return;
-      }
-      [self.asyncdisplaykit_asyncLayerTransactions removeObject:completedTransaction];
-      [self asyncdisplaykit_asyncTransactionContainerDidCompleteTransaction:completedTransaction];
-    }];
+    if (ASActivateExperimentalFeature(ASExperimentalTransactionOperationRetainCycle)) {
+      transaction = [[_ASAsyncTransaction alloc] initWithCompletionBlock:^(_ASAsyncTransaction *completedTransaction, BOOL cancelled) {
+        __strong CALayer *self = weakSelf;
+        if (self == nil) {
+          return;
+        }
+        [self.asyncdisplaykit_asyncLayerTransactions removeObject:completedTransaction];
+        [self asyncdisplaykit_asyncTransactionContainerDidCompleteTransaction:completedTransaction];
+      }];
+    } else {
+      transaction = [[_ASAsyncTransaction alloc] initWithCompletionBlock:^(_ASAsyncTransaction *completedTransaction, BOOL cancelled) {
+        __strong CALayer *self = weakSelf;
+        if (self == nil) {
+          return;
+        }
+        [transactions removeObject:completedTransaction];
+        [self asyncdisplaykit_asyncTransactionContainerDidCompleteTransaction:completedTransaction];
+      }];
+    }
     [transactions addObject:transaction];
     self.asyncdisplaykit_currentAsyncTransaction = transaction;
     [self asyncdisplaykit_asyncTransactionContainerWillBeginTransaction:transaction];

diff --git a/Tests/ASConfigurationTests.mm b/Tests/ASConfigurationTests.mm
@@ -32,7 +32,8 @@
   ASExperimentalImageDownloaderPriority,
   ASExperimentalTextDrawing,
   ASExperimentalFixRangeController,
-  ASExperimentalOOMBackgroundDeallocDisable
+  ASExperimentalOOMBackgroundDeallocDisable,
+  ASExperimentalTransactionOperationRetainCycle,
 };
 
 @interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
@@ -59,7 +60,8 @@ + (NSArray *)names {
     @"exp_image_downloader_priority",
     @"exp_text_drawing",
     @"exp_fix_range_controller",
-    @"exp_oom_bg_dealloc_disable"
+    @"exp_oom_bg_dealloc_disable",
+    @"exp_transaction_operation_retain_cycle"
   ];
 }
 

diff --git a/Tests/ASTransactionTests.mm b/Tests/ASTransactionTests.mm
@@ -2,8 +2,8 @@
 //  ASTransactionTests.m
 //  AsyncDisplayKitTests
 //
-//  Created by Greg Bolsinga on 3/26/19.
-//  Copyright © 2019 Pinterest. All rights reserved.
+//  Copyright (c) Pinterest, Inc.  All rights reserved.
+//  Licensed under Apache 2.0: http://www.apache.org/licenses/LICENSE-2.0
 //
 
 #import "ASTestCase.h"
@@ -52,8 +52,41 @@ - (void)testWeakWhenCancelled
   XCTAssertNil(weakTransaction);
 }
 
+- (void)testWeakWithSingleOperation_noExperiment
+{
+  __weak _ASAsyncTransaction* weakTransaction = nil;
+  @autoreleasepool {
+    CALayer *layer = [[CALayer alloc] init];
+    _ASAsyncTransaction *transaction = layer.asyncdisplaykit_asyncTransaction;
+
+    [transaction addOperationWithBlock:^id<NSObject> _Nullable{
+      return nil;
+    } priority:1
+                                 queue:dispatch_get_main_queue()
+                            completion:^(id  _Nullable value, BOOL canceled) {
+                              ;
+                            }];
+
+    weakTransaction = transaction;
+    layer = nil;
+  }
+
+  // held by main transaction group
+  XCTAssertNotNil(weakTransaction);
+
+  // run so that transaction group drains.
+  static NSTimeInterval delay = 0.1;
+  [[NSRunLoop currentRunLoop] runUntilDate:[NSDate dateWithTimeIntervalSinceNow:delay]];
+
+  XCTAssertNotNil(weakTransaction); // Oops! Experiment will fix this.
+}
+
 - (void)testWeakWithSingleOperation
 {
+  ASConfiguration *config = [[ASConfiguration alloc] initWithDictionary:nil];
+  config.experimentalFeatures = ASExperimentalTransactionOperationRetainCycle;
+  [ASConfigurationManager test_resetWithConfiguration:config];
+
   __weak _ASAsyncTransaction* weakTransaction = nil;
   @autoreleasepool {
     CALayer *layer = [[CALayer alloc] init];