Mhurd has uploaded a new change for review.
https://gerrit.wikimedia.org/r/197277
Change subject: Fix for expensive image data inflation for determing image size.
......................................................................
Fix for expensive image data inflation for determing image size.
Fix for NSURLCache thinking placeholder image records existed when they didn't.
This was causing images which didn't exceed the THUMBNAIL_MINIMUM_SIZE_TO_CACHE
to be incorrectly routed to the article data store! It also caused ALL of these
images to run through the expensive image data inflation mentioned above. All
Fixed now! :)
Change-Id: I930aa904a1e5235dd068d76f98fb3bbfa16503c9
---
M MediaWikiKit/MediaWikiKit/MWKImage.m
M wikipedia/Networking/Fetchers/ArticleFetcher.m
M wikipedia/Web Image Interception/URLCache.m
3 files changed, 32 insertions(+), 52 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/apps/ios/wikipedia
refs/changes/77/197277/1
diff --git a/MediaWikiKit/MediaWikiKit/MWKImage.m
b/MediaWikiKit/MediaWikiKit/MWKImage.m
index 251bbfb..b784e2b 100644
--- a/MediaWikiKit/MediaWikiKit/MWKImage.m
+++ b/MediaWikiKit/MediaWikiKit/MWKImage.m
@@ -127,10 +127,12 @@
_dateLastAccessed = [[NSDate alloc] init];
_mimeType = [self getImageMimeTypeForExtension:self.extension];
-#warning TODO(bgerstle): get width & height info w/o expensively inflating the
image data and then throwing it away
- UIImage* img = [UIImage imageWithData:data scale:1.0];
- _width = [NSNumber numberWithInt:img.size.width];
- _height = [NSNumber numberWithInt:img.size.height];
+ // Width / height may already be set, so only inflate image data to get
these if necessary.
+ if (!_width || !_height) {
+ UIImage* img = [UIImage imageWithData:data];
+ _width = [NSNumber numberWithInt:img.size.width];
+ _height = [NSNumber numberWithInt:img.size.height];
+ }
}
- (NSString*)getImageMimeTypeForExtension:(NSString*)extension {
diff --git a/wikipedia/Networking/Fetchers/ArticleFetcher.m
b/wikipedia/Networking/Fetchers/ArticleFetcher.m
index c695070..0cba1b8 100644
--- a/wikipedia/Networking/Fetchers/ArticleFetcher.m
+++ b/wikipedia/Networking/Fetchers/ArticleFetcher.m
@@ -281,8 +281,8 @@
continue;
}
- NSString* src = imageNode.attributes[@"src"];
- int density = 1;
+ NSString* src = imageNode.attributes[@"src"];
+ NSInteger density = 1;
// This is a horrible hack to compensate for iOS 8 WebKit's srcset
// handling and the way we currently handle image caching which
@@ -319,6 +319,18 @@
}
MWKImage* image = [self.article importImageURL:src
sectionId:sectionId];
+
+ // If dimensions determined, save them so they don't have to be
expensively determined later.
+ if (width && height) {
+ // Don't record dimensions if image file name doesn't have size
prefix.
+ // (Sizes from the img tag don't tend to correspond closely to
actual
+ // image binary sizes for these.)
+ if (![src.lastPathComponent
isEqualToString:image.fileNameNoSizePrefix]) {
+ image.width = @(width.integerValue * density);
+ image.height = @(height.integerValue * density);
+ }
+ }
+
[image save];
imageIndexInSection++;
diff --git a/wikipedia/Web Image Interception/URLCache.m b/wikipedia/Web Image
Interception/URLCache.m
index d64248f..4c928af 100644
--- a/wikipedia/Web Image Interception/URLCache.m
+++ b/wikipedia/Web Image Interception/URLCache.m
@@ -22,25 +22,9 @@
@property (readonly) MWKArticle* article;
-// Reminder: When using this debugging image to test caching (i.e. seeing if
article images
-// show the placeholder) be sure to quit and restart the app (double-tap the
home button
-// and flick the app up offscreen) otherwise the web view keeps showing its
memory cache
-// version of the actual image it downloaded - that is, it has no need to
attempt a cache hit.
-// Once the app is then restarted if everything is working the article images
should all
-// show the placeholder image.
-@property (strong, nonatomic) NSData* debuggingPlaceHolderImageData;
-
@end
@implementation URLCache
-
-- (id)initWithMemoryCapacity:(NSUInteger)memoryCapacity
diskCapacity:(NSUInteger)diskCapacity diskPath:(NSString*)path {
- self = [super initWithMemoryCapacity:memoryCapacity
diskCapacity:diskCapacity diskPath:path];
- if (self) {
- self.debuggingPlaceHolderImageData = UIImagePNGRepresentation([UIImage
imageNamed:@"logo-onboarding-subtitle.png"]);
- }
- return self;
-}
- (MWKArticle*)article {
return [SessionSingleton sharedInstance].currentArticle;
@@ -86,43 +70,25 @@
return;
}
- // Save image to articleData store instead of default NSURLCache store.
- NSURL* url = cachedResponse.response.URL;
- NSString* urlStr = [url absoluteString];
+ NSString* urlStr = [[cachedResponse.response.URL absoluteString]
getUrlWithoutScheme];
- // Strip "http:" or "https:"
- urlStr = [urlStr getUrlWithoutScheme];
+ // Save image to article data store instead of default NSURLCache store if
+ // "createImageRecordsForSection:" pre-created a placeholder MWKImage
record.
- NSData* imageDataToUse = cachedResponse.data;
-
- MWKImage* image = [self.article imageWithURL:urlStr];
-
- if (!image) {
- // If an Image object wasn't pre-created by
:createSectionImageRecordsForSectionHtml:onContext:" then don't try to cache.
+ // If no placeholder record, call super and return for default NSURLCache
behavior.
+ NSString* thisImageCacheFolderPath = [self.article.dataStore
pathForImageURL:urlStr title:self.article.title];
+ BOOL isDirectory = NO;
+ BOOL thisImageCacheFolderPathExists = [[NSFileManager defaultManager]
fileExistsAtPath:thisImageCacheFolderPath isDirectory:&isDirectory];
+ if (!thisImageCacheFolderPathExists) {
+ // If an Image folder wasn't pre-created by
"createImageRecordsForSection:" then don't try to cache.
URLCacheLog(@"Storing cached response without image record for %@",
request);
[super storeCachedResponse:cachedResponse forRequest:request];
return;
}
- /*
- // Quick debugging filter which makes image blue before saving them to
our articleData store.
- // (locks up occasionally - probably not thread safe - only for testing
so no worry for now)
- CIImage *inputImage = [[CIImage alloc]
initWithData:cachedResponse.data];
-
- CIFilter *colorMonochrome = [CIFilter
filterWithName:@"CIColorMonochrome"];
- [colorMonochrome setDefaults];
- [colorMonochrome setValue: inputImage forKey: @"inputImage"];
- [colorMonochrome setValue: [CIColor colorWithRed:0.6f green:0.6f
blue:1.0f alpha:1.0f] forKey: @"inputColor"];
-
- CIImage *outputImage = [colorMonochrome valueForKey:@"outputImage"];
- CIContext *context = [CIContext contextWithOptions:nil];
- UIImage *outputUIImage = [UIImage imageWithCGImage:[context
createCGImage:outputImage fromRect:outputImage.extent]];
- imageDataToUse = UIImagePNGRepresentation(outputUIImage);
- */
-
- // Another quick debugging indicator which caches a "W" logo image instead
of the real image.
- // (This one has no thread safety issues.)
- //imageDataToUse = self.debuggingPlaceHolderImageData;
+ // Placeholder record found, so route image data to article data store.
+ MWKImage* image = [self.article imageWithURL:urlStr];
+ NSData* imageDataToUse = cachedResponse.data;
@try {
URLCacheLog(@"Rerouting cached response to WMF data store for %@",
request);
--
To view, visit https://gerrit.wikimedia.org/r/197277
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I930aa904a1e5235dd068d76f98fb3bbfa16503c9
Gerrit-PatchSet: 1
Gerrit-Project: apps/ios/wikipedia
Gerrit-Branch: master
Gerrit-Owner: Mhurd <mh...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
