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

Commit a758509

Browse files
kgoodierkgoodier
committed
Caching code cleanup per code review feedback.
Simplify some things, handle edge cases, handle OSX.
1 parent 4b9f633 commit a758509

File tree

2 files changed

+89
-51
lines changed

2 files changed

+89
-51
lines changed

‎SPDY/SPDYProtocol.m‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -370,15 +370,21 @@ - (NSCachedURLResponse *)loadCachedResponseIfAllowed
370370
//
371371
// This behavior may change in the future.
372372

373-
// version 7 and lower will be represented as 0
374-
static NSInteger osVersion;
373+
static BOOL osVersionRequiresManualLoadFromCache;
375374
static dispatch_once_t once;
376375
dispatch_once(&once, ^{
377376
NSProcessInfo *processInfo = [NSProcessInfo processInfo];
378377
if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
379-
osVersion = [processInfo operatingSystemVersion].majorVersion;
378+
NSOperatingSystemVersion osVersion = [processInfo operatingSystemVersion];
379+
#if TARGET_OS_MAC
380+
// 10.9 and earlier
381+
osVersionRequiresManualLoadFromCache = osVersion.majorVersion < 10 || (osVersion.majorVersion == 10 && osVersion.minorVersion <= 9);
382+
#else
383+
// iOS 7 and earlier
384+
osVersionRequiresManualLoadFromCache = osVersion.majorVersion < 8;
385+
#endif
380386
} else {
381-
osVersion = 0;
387+
osVersionRequiresManualLoadFromCache = YES;
382388
}
383389
});
384390

@@ -389,14 +395,14 @@ - (NSCachedURLResponse *)loadCachedResponseIfAllowed
389395
NSURLSessionConfiguration *config = _associatedSession.configuration;
390396
NSURLRequestCachePolicy cachePolicy = config.requestCachePolicy;
391397
if (cachePolicy == NSURLRequestUseProtocolCachePolicy ||
392-
(osVersion < 8 && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad))) {
398+
(osVersionRequiresManualLoadFromCache && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad))) {
393399
return [config.URLCache cachedResponseForRequest:self.request];
394400
}
395401
} else {
396402
// NSURLConnection on iOS 7 forces us to always load the cache item. But we don't want to
397403
// do that for NSURLRequestUseProtocolCachePolicy.
398404
NSURLRequestCachePolicy cachePolicy = self.request.cachePolicy;
399-
if (osVersion < 8 && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad)) {
405+
if (osVersionRequiresManualLoadFromCache && (cachePolicy == NSURLRequestReturnCacheDataDontLoad || cachePolicy == NSURLRequestReturnCacheDataElseLoad)) {
400406
return [[NSURLCache sharedURLCache] cachedResponseForRequest:self.request];
401407
}
402408
}

‎SPDY/SPDYStream.m‎

Lines changed: 77 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -337,13 +337,13 @@ - (void)_close
337337
_metadata.blockedMs = _blockedElapsed * 1000;
338338

339339
// Manually make the willCacheResponse callback when needed
340-
[self _storeCacheResponse];
340+
[self _storeCacheResponseCompletion:^{
341+
[_client URLProtocolDidFinishLoading:_protocol];
341342

342-
[_client URLProtocolDidFinishLoading:_protocol];
343-
344-
if (_delegate && [_delegate respondsToSelector:@selector(streamClosed:)]) {
345-
[_delegate streamClosed:self];
346-
}
343+
if (_delegate && [_delegate respondsToSelector:@selector(streamClosed:)]) {
344+
[_delegate streamClosed:self];
345+
}
346+
}];
347347
}
348348

349349
- (bool)closed
@@ -597,19 +597,18 @@ - (void)didReceiveResponse
597597
didReceiveResponse:_response
598598
cacheStoragePolicy:_cachePolicy];
599599

