Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

Commit

Permalink
Caching code cleanup per code review feedback.
Browse files Browse the repository at this point in the history 
Simplify some things, handle edge cases, handle OSX.
  • Loading branch information
@kgoodier
kgoodier committed Dec 12, 2015
1 parent 4b9f633 commit 5e6d02c
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 51 deletions.
18 changes: 12 additions & 6 deletions SPDY/SPDYProtocol.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,15 +370,21 @@ - (NSCachedURLResponse *)loadCachedResponseIfAllowed
//
// This behavior may change in the future.

// version 7 and lower will be represented as 0
static NSInteger osVersion;
static BOOL osVersionNeedsHack;

This comment has been minimized.

Sign in to view

Copy link
@NSProgrammer

NSProgrammer Dec 12, 2015

Collaborator

nit: can we be much more explicit with the name? Like: osVersionRequiresManualLoadFromCacheHack
static dispatch_once_t once;
dispatch_once(&once, ^{
NSProcessInfo *processInfo = [NSProcessInfo processInfo];
if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
osVersion = [processInfo operatingSystemVersion].majorVersion;
NSOperatingSystemVersion osVersion = [processInfo operatingSystemVersion];
#if TARGET_OS_MAC
// 10.9 and earlier
osVersionNeedsHack = osVersion.majorVersion < 10 || (osVersion.majorVersion == 10 && osVersion.minorVersion <= 9);
#else
// iOS 7 and earlier
osVersionNeedsHack = osVersion.majorVersion < 8;
#endif
} else {
osVersion = 0;
osVersionNeedsHack = YES;
}
});

Expand All @@ -389,14 +395,14 @@ - (NSCachedURLResponse *)loadCachedResponseIfAllowed
NSURLSessionConfiguration *config = _associatedSession.configuration;
NSURLRequestCachePolicy cachePolicy = config.requestCachePolicy;
if (cachePolicy == NSURLRequestUseProtocolCachePolicy ||
(osVersion < 8 && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad))) {
(osVersionNeedsHack && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad))) {
return [config.URLCache cachedResponseForRequest:self.request];
}
} else {
// NSURLConnection on iOS 7 forces us to always load the cache item. But we don't want to
// do that for NSURLRequestUseProtocolCachePolicy.
NSURLRequestCachePolicy cachePolicy = self.request.cachePolicy;
if (osVersion < 8 && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad)) {
if (osVersionNeedsHack && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad)) {
return [[NSURLCache sharedURLCache] cachedResponseForRequest:self.request];
}
}
Expand Down
122 changes: 77 additions & 45 deletions SPDY/SPDYStream.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,13 +337,13 @@ - (void)_close
_metadata.blockedMs = _blockedElapsed * 1000;

// Manually make the willCacheResponse callback when needed
[self _storeCacheResponse];
[self _storeCacheResponseCompletion:^{
[_client URLProtocolDidFinishLoading:_protocol];

[_client URLProtocolDidFinishLoading:_protocol];

if (_delegate && [_delegate respondsToSelector:@selector(streamClosed:)]) {
[_delegate streamClosed:self];
}
if (_delegate && [_delegate respondsToSelector:@selector(streamClosed:)]) {
[_delegate streamClosed:self];
}
}];
}

- (bool)closed
Expand Down Expand Up @@ -597,19 +597,18 @@ - (void)didReceiveResponse
didReceiveResponse:_response
cacheStoragePolicy:_cachePolicy];

// Prepare for manual caching, if needed
NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
if (_cachePolicy != NSURLCacheStorageNotAllowed && // cacheable?
[self _shouldUseManualCaching] && // hack needed and NSURLSession used?
cache != nil && // cache configured (NSURLSession-specific)?
_local) { // not a push request?

// The NSURLCache has a heuristic to limit the max size of items based on the capacity of the
// cache. This is our attempt to mimic that behavior and prevent unlimited buffering of large
// responses. These numbers were found by manually experimenting and are only approximate.
// See SPDYNSURLCachingTest testResponseNearItemCacheSize_DoesUseCache.
_cacheDataBuffer = [[NSMutableData alloc] init];
_cacheMaxItemSize = MAX(cache.memoryCapacity * 0.05, cache.diskCapacity * 0.01);
// Prepare for manual caching, if allowed and needed
if (_cachePolicy != NSURLCacheStorageNotAllowed && [self _shouldUseManualCaching]) {
// Cache configured and not a push stream?
NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
if (cache != nil && _local) {
// The NSURLCache has a heuristic to limit the max size of items based on the capacity of the
// cache. This is our attempt to mimic that behavior and prevent unlimited buffering of large
// responses. These numbers were found by manually experimenting and are only approximate.
// See SPDYNSURLCachingTest testResponseNearItemCacheSize_DoesUseCache.
_cacheDataBuffer = [[NSMutableData alloc] init];
_cacheMaxItemSize = MAX(cache.memoryCapacity * 0.05, cache.diskCapacity * 0.05);
}
}
}

Expand Down Expand Up @@ -741,7 +740,7 @@ - (void)_didLoadDataChunk:(NSData *)data

