- Revision
- 260611
- Author
- [email protected]
- Date
- 2020-04-23 17:46:18 -0700 (Thu, 23 Apr 2020)
Log Message
Clean up QuickLookThumbnailLoader
<https://webkit.org/b/210814>
Reviewed by Darin Adler.
The following items are cleaned up:
- Extract `using PlatformImage` into QuickLookThumbnailLoader.h,
rename to `CocoaImage` and use to get rid of duplicate
code.
- Change `id` to `instancetype` for -init methods.
- Add `atomic` keyword to @property definitions that were using
it as the default. (Use of atomic properties is rare in
WebKit, so being explicit avoids a scenario where it looks
like `nonatomic` was left off by accident.)
- Change @property definitions to `readonly` that are never
written to outside of QuickLookThumbnailLoader.mm.
- Delete unused @property definitions.
- Change method declarations into read-only @property
definitions.
- Re-declare atomic read-only @property definitions in
QuickLookThumbnailLoader.h as read-write definitions in
QuickLookThumbnailLoader.mm if they are written to.
* UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::convertPlatformImageToBitmap):
* UIProcess/QuickLookThumbnailLoader.h:
- Rename qlThumbnailGenerationQueue @property to just `queue`.
- Remove `contentType` @property. It is not used anywhere.
This also fixes a theoretical leak found by the clang static
analyzer.
- Remove `shouldWrite` @property. It is only used within
QuickLookThumbnailLoader.mm.
- Change `identifier` and `thumbnail` to @property declarations.
* UIProcess/QuickLookThumbnailLoader.mm:
- Change WKQLThumbnailLoadOperation._identifier type from
NSMutableString to NSString. There was no reason for it to
be mutable.
(-[WKQLThumbnailQueueManager init]):
(-[WKQLThumbnailQueueManager dealloc]):
- Release `_queue` to fix theoretical leak found by the clang
static analyzer.
(-[WKQLThumbnailLoadOperation initWithAttachment:identifier:]):
(-[WKQLThumbnailLoadOperation initWithURL:identifier:]):
(-[WKQLThumbnailLoadOperation start]):
- Rename `req` to `request`.
- Change separate #if macros to #if/#else since only one version
of this code can be used at a time.
(-[WKQLThumbnailLoadOperation thumbnail]):
- Use CocoaImage to use one copy of the method.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (260610 => 260611)
--- trunk/Source/WebKit/ChangeLog 2020-04-24 00:12:00 UTC (rev 260610)
+++ trunk/Source/WebKit/ChangeLog 2020-04-24 00:46:18 UTC (rev 260611)
@@ -1,3 +1,55 @@
+2020-04-23 David Kilzer <[email protected]>
+
+ Clean up QuickLookThumbnailLoader
+ <https://webkit.org/b/210814>
+
+ Reviewed by Darin Adler.
+
+ The following items are cleaned up:
+ - Extract `using PlatformImage` into QuickLookThumbnailLoader.h,
+ rename to `CocoaImage` and use to get rid of duplicate
+ code.
+ - Change `id` to `instancetype` for -init methods.
+ - Add `atomic` keyword to @property definitions that were using
+ it as the default. (Use of atomic properties is rare in
+ WebKit, so being explicit avoids a scenario where it looks
+ like `nonatomic` was left off by accident.)
+ - Change @property definitions to `readonly` that are never
+ written to outside of QuickLookThumbnailLoader.mm.
+ - Delete unused @property definitions.
+ - Change method declarations into read-only @property
+ definitions.
+ - Re-declare atomic read-only @property definitions in
+ QuickLookThumbnailLoader.h as read-write definitions in
+ QuickLookThumbnailLoader.mm if they are written to.
+
+ * UIProcess/Cocoa/WebPageProxyCocoa.mm:
+ (WebKit::convertPlatformImageToBitmap):
+ * UIProcess/QuickLookThumbnailLoader.h:
+ - Rename qlThumbnailGenerationQueue @property to just `queue`.
+ - Remove `contentType` @property. It is not used anywhere.
+ This also fixes a theoretical leak found by the clang static
+ analyzer.
+ - Remove `shouldWrite` @property. It is only used within
+ QuickLookThumbnailLoader.mm.
+ - Change `identifier` and `thumbnail` to @property declarations.
+ * UIProcess/QuickLookThumbnailLoader.mm:
+ - Change WKQLThumbnailLoadOperation._identifier type from
+ NSMutableString to NSString. There was no reason for it to
+ be mutable.
+ (-[WKQLThumbnailQueueManager init]):
+ (-[WKQLThumbnailQueueManager dealloc]):
+ - Release `_queue` to fix theoretical leak found by the clang
+ static analyzer.
+ (-[WKQLThumbnailLoadOperation initWithAttachment:identifier:]):
+ (-[WKQLThumbnailLoadOperation initWithURL:identifier:]):
+ (-[WKQLThumbnailLoadOperation start]):
+ - Rename `req` to `request`.
+ - Change separate #if macros to #if/#else since only one version
+ of this code can be used at a time.
+ (-[WKQLThumbnailLoadOperation thumbnail]):
+ - Use CocoaImage to use one copy of the method.
+
2020-04-23 Megan Gardner <[email protected]>
Long pressing attachments in Notes does not activate Context Menu.
Modified: trunk/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm (260610 => 260611)
--- trunk/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm 2020-04-24 00:12:00 UTC (rev 260610)
+++ trunk/Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm 2020-04-24 00:46:18 UTC (rev 260611)
@@ -389,13 +389,8 @@
#endif
#if HAVE(QUICKLOOK_THUMBNAILING)
-#if PLATFORM(MAC)
-using PlatformImage = NSImage*;
-#elif PLATFORM(IOS_FAMILY)
-using PlatformImage = UIImage*;
-#endif
-static RefPtr<WebKit::ShareableBitmap> convertPlatformImageToBitmap(PlatformImage image, const WebCore::IntSize& size)
+static RefPtr<WebKit::ShareableBitmap> convertPlatformImageToBitmap(CocoaImage *image, const WebCore::IntSize& size)
{
WebKit::ShareableBitmap::Configuration bitmapConfiguration;
auto bitmap = WebKit::ShareableBitmap::createShareable(size, bitmapConfiguration);
@@ -432,7 +427,7 @@
});
}];
- [[WKQLThumbnailQueueManager sharedInstance].qlThumbnailGenerationQueue addOperation:operation];
+ [[WKQLThumbnailQueueManager sharedInstance].queue addOperation:operation];
}
@@ -449,7 +444,8 @@
}
-#endif
+#endif // HAVE(QUICKLOOK_THUMBNAILING)
+
} // namespace WebKit
#undef MESSAGE_CHECK_COMPLETION
Modified: trunk/Source/WebKit/UIProcess/QuickLookThumbnailLoader.h (260610 => 260611)
--- trunk/Source/WebKit/UIProcess/QuickLookThumbnailLoader.h 2020-04-24 00:12:00 UTC (rev 260610)
+++ trunk/Source/WebKit/UIProcess/QuickLookThumbnailLoader.h 2020-04-24 00:46:18 UTC (rev 260611)
@@ -25,36 +25,35 @@
#if HAVE(QUICKLOOK_THUMBNAILING)
-#if PLATFORM(IOS_FAMILY)
+#if USE(APPKIT)
+@class NSImage;
+using CocoaImage = NSImage;
+#else
@class UIImage;
+using CocoaImage = UIImage;
#endif
@interface WKQLThumbnailQueueManager : NSObject
-@property (nonatomic, readwrite, retain) NSOperationQueue* qlThumbnailGenerationQueue;
-- (id)init;
+@property (nonatomic, readonly, retain) NSOperationQueue *queue;
+
+- (instancetype)init;
+ (WKQLThumbnailQueueManager *)sharedInstance;
+
@end
@interface WKQLThumbnailLoadOperation : NSOperation
-@property (readonly, getter=isAsynchronous) BOOL asynchronous;
-@property (readonly, getter=isExecuting) BOOL executing;
-@property (readonly, getter=isFinished) BOOL finished;
+@property (atomic, readonly, getter=isAsynchronous) BOOL asynchronous;
+@property (atomic, readonly, getter=isExecuting) BOOL executing;
+@property (atomic, readonly, getter=isFinished) BOOL finished;
-@property (nonatomic, copy) NSString *contentType;
-@property (nonatomic) BOOL shouldWrite;
+@property (nonatomic, readonly, copy) NSString *identifier;
+@property (nonatomic, readonly, retain) CocoaImage *thumbnail;
-- (id)initWithAttachment:(NSFileWrapper *)fileWrapper identifier:(NSString *)identifier;
-- (id)initWithURL:(NSString *)fileURL identifier:(NSString *)identifier;
-- (NSString *)identifier;
-#if PLATFORM(IOS_FAMILY)
--(UIImage *)thumbnail;
-#endif
-#if PLATFORM(MAC)
--(NSImage *)thumbnail;
-#endif
+- (instancetype)initWithAttachment:(NSFileWrapper *)fileWrapper identifier:(NSString *)identifier;
+- (instancetype)initWithURL:(NSString *)fileURL identifier:(NSString *)identifier;
@end
-#endif
+#endif // HAVE(QUICKLOOK_THUMBNAILING)
Modified: trunk/Source/WebKit/UIProcess/QuickLookThumbnailLoader.mm (260610 => 260611)
--- trunk/Source/WebKit/UIProcess/QuickLookThumbnailLoader.mm 2020-04-24 00:12:00 UTC (rev 260610)
+++ trunk/Source/WebKit/UIProcess/QuickLookThumbnailLoader.mm 2020-04-24 00:46:18 UTC (rev 260611)
@@ -34,14 +34,20 @@
@implementation WKQLThumbnailQueueManager
-- (id)init
+- (instancetype)init
{
self = [super init];
if (self)
- _qlThumbnailGenerationQueue = [[NSOperationQueue alloc] init];
+ _queue = [[NSOperationQueue alloc] init];
return self;
}
+- (void)dealloc
+{
+ [_queue release];
+ [super dealloc];
+}
+
+ (WKQLThumbnailQueueManager *)sharedInstance
{
static WKQLThumbnailQueueManager *sharedInstance = [[WKQLThumbnailQueueManager alloc] init];
@@ -50,19 +56,20 @@
@end
+@interface WKQLThumbnailLoadOperation ()
+@property (atomic, readwrite, getter=isExecuting) BOOL executing;
+@property (atomic, readwrite, getter=isFinished) BOOL finished;
+@end
+
@implementation WKQLThumbnailLoadOperation {
RetainPtr<NSURL> _filePath;
- RetainPtr<NSMutableString> _identifier;
+ RetainPtr<NSString> _identifier;
RetainPtr<NSFileWrapper> _fileWrapper;
-#if PLATFORM(MAC)
- RetainPtr<NSImage> _thumbnail;
-#endif
-#if PLATFORM(IOS_FAMILY)
- RetainPtr<UIImage> _thumbnail;
-#endif
+ RetainPtr<CocoaImage> _thumbnail;
+ BOOL _shouldWrite;
}
-- (id)initWithAttachment:(NSFileWrapper *)fileWrapper identifier:(NSString *)identifier
+- (instancetype)initWithAttachment:(NSFileWrapper *)fileWrapper identifier:(NSString *)identifier
{
if (self = [super init]) {
_fileWrapper = fileWrapper;
@@ -72,7 +79,7 @@
return self;
}
-- (id)initWithURL:(NSString *)fileURL identifier:(NSString *)identifier
+- (instancetype)initWithURL:(NSString *)fileURL identifier:(NSString *)identifier
{
if (self = [super init]) {
_identifier = adoptNS([identifier copy]);
@@ -84,10 +91,10 @@
- (void)start
{
self.executing = YES;
-
+
if (_shouldWrite) {
NSString *temporaryDirectory = FileSystem::createTemporaryDirectory(@"QLTempFileData");
-
+
NSString *filePath = [temporaryDirectory stringByAppendingPathComponent:[_fileWrapper preferredFilename]];
NSFileWrapperWritingOptions options = 0;
NSError *error = nil;
@@ -100,18 +107,17 @@
return;
}
- auto req = adoptNS([WebKit::allocQLThumbnailGenerationRequestInstance() initWithFileAtURL:_filePath.get() size:CGSizeMake(400, 400) scale:1 representationTypes:QLThumbnailGenerationRequestRepresentationTypeAll]);
- [req setIconMode:YES];
+ auto request = adoptNS([WebKit::allocQLThumbnailGenerationRequestInstance() initWithFileAtURL:_filePath.get() size:CGSizeMake(400, 400) scale:1 representationTypes:QLThumbnailGenerationRequestRepresentationTypeAll]);
+ [request setIconMode:YES];
- [[WebKit::getQLThumbnailGeneratorClass() sharedGenerator] generateBestRepresentationForRequest:req.get() completionHandler:^(QLThumbnailRepresentation *thumbnail, NSError *error) {
+ [[WebKit::getQLThumbnailGeneratorClass() sharedGenerator] generateBestRepresentationForRequest:request.get() completionHandler:^(QLThumbnailRepresentation *thumbnail, NSError *error) {
if (error)
return;
if (_thumbnail)
return;
-#if PLATFORM(MAC)
+#if USE(APPKIT)
_thumbnail = thumbnail.NSImage;
-#endif
-#if PLATFORM(IOS_FAMILY)
+#else
_thumbnail = thumbnail.UIImage;
#endif
if (_shouldWrite)
@@ -122,20 +128,11 @@
}];
}
-#if PLATFORM(IOS_FAMILY)
-- (UIImage *)thumbnail
+- (CocoaImage *)thumbnail
{
return _thumbnail.get();
}
-#endif
-#if PLATFORM(MAC)
-- (NSImage *)thumbnail
-{
- return _thumbnail.get();
-}
-#endif
-
- (NSString *)identifier
{
return _identifier.get();
@@ -188,4 +185,4 @@
@end
-#endif
+#endif // HAVE(QUICKLOOK_THUMBNAILING)