600-
// Prepare for manual caching, if needed
601-
NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
602-
if (_cachePolicy != NSURLCacheStorageNotAllowed && // cacheable?
603-
[self _shouldUseManualCaching] && // hack needed and NSURLSession used?
604-
cache != nil && // cache configured (NSURLSession-specific)?
605-
_local) { // not a push request?
606-
607-
// The NSURLCache has a heuristic to limit the max size of items based on the capacity of the
608-
// cache. This is our attempt to mimic that behavior and prevent unlimited buffering of large
609-
// responses. These numbers were found by manually experimenting and are only approximate.
610-
// See SPDYNSURLCachingTest testResponseNearItemCacheSize_DoesUseCache.
611-
_cacheDataBuffer = [[NSMutableData alloc] init];
612-
_cacheMaxItemSize = MAX(cache.memoryCapacity * 0.05, cache.diskCapacity * 0.01);
600+
// Prepare for manual caching, if allowed and needed
601+
if (_cachePolicy != NSURLCacheStorageNotAllowed && [self _shouldUseManualCaching]) {
602+
// Cache configured and not a push stream?
603+
NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
604+
if (cache != nil && _local) {
605+
// The NSURLCache has a heuristic to limit the max size of items based on the capacity of the
606+
// cache. This is our attempt to mimic that behavior and prevent unlimited buffering of large
607+
// responses. These numbers were found by manually experimenting and are only approximate.
608+
// See SPDYNSURLCachingTest testResponseNearItemCacheSize_DoesUseCache.
609+
_cacheDataBuffer = [[NSMutableData alloc] init];
610+
_cacheMaxItemSize = MAX(cache.memoryCapacity * 0.05, cache.diskCapacity * 0.05);
611+
}
613612
}
614613
}
615614

@@ -741,7 +740,7 @@ - (void)_didLoadDataChunk:(NSData *)data
741740

