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

Commit

Permalink
Fix caching on iOS 7
Browse files Browse the repository at this point in the history
On iOS 7, the protocol is required to *always* load its own cache item,
even for the NSURLRequestReturnCacheDataDontLoad and
NSURLRequestReturnCacheDataElseLoad policies. This applies to both
NSURLConnection and NSURLSession.

Found by unit testing on iOS 7 simulator with XCode 6.4.
  • Loading branch information
kgoodier committed Dec 11, 2015
1 parent 2a2c2f8 commit 4b9f633
Showing 1 changed file with 32 additions and 6 deletions.
38 changes: 32 additions & 6 deletions SPDY/SPDYProtocol.m
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,15 @@ - (NSCachedURLResponse *)loadCachedResponseIfAllowed
// We're making some choices here to limit the surface area of caching, given we don't yet
// have a fully-featured client caching implementation (missing sufficient validity checks).
//
// For the default request cache policy (NSURLRequestUseProtocolCachePolicy), we have to load
// the cache ourselves. We're applying the following rules in that case:
// On iOS 8 and 9, the NSURL loading system will supply the cached item to our constructor
// for NSURLRequestReturnCacheDataElseLoad and NSURLRequestReturnCacheDataDontLoad. For
// NSURLRequestUseProtocolCachePolicy, we have to load it ourselves.
//
// On iOS 7, the NSURL loading system never supplies the cached item to our constructor.
// We have to load it ourselves for NSURLRequestReturnCacheDataElseLoad,
// NSURLRequestReturnCacheDataDontLoad, and NSURLRequestUseProtocolCachePolicy.

// For NSURLRequestUseProtocolCachePolicy, we're applying the following rules regarding loading:
// - NSURLConnection-based requests will not support caching.
// - NSURLSession-based requests must set the SPDYURLSession property on the request, and
// must provide a NSURLCache in their NSURLSessionConfiguration. There is no fallback to
Expand All @@ -363,19 +370,38 @@ - (NSCachedURLResponse *)loadCachedResponseIfAllowed
//
// This behavior may change in the future.

NSCachedURLResponse *response;
// version 7 and lower will be represented as 0
static NSInteger osVersion;
static dispatch_once_t once;
dispatch_once(&once, ^{
NSProcessInfo *processInfo = [NSProcessInfo processInfo];
if ([processInfo respondsToSelector:@selector(operatingSystemVersion)]) {
osVersion = [processInfo operatingSystemVersion].majorVersion;

This comment has been minimized.

Copy link
@NSProgrammer

NSProgrammer Dec 11, 2015

Collaborator

this isn't considerate of Mac OS X support

This comment has been minimized.

Copy link
@kgoodier

kgoodier Dec 11, 2015

Author Contributor

done

} else {
osVersion = 0;
}
});

BOOL isNSURLSession = (_associatedSession != nil ||
_associatedSessionTask != nil ||
([self respondsToSelector:@selector(task)] && self.task != nil));
if (isNSURLSession) {

This comment has been minimized.

Copy link
@NSProgrammer

NSProgrammer Dec 11, 2015

Collaborator

the if else code below looks to have a lot of redundancy, can we clean it up?

BOOL isNSURLSession = ...;
NSURLCache *loadingCache = nil;
NSURLRequestCachePolicy cachePolicy = NSURLRequestUseProtocolCachePolicy;
if (isNSURLSession) {
    NSURLSessionConfiguration *config = _associatedSession.configuration;
    cachePolicy = config.requestCachePolicy;
    if (NSURLRequestUseProtocolCachePolicy == cachePolicy) {
        cachePolicy = NSURLRequestReturnCacheDataElseLoad;
    }
    loadingCache = config.URLCache;
} else {
    cachePolicy = self.request.cachePolicy;
    if (NSURLRequestUseProtocolCachePolicy == cachePolicy) {
        cachePolicy = NSURLRequestReloadIgnoringCacheData;
    }
    loadingCache = [NSURLCache sharedURLCache];
}

if (loadingCache && osVersion < 8 /* though this needs to consider Mac OS X */) {
    switch (cachePolicy) {
        NSURLRequestReturnCacheDataElseLoad:
        NSURLRequestReturnCacheDataDontLoad:
            return [loadingCache cachedResponseForRequest:self.request];
        default:
            break;
    }
}

This comment has been minimized.

Copy link
@kgoodier

kgoodier Dec 11, 2015

Author Contributor

Despite this being more code, I think your suggestion makes it a little clearer what's going on. But it has a bug for NSURLSession on iOS 8+ for NSURLRequestUseProtocolCachePolicy (no osVersion hack needed). I'll see if I can't incorporate the 2 to get something clearer, but may just leave it for now.

NSURLSessionConfiguration *config = _associatedSession.configuration;
if (config.requestCachePolicy == NSURLRequestUseProtocolCachePolicy) {
response = [config.URLCache cachedResponseForRequest:self.request];
NSURLRequestCachePolicy cachePolicy = config.requestCachePolicy;
if (cachePolicy == NSURLRequestUseProtocolCachePolicy ||
(osVersion < 8 && (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)) {
return [[NSURLCache sharedURLCache] cachedResponseForRequest:self.request];
}
}

return response;
return nil;
}

- (instancetype)initWithRequest:(NSURLRequest *)request cachedResponse:(NSCachedURLResponse *)cachedResponse client:(id <NSURLProtocolClient>)client
Expand Down

0 comments on commit 4b9f633

Please sign in to comment.