Address comments

TextureGroup · Mar 7, 2019 · d7413c8 · d7413c8
1 parent 6a1f368
commit d7413c8
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 29 deletions.
diff --git a/Source/ASMultiplexImageNode.mm b/Source/ASMultiplexImageNode.mm
@@ -96,7 +96,6 @@ @interface ASMultiplexImageNode ()
 
   //set on init only
   BOOL _cacheSupportsClearing;
-  BOOL _isInImageDownloaderPriorityExperiment;
 }
 
 //! @abstract Read-write redeclaration of property declared in ASMultiplexImageNode.h.
@@ -184,8 +183,6 @@ - (instancetype)initWithCache:(id<ASImageCacheProtocol>)cache downloader:(id<ASI
 
   _shouldRenderProgressImages = YES;
 
-  _isInImageDownloaderPriorityExperiment = ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority);
-
   self.shouldBypassEnsureDisplay = YES;
 
   return self;
@@ -302,7 +299,7 @@ - (void)didExitVisibleState
 - (void)didExitDisplayState
 {
   [super didExitDisplayState];
-  if (_isInImageDownloaderPriorityExperiment) {
+  if (ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority)) {
     [self _updatePriorityOnDownloaderIfNeededWithDefaultPriority:ASImageDownloaderPriorityPreload];
   }
 }
@@ -488,20 +485,19 @@ - (UIImage *)_bestImmediatelyAvailableImageFromDataSource:(id *)imageIdentifierO
 - (void)_updatePriorityOnDownloaderIfNeededWithDefaultPriority:(ASImageDownloaderPriority)defaultPriority
 {
   ASAssertUnlocked(_downloadIdentifierLock);
+
   if (_downloaderFlags.downloaderImplementsSetPriority) {
-    id downloadIdentifier;
-    {
-      ASDN::MutexLocker l(_downloadIdentifierLock);
-      downloadIdentifier = _downloadIdentifier;
-    }
+    // Read our interface state before locking so that we don't lock super while holding our lock.
+    ASInterfaceState interfaceState = self.interfaceState;
+    ASDN::MutexLocker l(_downloadIdentifierLock);
 
-    if (downloadIdentifier != nil) {
+    if (_downloadIdentifier != nil) {
       ASImageDownloaderPriority priority = defaultPriority;
-      if (_isInImageDownloaderPriorityExperiment) {
-        priority = ASImageDownloaderPriorityWithInterfaceState(self.interfaceState);
+      if (ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority)) {
+        priority = ASImageDownloaderPriorityWithInterfaceState(interfaceState);
       }
 
-      [_downloader setPriority:priority withDownloadIdentifier:downloadIdentifier];
+      [_downloader setPriority:priority withDownloadIdentifier:_downloadIdentifier];
     }
   }
 }
@@ -875,7 +871,7 @@ - (void)_downloadImageWithIdentifier:(id)imageIdentifier URL:(NSURL *)imageURL c
 
     id downloadIdentifier;
     if (strongSelf->_downloaderFlags.downloaderImplementsDownloadWithPriority
-        && strongSelf->_isInImageDownloaderPriorityExperiment) {
+        && ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority)) {
 
       /*
        Decide a priority based on the current interface state of this node.

diff --git a/Source/ASNetworkImageNode.mm b/Source/ASNetworkImageNode.mm
@@ -47,7 +47,6 @@ @interface ASNetworkImageNode ()
   CGFloat _currentImageQuality;
   CGFloat _renderedImageQuality;
   BOOL _shouldRenderProgressImages;
-  BOOL _isInImageDownloaderPriorityExperiment;
 
   struct {
     unsigned int delegateWillStartDisplayAsynchronously:1;
@@ -107,7 +106,6 @@ - (instancetype)initWithCache:(id<ASImageCacheProtocol>)cache downloader:(id<ASI
 
   _shouldCacheImage = YES;
   _shouldRenderProgressImages = YES;
-  _isInImageDownloaderPriorityExperiment = ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority);
   self.shouldBypassEnsureDisplay = YES;
 
   return self;
@@ -397,7 +395,7 @@ - (void)didExitVisibleState
 - (void)didExitDisplayState
 {
   [super didExitDisplayState];
-  if (_isInImageDownloaderPriorityExperiment) {
+  if (ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority)) {
     [self _updatePriorityOnDownloaderIfNeededWithDefaultPriority:ASImageDownloaderPriorityPreload];
   }
 }
@@ -451,21 +449,15 @@ - (void)handleProgressImage:(UIImage *)progressImage progress:(CGFloat)progress
 - (void)_updatePriorityOnDownloaderIfNeededWithDefaultPriority:(ASImageDownloaderPriority)defaultPriority
 {
   if (_downloaderFlags.downloaderImplementsSetPriority) {
-    id downloadIdentifier;
-    ASInterfaceState interfaceState;
-    {
-      ASLockScopeSelf();
-      downloadIdentifier = _downloadIdentifier;
-      interfaceState = _interfaceState;
-    }
+    ASLockScopeSelf();
 
-    if (downloadIdentifier != nil) {
+    if (_downloadIdentifier != nil) {
       ASImageDownloaderPriority priority = defaultPriority;
-      if (_isInImageDownloaderPriorityExperiment) {
-        priority = ASImageDownloaderPriorityWithInterfaceState(interfaceState);
+      if (ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority)) {
+        priority = ASImageDownloaderPriorityWithInterfaceState(_interfaceState);
       }
 
-      [_downloader setPriority:priority withDownloadIdentifier:downloadIdentifier];
+      [_downloader setPriority:priority withDownloadIdentifier:_downloadIdentifier];
     }
   }
 }
@@ -607,7 +599,7 @@ - (void)_downloadImageWithCompletion:(void (^)(id <ASImageContainerProtocol> ima
     };
 
     if (self->_downloaderFlags.downloaderImplementsDownloadWithPriority
-        && self->_isInImageDownloaderPriorityExperiment) {
+        && ASActivateExperimentalFeature(ASExperimentalImageDownloaderPriority)) {
       /*
         Decide a priority based on the current interface state of this node.
         It can happen that this method was called when the node entered preload state