742741
if (_cacheDataBuffer) {
743742
NSUInteger bufferSize = _cacheDataBuffer.length + data.length;
744-
if (bufferSize < _cacheMaxItemSize) {
743+
if (bufferSize <= _cacheMaxItemSize) {
745744
[_cacheDataBuffer appendData:data];
746745
} else {
747746
// Throw away anything already buffered, it's going to be too big
@@ -755,19 +754,33 @@ - (void)_didLoadDataChunk:(NSData *)data
755754
// ourselves, is turned on.
756755
- (bool)_shouldUseManualCaching
757756
{
758-
NSInteger osVersion = 0;
759-
NSProcessInfo *processInfo = [NSProcessInfo processInfo];
760-
if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
761-
osVersion = [processInfo operatingSystemVersion].majorVersion;
762-
}
757+
static BOOL osVersionRequiresManualStoreToCache;
758+
static dispatch_once_t once;
759+
dispatch_once(&once, ^{
760+
NSProcessInfo *processInfo = [NSProcessInfo processInfo];
761+
if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
762+
NSOperatingSystemVersion osVersion = [processInfo operatingSystemVersion];
763+
#if TARGET_OS_MAC
764+
// 10.10 and earlier
765+
osVersionRequiresManualStoreToCache = osVersion.majorVersion < 10 || (osVersion.majorVersion == 10 && osVersion.minorVersion <= 10);
766+
#else
767+
// iOS 8 and earlier
768+
osVersionRequiresManualStoreToCache = osVersion.majorVersion <= 8;
769+
#endif
770+
} else {
771+
osVersionRequiresManualStoreToCache = YES;
772+
}
773+
});
763774

764-
// iOS 8 and earlier and using NSURLSession
765-
return (osVersion <= 8 && _protocol.associatedSession != nil && _protocol.associatedSessionTask != nil);
775+
return (osVersionRequiresManualStoreToCache && _protocol.associatedSession != nil && _protocol.associatedSessionTask != nil);
766776
}
767777

768-
- (void)_storeCacheResponse
778+
- (void)_storeCacheResponseCompletion:(dispatch_block_t)completion
769779
{
770780
if (_cacheDataBuffer == nil) {
781+
if (completion) {
782+
completion();
783+
}
771784
return;
772785
}
773786

@@ -776,32 +789,51 @@ - (void)_storeCacheResponse
776789
data:_cacheDataBuffer
777790
userInfo:nil
778791
storagePolicy:_cachePolicy];
779-
NSURLCache *cache = _protocol.associatedSession.configuration.URLCache;
792+
NSURLSession *session = _protocol.associatedSession;
780793
NSURLSessionDataTask *dataTask = (NSURLSessionDataTask *)_protocol.associatedSessionTask;
794+
NSURLCache *cache = session.configuration.URLCache;
795+
796+
// Already validated these items are not nil, just being safe
797+
NSParameterAssert(session);
798+
NSParameterAssert(cache);
799+
NSParameterAssert(dataTask);
800+
801+
void (^finalCompletion)(NSCachedURLResponse *) = ^(NSCachedURLResponse *finalCachedResponse){
802+
if (finalCachedResponse) {
803+
if ([cache respondsToSelector:@selector(storeCachedResponse:forDataTask:)]) {
804+
[cache storeCachedResponse:finalCachedResponse forDataTask:dataTask];
805+
} else {
806+
[cache storeCachedResponse:finalCachedResponse forRequest:_request];
807+
}
808+
}
809+
810+
if (completion) {
811+
completion();
812+
}
813+
};
781814

782815
// Make "official" willCacheResponse callback to app, bypassing the NSURL loading system.
783-
id<NSURLSessionDataDelegate> delegate = (id)_protocol.associatedSession.delegate;
816+
id<NSURLSessionDataDelegate> delegate = (id)session.delegate;
784817
if ([delegate respondsToSelector:@selector(URLSession:dataTask:willCacheResponse:completionHandler:)]) {
785-
NSOperationQueue *queue = _protocol.associatedSession.delegateQueue;
818+
CFRunLoopRef clientRunLoop = CFRunLoopGetCurrent();
819+
CFRetain(clientRunLoop);
820+
821+
NSOperationQueue *queue = session.delegateQueue;
786822
[(queue) ?: [NSOperationQueue mainQueue] addOperationWithBlock:^{
787-
[delegate URLSession:_protocol.associatedSession dataTask:dataTask willCacheResponse:cachedResponse completionHandler:^(NSCachedURLResponse * cachedResponse) {
788-
// This block may execute asynchronously at any time. No need to come back to the SPDY/NSURL thread
789-
if (cachedResponse) {
790-
if ([cache respondsToSelector:@selector(storeCachedResponse:forDataTask:)]) {
791-
[cache storeCachedResponse:cachedResponse forDataTask:dataTask];
792-
} else {
793-
[cache storeCachedResponse:cachedResponse forRequest:_request];
794-
}
795-
}
823+
[delegate URLSession:session dataTask:dataTask willCacheResponse:cachedResponse completionHandler:^(NSCachedURLResponse * cachedResponse) {
824+
825+
// Hop back to the NSURL thread and finish up
826+
CFRunLoopPerformBlock(clientRunLoop, kCFRunLoopDefaultMode, ^{
827+
finalCompletion(cachedResponse);
828+
});
829+
CFRunLoopWakeUp(clientRunLoop);
830+
CFRelease(clientRunLoop);
831+
796832
}];
797833
}];
798834
} else {
799835
// willCacheResponse delegate not implemented. Default behavior is to cache.
800-
if ([cache respondsToSelector:@selector(storeCachedResponse:forDataTask:)]) {
801-
[cache storeCachedResponse:cachedResponse forDataTask:dataTask];
802-
} else {
803-
[cache storeCachedResponse:cachedResponse forRequest:_request];
804-
}
836+
finalCompletion(cachedResponse);
805837
}
806838
}
807839

0 commit comments

Comments
 (0)