Fix bug in ASRangeController that causes some cell nodes of a collect…

…ion view which is about to becomes invisible to load their backing view/layer and render (facebookarchive#1418)
digitalegarage · Mar 22, 2019 · 09365b9 · 09365b9
1 parent 74de765
commit 09365b9
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 16 deletions.
diff --git a/Schemas/configuration.json b/Schemas/configuration.json
@@ -25,7 +25,8 @@
                     "exp_disable_a11y_cache",
                     "exp_dispatch_apply",
                     "exp_image_downloader_priority",
-                    "exp_text_drawing"
+                    "exp_text_drawing",
+                    "exp_fix_range_controller"
                 ]
     		}
 		}

diff --git a/Source/ASExperimentalFeatures.h b/Source/ASExperimentalFeatures.h
@@ -31,6 +31,7 @@ typedef NS_OPTIONS(NSUInteger, ASExperimentalFeatures) {
   ASExperimentalDispatchApply = 1 << 10,                    // exp_dispatch_apply
   ASExperimentalImageDownloaderPriority = 1 << 11,          // exp_image_downloader_priority
   ASExperimentalTextDrawing = 1 << 12,                      // exp_text_drawing
+  ASExperimentalFixRangeController = 1 << 13,               // exp_fix_range_controller
   ASExperimentalFeatureAll = 0xFFFFFFFF
 };
 

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

diff --git a/Source/Details/ASRangeController.mm b/Source/Details/ASRangeController.mm
@@ -349,21 +349,25 @@ - (void)_updateVisibleNodeIndexPaths
         }
       }
     } else {
-      // If selfInterfaceState isn't visible, then visibleIndexPaths represents what /will/ be immediately visible at the
-      // instant we come onscreen.  So, preload and display all of those things, but don't waste resources preloading yet.
-      // We handle this as a separate case to minimize set operations for offscreen preloading, including containsObject:.
+      // If selfInterfaceState isn't visible, then visibleIndexPaths represents either what /will/ be immediately visible at the
+      // instant we come onscreen, or what /will/ no longer be visible at the instant we come offscreen.
+      // So, preload and display all of those things, but don't waste resources preloading others.
+      // We handle this as a separate case to minimize set operations, including -containsObject:.
+      //
+      // DO NOT set Visible: even though these elements are in the visible range / "viewport",
+      // our overall container object is itself not yet, or no longer, visible.
+      // The moment it becomes visible, we will run the condition above.
+
+      BOOL shouldUpdateInterfaceState = NO;
+      if (ASActivateExperimentalFeature(ASExperimentalFixRangeController)) {
+        shouldUpdateInterfaceState = [visibleIndexPaths containsObject:indexPath];
+      } else {
+        shouldUpdateInterfaceState = [allCurrentIndexPaths containsObject:indexPath];
+      }
 
-      if ([allCurrentIndexPaths containsObject:indexPath]) {
-        // DO NOT set Visible: even though these elements are in the visible range / "viewport",
-        // our overall container object is itself not visible yet.  The moment it becomes visible, we will run the condition above
-
-        // Set Layout, Preload
+      if (shouldUpdateInterfaceState) {
         interfaceState |= ASInterfaceStatePreload;
-
         if (rangeMode != ASLayoutRangeModeLowMemory) {
-          // Add Display.
-          // We might be looking at an indexPath that was previously in-range, but now we need to clear it.
-          // In that case we'll just set it back to MeasureLayout.  Only set Display | Preload if in allCurrentIndexPaths.
           interfaceState |= ASInterfaceStateDisplay;
         }
       }

diff --git a/Tests/ASConfigurationTests.mm b/Tests/ASConfigurationTests.mm
@@ -30,7 +30,8 @@
   ASExperimentalDisableAccessibilityCache,
   ASExperimentalDispatchApply,
   ASExperimentalImageDownloaderPriority,
-  ASExperimentalTextDrawing
+  ASExperimentalTextDrawing,
+  ASExperimentalFixRangeController
 };
 
 @interface ASConfigurationTests : ASTestCase <ASConfigurationDelegate>
@@ -55,7 +56,8 @@ + (NSArray *)names {
     @"exp_disable_a11y_cache",
     @"exp_dispatch_apply",
     @"exp_image_downloader_priority",
-    @"exp_text_drawing"
+    @"exp_text_drawing",
+    @"exp_fix_range_controller"
   ];
 }