Bug 189288 - Post review Weinig fix-ups
Summary: Post review Weinig fix-ups
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-09-04 17:14 PDT by Dean Jackson
Modified: 2018-09-04 17:56 PDT (History)
2 users (show)

See Also:


Attachments
Patch (8.43 KB, patch)
2018-09-04 17:16 PDT, Dean Jackson
sam: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2018-09-04 17:14:52 PDT
Post review Weinig fix-ups
Comment 1 Dean Jackson 2018-09-04 17:16:28 PDT
Created attachment 348876 [details]
Patch
Comment 2 Sam Weinig 2018-09-04 17:22:52 PDT
Comment on attachment 348876 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=348876&action=review

> Source/WebCore/platform/MIMETypeRegistry.cpp:710
> +    static NeverDestroyed<HashSet<String, ASCIICaseInsensitiveHash>> systemPreviewMIMETypes = std::initializer_list<String> {

I think you want to remove the global static above as well.

> Source/WebCore/rendering/RenderThemeIOS.mm:1866
>      static NSBundle *arKitBundle;
> -    static dispatch_once_t onceToken;
> -    dispatch_once(&onceToken, ^{
> +    if (!arKitBundle) {
>  #if PLATFORM(IOS_SIMULATOR)
>          dlopen("/System/Library/PrivateFrameworks/AssetViewer.framework/AssetViewer", RTLD_NOW);
>          arKitBundle = [NSBundle bundleForClass:NSClassFromString(@"ASVThumbnailView")];
>  #else
>          arKitBundle = [NSBundle bundleWithURL:[NSURL fileURLWithPath:@"/System/Library/PrivateFrameworks/AssetViewer.framework"]];
>  #endif
> -    });
> +    }

If you wanted to, you could do this with a lamba, removing the if-statement, but I am not sure it is any better.

static NSBundle *arKitBundle = []() { 
    ...
    return [NSBundle ...
}();

> Source/WebCore/rendering/RenderThemeIOS.mm:-1895
>      static CGPDFPageRef logoPage;
> -    static dispatch_once_t onceToken;
> -    dispatch_once(&onceToken, ^{
> +    if (!logoPage)
>          logoPage = loadARKitPDFPage(@"ARKitBadge").leakRef();
> -    });

This could all be one line.
Comment 3 Dean Jackson 2018-09-04 17:55:53 PDT
Committed r235651: <https://trac.webkit.org/changeset/235651>
Comment 4 Radar WebKit Bug Importer 2018-09-04 17:56:14 PDT
<rdar://problem/44121295>