[webkit-reviews] review granted: [Bug 46935] WebCore uses the main thread to process sleep notifications : [Attachment 69383] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Anders Carlsson
<andersca at apple.com>'s request for review:
Bug 46935: WebCore uses the main thread to process sleep notifications
https://bugs.webkit.org/show_bug.cgi?id=46935

Attachment 69383: Patch
https://bugs.webkit.org/attachment.cgi?id=69383&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=69383&action=review

> WebCore/platform/mac/SharedTimerMac.mm:40
> +#define IOKIT_CAN_USE_LIBDISPATCH 1

I’d like this to be reversed. Maybe IOKIT_WITHOUT_LIBDISPATCH?

> WebCore/platform/mac/SharedTimerMac.mm:67
> +    static void systemPowerCallback(void* context, io_service_t service,
uint32_t messageType, void* messageArgument);
> +    void systemPowerCallback(io_service_t service, uint32_t messageType,
void* messageArgument);

The argument name service could be left out in these two function declarations.


> WebCore/platform/mac/SharedTimerMac.mm:143
> +#ifdef IOKIT_CAN_USE_LIBDISPATCH
> +    // We need to call the timer functions on the main thread.
> +    CFRunLoopPerformBlock(CFRunLoopGetMain(), kCFRunLoopCommonModes, ^() {
> +#endif
>  
> -    if (WebCore::sharedTimer) {
> -	   WebCore::stopSharedTimer();
> -	   WebCore::timerFired(0, 0);
> +    if (sharedTimer) {
> +	   stopSharedTimer();
> +	   timerFired(0, 0);
>      }
> +    
> +#ifdef IOKIT_CAN_USE_LIBDISPATCH
> +    });
> +#endif
>  }

You could do this more cleanly if the body here was a member function. Then you
could call it in the block and in the #else and not feel the need to use #if in
this fancier way.