Fix missing images

Summary:
Under rare and as-yet-to-be determined circumstances, images can sometimes fail to load/download and get "stuck", without producing an error.

Because the `RCTNetworkTask` for these images is stuck in the "in progress" state, they clog up the RCTImageLoader task queue, which has a limit of 4 concurrent in-progress tasks.

This was previously masked by the fact that we automatically cancelled image requests when the RCTImageView moved offscreen, but we no longer do that.

This diff adds logic to detect some types of stuck task and remove them, thereby unblocking the queue. I've also restored the functionality of cancelling downloads for offscreen images (but not unloading the image itself) so that stuck images will be cancelled when you move to another screen, instead of using up space in the queue forever.

Reviewed By: fkgozali

Differential Revision: D3398105

fbshipit-source-id: 75ee40d06a872ae8e1cb57f02f9cad57c459143c

Author: nicklockwood

Libraries/Image/RCTImageLoader.m

@@ -67,7 +67,7 @@ - (void)setUp
   // Set defaults
   _maxConcurrentLoadingTasks = _maxConcurrentLoadingTasks ?: 4;
   _maxConcurrentDecodingTasks = _maxConcurrentDecodingTasks ?: 2;
-  _maxConcurrentDecodingBytes = _maxConcurrentDecodingBytes ?: 30 * 1024 *1024; // 30MB
+  _maxConcurrentDecodingBytes = _maxConcurrentDecodingBytes ?: 30 * 1024 * 1024; // 30MB
 
   _URLCacheQueue = dispatch_queue_create("com.facebook.react.ImageLoaderURLCacheQueue", DISPATCH_QUEUE_SERIAL);
 
@@ -241,8 +241,15 @@ - (void)dequeueTasks
           _activeTasks--;
           break;
         case RCTNetworkTaskPending:
+          break;
         case RCTNetworkTaskInProgress:
-          // Do nothing
+          // Check task isn't "stuck"
+          if (task.requestToken == nil) {
+            RCTLogWarn(@"Task orphaned for request %@", task.request);
+            [_pendingTasks removeObject:task];
+            _activeTasks--;
+            [task cancel];
+          }
           break;
       }
     }
@@ -412,6 +419,7 @@ - (RCTImageLoaderCancellationBlock)loadImageOrDataWithURLRequest:(NSURLRequest *
     RCTNetworkTask *task = [_bridge.networking networkTaskWithRequest:request completionBlock:^(NSURLResponse *response, NSData *data, NSError *error) {
       if (error) {
         completionHandler(error, nil);
+        [weakSelf dequeueTasks];
         return;
       }
 
@@ -429,7 +437,7 @@ - (RCTImageLoaderCancellationBlock)loadImageOrDataWithURLRequest:(NSURLRequest *
         // Process image data
         processResponse(response, data, nil);
 
-        //clean up
+        // Prepare for next task
         [weakSelf dequeueTasks];
 
       });
@@ -442,10 +450,7 @@ - (RCTImageLoaderCancellationBlock)loadImageOrDataWithURLRequest:(NSURLRequest *
     }
     if (task) {
       [_pendingTasks addObject:task];
-      if (MAX(_activeTasks, _scheduledDecodes) < _maxConcurrentLoadingTasks) {
-        [task start];
-        _activeTasks++;
-      }
+      [weakSelf dequeueTasks];
     }
 
     cancelLoad = ^{

Libraries/Image/RCTImageView.m

@@ -315,7 +315,13 @@ - (void)didMoveToWindow
 {
   [super didMoveToWindow];
 
-  if (self.window && (!self.image || self.image == _defaultImage)) {
+  if (!self.window) {
+    // Cancel loading the image if we've moved offscreen. In addition to helping
+    // pritoritise image requests that are actually on-screen, this removes
+    // requests that have gotten "stuck" from the queue, unblocking other images
+    // from loading.
+    [self cancelImageLoad];
+  } else if (!self.image || self.image == _defaultImage) {
     [self reloadImage];
   }
 }

Libraries/Network/RCTFileRequestHandler.m

@@ -49,7 +49,7 @@ - (NSOperation *)sendRequest:(NSURLRequest *)request
     NSError *error = nil;
     NSFileManager *fileManager = [NSFileManager new];
     NSDictionary<NSString *, id> *fileAttributes = [fileManager attributesOfItemAtPath:request.URL.path error:&error];
-    if (error) {
+    if (!fileAttributes) {
       [delegate URLRequest:weakOp didCompleteWithError:error];
       return;
     }

Libraries/Network/RCTNetworkTask.m

@@ -63,35 +63,40 @@ - (void)start
 
 - (void)cancel
 {
+  _status = RCTNetworkTaskFinished;
   __strong id strongToken = _requestToken;
   if (strongToken && [_handler respondsToSelector:@selector(cancelRequest:)]) {
     [_handler cancelRequest:strongToken];
   }
   [self invalidate];
-  _status = RCTNetworkTaskFinished;
 }
 
 - (BOOL)validateRequestToken:(id)requestToken
 {
+  BOOL valid = YES;
   if (_requestToken == nil) {
     if (requestToken == nil) {
-      return NO;
+      if (RCT_DEBUG) {
+        RCTLogError(@"Missing request token for request: %@", _request);
+      }
+      valid = NO;
     }
     _requestToken = requestToken;
-  }
-  if (![requestToken isEqual:_requestToken]) {
+  } else if (![requestToken isEqual:_requestToken]) {
     if (RCT_DEBUG) {
       RCTLogError(@"Unrecognized request token: %@ expected: %@", requestToken, _requestToken);
     }
+    valid = NO;
+  }
+  if (!valid) {
+    _status = RCTNetworkTaskFinished;
     if (_completionBlock) {
-      _completionBlock(_response, _data, [NSError errorWithDomain:RCTErrorDomain code:0
-        userInfo:@{NSLocalizedDescriptionKey: @"Unrecognized request token."}]);
+      _completionBlock(_response, nil, [NSError errorWithDomain:RCTErrorDomain code:0
+        userInfo:@{NSLocalizedDescriptionKey: @"Invalid request token."}]);
     }
     [self invalidate];
-    _status = RCTNetworkTaskFinished;
-    return NO;
   }
-  return YES;
+  return valid;
 }
 
 - (void)URLRequest:(id)requestToken didSendDataWithProgress:(int64_t)bytesSent
@@ -132,11 +137,11 @@ - (void)URLRequest:(id)requestToken didReceiveData:(NSData *)data
 - (void)URLRequest:(id)requestToken didCompleteWithError:(NSError *)error
 {
   if ([self validateRequestToken:requestToken]) {
+    _status = RCTNetworkTaskFinished;
     if (_completionBlock) {
       _completionBlock(_response, _data, error);
     }
     [self invalidate];
-    _status = RCTNetworkTaskFinished;
   }
 }