Changeset 196345 in webkit


Ignore:
Timestamp:
Feb 9, 2016 4:15:58 PM (8 years ago)
Author:
jer.noble@apple.com
Message:

[Mac] Graphical corruption in videos when enabling custom loading path
https://bugs.webkit.org/show_bug.cgi?id=154044

Reviewed by Alex Christensen.

The NSOperationQueue provided by AVFoundation from the AVAssetResourceLoader queue is not
set to be a serial queue. So when adding dataReceived operations to that queue, there exists
the possibility that some operations are handled before others, and the client will receieve
data out of order.

A real NSURLSession object will only issue another operation when the first operation
completes, so emulate this behavior in WebCoreNSURLSession by using a serial dispatch queue.
The internal queue will enqueue an operation to the resource loader's queue, and block until
that operation completes, thus ensuring ordering of the data (and other) operations.

  • platform/network/cocoa/WebCoreNSURLSession.h:
  • platform/network/cocoa/WebCoreNSURLSession.mm:

(-[WebCoreNSURLSession initWithResourceLoader:delegate:delegateQueue:]): Initialize _internalQueue
(-[WebCoreNSURLSession addDelegateOperation:]): Added utility method.
(-[WebCoreNSURLSession taskCompleted:]): Call -addDelegateOperation:
(-[WebCoreNSURLSession finishTasksAndInvalidate]): Ditto.
(-[WebCoreNSURLSession resetWithCompletionHandler:]): Ditto.
(-[WebCoreNSURLSession flushWithCompletionHandler:]): Ditto.
(-[WebCoreNSURLSession getTasksWithCompletionHandler:]): Ditto.
(-[WebCoreNSURLSession getAllTasksWithCompletionHandler:]): Ditto.
(-[WebCoreNSURLSessionDataTask resource:receivedResponse:]): Ditto.
(-[WebCoreNSURLSessionDataTask resource:receivedData:length:]): Ditto.
(-[WebCoreNSURLSessionDataTask resourceFinished:]): Ditto.

Drive-by fix:
(-[WebCoreNSURLSessionDataTask resource:receivedData:length:]): Set countOfBytesReceived outside the operation,

