-
Notifications
You must be signed in to change notification settings - Fork 514
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Refactors downloading and fixes download start time. (#360)
* Refactors downloading and fixes download start time. Download start time was being incorrectly computed since the timer was started when the image was added to the queue instead of when the image was actually started downloading. When attempting to address this, I decided to do some much needed cleanup and refactoring by moving much of the download work to PINRemoteImageDownloadTask * A bit more cleanup and fix a locking error * Fixed crash on NSURLSessionTask subclasses (dealloc) * Update Changelog.md * Rename methods to indicate locking * s/_locked_/l_ * Address feedback from @Adlai-Holler * Whoops removed too much. * Fix merge conflict
- Loading branch information
1 parent
ee315d2
commit 3781e4b
Showing
17 changed files
with
743 additions
and
354 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| // | ||
| // NSURLSessionTask+Timing.h | ||
| // PINRemoteImage | ||
| // | ||
| // Created by Garrett Moon on 5/19/17. | ||
| // Copyright © 2017 Pinterest. All rights reserved. | ||
| // | ||
|
|
||
| #import <Foundation/Foundation.h> | ||
|
|
||
| @interface NSURLSessionTask (Timing) | ||
|
|
||
| - (void)PIN_setupSessionTaskObserver; | ||
| - (CFTimeInterval)PIN_startTime; | ||
| - (CFTimeInterval)PIN_endTime; | ||
|
|
||
| @end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| // | ||
| // NSURLSessionTask+Timing.m | ||
| // PINRemoteImage | ||
| // | ||
| // Created by Garrett Moon on 5/19/17. | ||
| // Copyright © 2017 Pinterest. All rights reserved. | ||
| // | ||
|
|
||
| #import "NSURLSessionTask+Timing.h" | ||
|
|
||
| #import <objc/runtime.h> | ||
| #import <QuartzCore/QuartzCore.h> | ||
|
|
||
| @interface PINURLSessionTaskObserver : NSObject | ||
|
|
||
| @property (atomic, assign) CFTimeInterval startTime; | ||
| @property (atomic, assign) CFTimeInterval endTime; | ||
|
|
||
| - (instancetype)init NS_UNAVAILABLE; | ||
| - (instancetype)initWithTask:(NSURLSessionTask *)task NS_DESIGNATED_INITIALIZER; | ||
|
|
||
| @end | ||
|
|
||
| @implementation PINURLSessionTaskObserver | ||
|
|
||
| - (instancetype)initWithTask:(NSURLSessionTask *)task | ||
| { | ||
| if (self = [super init]) { | ||
| _startTime = 0; | ||
| _endTime = 0; | ||
| [task addObserver:self forKeyPath:@"state" options:0 context:nil]; | ||
| } | ||
| return self; | ||
| } | ||
|
|
||
| - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey,id> *)change context:(void *)context | ||
| { | ||
| NSURLSessionTask *task = (NSURLSessionTask *)object; | ||
| switch (task.state) { | ||
| case NSURLSessionTaskStateRunning: | ||
| if (self.startTime == 0) { | ||
| self.startTime = CACurrentMediaTime(); | ||
| } | ||
| break; | ||
|
|
||
| case NSURLSessionTaskStateCompleted: | ||
| NSAssert(self.startTime != 0, @"Expect that task was started before it's completed."); | ||
| if (self.endTime == 0) { | ||
| self.endTime = CACurrentMediaTime(); | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| @end | ||
|
|
||
| @implementation NSURLSessionTask (Additions) | ||
|
|
||
| - (void)PIN_setupSessionTaskObserver | ||
| { | ||
| // It's necessary to swizzle dealloc here to remove the observer :( | ||
| // I wasn't able to figure out another way; kvo assertion about observed objects being observed occurs | ||
| // *before* associated objects are dealloc'd | ||
| static dispatch_once_t onceToken; | ||
| dispatch_once(&onceToken, ^{ | ||
| SEL deallocSelector = NSSelectorFromString(@"dealloc"); | ||
| Method deallocMethod = class_getInstanceMethod([NSURLSessionTask class], deallocSelector); | ||
| IMP originalImplementation = method_getImplementation(deallocMethod); | ||
| IMP newImplementation = imp_implementationWithBlock(^(void *obj){ | ||
| @autoreleasepool { | ||
| //remove state observer | ||
| PINURLSessionTaskObserver *observer = objc_getAssociatedObject((__bridge id)obj, @selector(PIN_setupSessionTaskObserver)); | ||
| if (observer) { | ||
| [(__bridge id)obj removeObserver:observer forKeyPath:@"state"]; | ||
| } | ||
| } | ||
|
|
||
| //casting original implementation is necessary to ensure ARC doesn't attempt to retain during dealloc | ||
| ((void (*)(void *, SEL))originalImplementation)(obj, deallocSelector); | ||
| }); | ||
| class_replaceMethod([NSURLSessionTask class], deallocSelector, newImplementation, method_getTypeEncoding(deallocMethod)); | ||
| }); | ||
|
|
||
| PINURLSessionTaskObserver *observer = [[PINURLSessionTaskObserver alloc] initWithTask:self]; | ||
| objc_setAssociatedObject(self, @selector(PIN_setupSessionTaskObserver), observer, OBJC_ASSOCIATION_RETAIN); | ||
| } | ||
|
|
||
| - (CFTimeInterval)PIN_startTime | ||
| { | ||
| PINURLSessionTaskObserver *observer = objc_getAssociatedObject(self, @selector(PIN_setupSessionTaskObserver)); | ||
| NSAssert(observer != nil, @"setupSessionTaskObserver should have been called before."); | ||
| if (observer == nil) { | ||
| return 0; | ||
| } | ||
| return observer.startTime; | ||
| } | ||
|
|
||
| - (CFTimeInterval)PIN_endTime | ||
| { | ||
| PINURLSessionTaskObserver *observer = objc_getAssociatedObject(self, @selector(PIN_setupSessionTaskObserver)); | ||
| NSAssert(observer != nil, @"setupSessionTaskObserver should have been called before."); | ||
| if (observer == nil) { | ||
| return 0; | ||
| } | ||
| return observer.endTime; | ||
| } | ||
|
|
||
| @end |
20 changes: 20 additions & 0 deletions
20
Source/Classes/Categories/PINRemoteImageTask+Subclassing.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| // | ||
| // PINRemoteImageTask+Subclassing.h | ||
| // PINRemoteImage | ||
| // | ||
| // Created by Garrett Moon on 5/22/17. | ||
| // Copyright © 2017 Pinterest. All rights reserved. | ||
| // | ||
|
|
||
| #import "PINRemoteImageTask.h" | ||
|
|
||
| NS_ASSUME_NONNULL_BEGIN | ||
|
|
||
| @interface PINRemoteImageTask (Subclassing) | ||
|
|
||
| - (NSMutableDictionary *)l_callbackBlocks; | ||
| - (BOOL)l_cancelWithUUID:(NSUUID *)UUID resume:(PINResume * _Nullable * _Nullable)resume; | ||
|
|
||
| @end | ||
|
|
||
| NS_ASSUME_NONNULL_END |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.