Soft link QuickLookThumbnailing framework
Created attachment 397306 [details]
Comment on attachment 397306 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=397306&action=review
> +//WK_QUICKLOOK_THUMBNAILING_LDFLAGS_macosx = $(WK_QUICKLOOK_THUMBNAILING_LDFLAGS$(WK_MACOS_1015));
> +//WK_QUICKLOOK_THUMBNAILING_LDFLAGS_MACOS_SINCE_1015 = -framework QuickLookThumbnailing;
> +//WK_QUICKLOOK_THUMBNAILING_LDFLAGS_iphoneos = -framework QuickLookThumbnailing;
You'll want to actually delete this, not just comment it out
> + _filePath = adoptNS([[NSURL fileURLWithPath:fileURL] copy]);
No need for the adopt OR the copy: you can just assign an autoreleased NSURL (as returned by -fileURLWithPath) to a RetainPtr. It will retain it, and all will be well. The previous code was OK
> - auto fileURLPath = adoptNS([NSURL fileURLWithPath:filePath]);
> + auto fileURLPath = adoptNS([[NSURL fileURLWithPath:filePath] copy]);
THIS one was wrong, and will crash. Remove the adoptNS(), and don't add a copy (you only need adopt if you have a +1 object, returned by a -new or -alloc or -create -- NOT for an autoreleased object like here).
> + req.get().iconMode = YES;
we generally prefer [req setIconMode:YES]; (normally we'd rather dot notation, but the .get(). is SO UGLY)
> +#if HAVE(QUICKLOOK_THUMBNAILING)
You don't actually need this SPI header! QuickLookThumbnailing is API, you can include the real header in QuickLookThumbnailingSoftLink, no problem. Soft linking is only about linking, it has no impact on whether or not you can include headers from the framework.
Created attachment 397309 [details]
Created attachment 397310 [details]
Committed r260553: <https://trac.webkit.org/changeset/260553>
All reviewed patches have been landed. Closing bug and clearing flags on attachment 397310 [details].
This change broke iOS and Catalina builds:
/Volumes/Data/worker/iOS-13-Build-EWS/build/Source/WebKit/UIProcess/QuickLookThumbnailLoader.mm:97:46: error: property 'get' not found on object of type 'NSURL *'
Reverted r260553 for reason:
Broke iOS and Catalina builds.
Committed r260558: <https://trac.webkit.org/changeset/260558>
Created attachment 397356 [details]
Committed r260581: <https://trac.webkit.org/changeset/260581>
All reviewed patches have been landed. Closing bug and clearing flags on attachment 397356 [details].