queue, matching NSURLSessionDataTask's behavior.

Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r196339 r196345  
     12016-02-09  Jer Noble  <jer.noble@apple.com>
     2
     3        [Mac] Graphical corruption in videos when enabling custom loading path
     4        https://bugs.webkit.org/show_bug.cgi?id=154044
     5
     6        Reviewed by Alex Christensen.
     7
     8        The NSOperationQueue provided by AVFoundation from the AVAssetResourceLoader queue is not
     9        set to be a serial queue. So when adding dataReceived operations to that queue, there exists
     10        the possibility that some operations are handled before others, and the client will receieve
     11        data out of order.
     12
     13        A real NSURLSession object will only issue another operation when the first operation
     14        completes, so emulate this behavior in WebCoreNSURLSession by using a serial dispatch queue.
     15        The internal queue will enqueue an operation to the resource loader's queue, and block until
     16        that operation completes, thus ensuring ordering of the data (and other) operations.
     17
     18        * platform/network/cocoa/WebCoreNSURLSession.h:
     19        * platform/network/cocoa/WebCoreNSURLSession.mm:
     20        (-[WebCoreNSURLSession initWithResourceLoader:delegate:delegateQueue:]): Initialize _internalQueue
     21        (-[WebCoreNSURLSession addDelegateOperation:]): Added utility method.
     22        (-[WebCoreNSURLSession taskCompleted:]): Call -addDelegateOperation:
     23        (-[WebCoreNSURLSession finishTasksAndInvalidate]): Ditto.
     24        (-[WebCoreNSURLSession resetWithCompletionHandler:]): Ditto.
     25        (-[WebCoreNSURLSession flushWithCompletionHandler:]): Ditto.
     26        (-[WebCoreNSURLSession getTasksWithCompletionHandler:]): Ditto.
     27        (-[WebCoreNSURLSession getAllTasksWithCompletionHandler:]): Ditto.
     28        (-[WebCoreNSURLSessionDataTask resource:receivedResponse:]): Ditto.
     29        (-[WebCoreNSURLSessionDataTask resource:receivedData:length:]): Ditto.
     30        (-[WebCoreNSURLSessionDataTask resourceFinished:]): Ditto.
     31
     32        Drive-by fix:
     33        (-[WebCoreNSURLSessionDataTask resource:receivedData:length:]): Set countOfBytesReceived outside the operation,
     34            queue, matching NSURLSessionDataTask's behavior.
     35
    1362016-02-09  Nan Wang  <n_wang@apple.com>
    237
  • trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.h

    r196092 r196345  
    3333#import <Foundation/NSURLSession.h>
    3434#import <wtf/HashSet.h>
     35#import <wtf/OSObjectPtr.h>
    3536#import <wtf/RefPtr.h>
    3637#import <wtf/RetainPtr.h>
     
    5960    BOOL _invalidated;
    6061    NSUInteger _nextTaskIdentifier;
     62    OSObjectPtr<dispatch_queue_t> _internalQueue;
    6163}
    6264- (id)initWithResourceLoader:(WebCore::CachedResourceLoader&)loader delegate:(id<NSURLSessionTaskDelegate>)delegate delegateQueue:(NSOperationQueue*)queue;
  • trunk/Source/WebCore/platform/network/cocoa/WebCoreNSURLSession.mm

    r196092 r196345  
    4444@property (readwrite, retain) id<NSURLSessionTaskDelegate> delegate;
    4545- (void)taskCompleted:(WebCoreNSURLSessionDataTask *)task;
     46- (void)addDelegateOperation:(void (^)(void))operation;
    4647@end
    4748
     
    7778    _queue = inQueue ? inQueue : [NSOperationQueue mainQueue];
    7879    _invalidated = NO;
     80    _internalQueue = adoptOSObject(dispatch_queue_create("WebCoreNSURLSession _internalQueue", DISPATCH_QUEUE_SERIAL));
     81
    7982    return self;
    8083}
     
    103106
    104107    RetainPtr<WebCoreNSURLSession> strongSelf { self };
    105     [self.delegateQueue addOperationWithBlock:[strongSelf] {
     108    [self addDelegateOperation:[strongSelf] {
    106109        if ([strongSelf.get().delegate respondsToSelector:@selector(URLSession:didBecomeInvalidWithError:)])
    107110            [strongSelf.get().delegate URLSession:(NSURLSession *)strongSelf.get() didBecomeInvalidWithError:nil];
    108111    }];
     112}
     113
     114- (void)addDelegateOperation:(void (^)(void))block
     115{
     116    RetainPtr<WebCoreNSURLSession> strongSelf { self };
     117    RetainPtr<NSBlockOperation> operation = [NSBlockOperation blockOperationWithBlock:block];
     118    dispatch_async(_internalQueue.get(), [strongSelf, operation] {
     119        [strongSelf.get().delegateQueue addOperation:operation.get()];
     120        [operation waitUntilFinished];
     121    });
    109122}
    110123
     
    147160
    148161    RetainPtr<WebCoreNSURLSession> strongSelf { self };
    149     [self.delegateQueue addOperationWithBlock:[strongSelf] {
     162    [self addDelegateOperation:[strongSelf] {
    150163        if ([strongSelf.get().delegate respondsToSelector:@selector(URLSession:didBecomeInvalidWithError:)])
    151164            [strongSelf.get().delegate URLSession:(NSURLSession *)strongSelf.get() didBecomeInvalidWithError:nil];
     
    164177{
    165178    // FIXME: This cannot currently be implemented. We cannot guarantee that the next connection will happen on a new socket.
    166     [self.delegateQueue addOperationWithBlock:completionHandler];
     179    [self addDelegateOperation:completionHandler];
    167180}
    168181
     
    170183{
    171184    // FIXME: This cannot currently be implemented. We cannot guarantee that the next connection will happen on a new socket.
    172     [self.delegateQueue addOperationWithBlock:completionHandler];
     185    [self addDelegateOperation:completionHandler];
    173186}
    174187
     
    178191    for (auto& task : _dataTasks)
    179192        [array addObject:task.get()];
    180     [self.delegateQueue addOperationWithBlock:^{
     193    [self addDelegateOperation:^{
    181194        completionHandler(array, nil, nil);
    182195    }];
     
    188201    for (auto& task : _dataTasks)
    189202        [array addObject:task.get()];
    190     [self.delegateQueue addOperationWithBlock:^{
     203    [self addDelegateOperation:^{
    191204        completionHandler(array);
    192205    }];
     
    470483    [self _setDefersLoading:YES];
    471484    RetainPtr<WebCoreNSURLSessionDataTask> strongSelf { self };
    472     [self.session.delegateQueue addOperationWithBlock:[strongSelf] {
     485    [self.session addDelegateOperation:[strongSelf] {
    473486        id<NSURLSessionDataDelegate> dataDelegate = (id<NSURLSessionDataDelegate>)strongSelf.get().session.delegate;
    474487        if (![dataDelegate respondsToSelector:@selector(URLSession:dataTask:didReceiveResponse:completionHandler:)]) {
     
    501514    // e.g., RetainPtr<CFDataRef> cfData = resource->resourceBuffer()->createCFData();
    502515
     516    self.countOfBytesReceived += length;
     517
    503518    RetainPtr<NSData> nsData = adoptNS([[NSData alloc] initWithBytes:data length:length]);
    504519    RetainPtr<WebCoreNSURLSessionDataTask> strongSelf { self };
    505     [self.session.delegateQueue addOperationWithBlock:[strongSelf, length, nsData] {
    506         strongSelf.get().countOfBytesReceived += length;
     520    [self.session addDelegateOperation:[strongSelf, length, nsData] {
    507521        id<NSURLSessionDataDelegate> dataDelegate = (id<NSURLSessionDataDelegate>)strongSelf.get().session.delegate;
    508522        if ([dataDelegate respondsToSelector:@selector(URLSession:dataTask:didReceiveData:)])
     
    530544
    531545    RetainPtr<WebCoreNSURLSessionDataTask> strongSelf { self };
    532     [self.session.delegateQueue addOperationWithBlock:[strongSelf] {
     546    [self.session addDelegateOperation:[strongSelf] {
    533547        id<NSURLSessionTaskDelegate> delegate = (id<NSURLSessionTaskDelegate>)strongSelf.get().session.delegate;
    534548        if ([delegate respondsToSelector:@selector(URLSession:task:didCompleteWithError:)])
Note: See TracChangeset for help on using the changeset viewer.