[webkit-reviews] review granted: [Bug 209911] [macOS] Remove redundant call to check in with Launch Services : [Attachment 395271] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Per Arne Vollan <pvollan at apple.com>'s
request for review:
Bug 209911: [macOS] Remove redundant call to check in with Launch Services
https://bugs.webkit.org/show_bug.cgi?id=209911

Attachment 395271: Patch

https://bugs.webkit.org/attachment.cgi?id=395271&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 395271
  --> https://bugs.webkit.org/attachment.cgi?id=395271
Patch

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

r=me assuming it builds and passes tests

> Source/WebCore/PAL/pal/spi/cocoa/CoreServicesSPI.h:28
> +extern "C" void _CSCheckFixDisable();

I would have expected this earlier in the file:

#if USE(APPLE_INTERNAL_SDK)

#include <CarbonCore/RuntimePriv.h>

#endif

Unless there is some reason we can’t or shouldn’t do that?

> Source/WebCore/PAL/pal/spi/cocoa/LaunchServicesSPI.h:108
> +typedef bool (^LSServerConnectionAllowedBlock) (CFDictionaryRef optionsRef);
> +extern "C" void
_LSSetApplicationLaunchServicesServerConnectionStatus(uint64_t flags,
LSServerConnectionAllowedBlock block);
> +extern "C" CFDictionaryRef _LSApplicationCheckIn(LSSessionID sessionID,
CFDictionaryRef applicationInfo);

Similarly, kind of expected, within the #if USE(APPLE_INTERNAL_SDK) section:

#import <LaunchServices/LSApplicationServicesPriv.h>

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:542
> +    // _CSCheckFixDisable() needs to be called before checking in with
Launch Services.

This comment leaves out critical context. It’s not obvious how this is the
right place to do this. How is this place "before checking in with Launch
Services"? The comment should say.