if (_cacheDataBuffer) {
NSUInteger bufferSize = _cacheDataBuffer.length + data.length;
if (bufferSize < _cacheMaxItemSize) {
if (bufferSize <= _cacheMaxItemSize) {
[_cacheDataBuffer appendData:data];
} else {
// Throw away anything already buffered, it's going to be too big
Expand All @@ -755,19 +754,33 @@ - (void)_didLoadDataChunk:(NSData *)data
// ourselves, is turned on.
- (bool)_shouldUseManualCaching
{
NSInteger osVersion = 0;
NSProcessInfo *processInfo = [NSProcessInfo processInfo];
if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
osVersion = [processInfo operatingSystemVersion].majorVersion;
}
static BOOL osVersionNeedsHack;

This comment has been minimized.

Sign in to view

Copy link
@NSProgrammer

NSProgrammer Dec 12, 2015

Collaborator

osVersionRequiresManualStoreToCacheHack
static dispatch_once_t once;
dispatch_once(&once, ^{
NSProcessInfo *processInfo = [NSProcessInfo processInfo];
if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
NSOperatingSystemVersion osVersion = [processInfo operatingSystemVersion];
#if TARGET_OS_MAC
// 10.10 and earlier
osVersionNeedsHack = osVersion.majorVersion < 10 || (osVersion.majorVersion == 10 && osVersion.minorVersion <= 10);
#else
// iOS 8 and earlier
osVersionNeedsHack = osVersion.majorVersion <= 8;
#endif
} else {
osVersionNeedsHack = YES;
}
});

// iOS 8 and earlier and using NSURLSession
return (osVersion <= 8 && _protocol.associatedSession != nil && _protocol.associatedSessionTask != nil);
return (osVersionNeedsHack && _protocol.associatedSession != nil && _protocol.associatedSessionTask != nil);
}

- (void)_storeCacheResponse
- (void)_storeCacheResponseCompletion:(dispatch_block_t)completion
{
if (_cacheDataBuffer == nil) {
if (completion) {
completion();
}
return;
}

Expand All @@ -776,32 +789,51 @@ - (void)_storeCacheResponse
data:_cacheDataBuffer
userInfo:nil
storagePolicy:_cachePolicy];
NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
NSURLSession *session = _protocol.associatedSession;
NSURLSessionDataTask *dataTask = (NSURLSessionDataTask *)_protocol.associatedSessionTask;
NSURLCache *cache = session.configuration.URLCache;

// Already validated these items are not nil, just being safe
NSParameterAssert(session);
NSParameterAssert(cache);
NSParameterAssert(dataTask);

void (^finalCompletion)(NSCachedURLResponse *) = ^(NSCachedURLResponse *finalCachedResponse){
if (finalCachedResponse) {
if ([cache respondsToSelector:@selector(storeCachedResponse:forDataTask:)]) {
[cache storeCachedResponse:finalCachedResponse forDataTask:dataTask];
} else {
[cache storeCachedResponse:finalCachedResponse forRequest:_request];
}
}

if (completion) {
completion();
}
};

// Make "official" willCacheResponse callback to app, bypassing the NSURL loading system.
id<NSURLSessionDataDelegate> delegate = (id)_protocol.associatedSession.delegate;
id<NSURLSessionDataDelegate> delegate = (id)session.delegate;
if ([delegate respondsToSelector:@selector(URLSession:dataTask:willCacheResponse:completionHandler:)]) {
NSOperationQueue *queue = _protocol.associatedSession.delegateQueue;
CFRunLoopRef clientRunLoop = CFRunLoopGetCurrent();
CFRetain(clientRunLoop);

NSOperationQueue *queue = session.delegateQueue;
[(queue) ?: [NSOperationQueue mainQueue] addOperationWithBlock:^{
[delegate URLSession:_protocol.associatedSession dataTask:dataTask willCacheResponse:cachedResponse completionHandler:^(NSCachedURLResponse * cachedResponse) {
// This block may execute asynchronously at any time. No need to come back to the SPDY/NSURL thread
if (cachedResponse) {
if ([cache respondsToSelector:@selector(storeCachedResponse:forDataTask:)]) {
[cache storeCachedResponse:cachedResponse forDataTask:dataTask];
} else {
[cache storeCachedResponse:cachedResponse forRequest:_request];
}
}
[delegate URLSession:session dataTask:dataTask willCacheResponse:cachedResponse completionHandler:^(NSCachedURLResponse * cachedResponse) {

// Hop back to the NSURL thread and finish up
CFRunLoopPerformBlock(clientRunLoop, kCFRunLoopDefaultMode, ^{
finalCompletion(cachedResponse);
});
CFRunLoopWakeUp(clientRunLoop);
CFRelease(clientRunLoop);

}];
}];
} else {
// willCacheResponse delegate not implemented. Default behavior is to cache.
if ([cache respondsToSelector:@selector(storeCachedResponse:forDataTask:)]) {
[cache storeCachedResponse:cachedResponse forDataTask:dataTask];
} else {
[cache storeCachedResponse:cachedResponse forRequest:_request];
}
finalCompletion(cachedResponse);
}
}

Expand Down

0 comments on commit 5e6d02c

Please sign in to comment.