Bug 46935 - WebCore uses the main thread to process sleep notifications
Summary: WebCore uses the main thread to process sleep notifications
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Anders Carlsson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2010-09-30 14:06 PDT by Anders Carlsson
Modified: 2010-09-30 15:02 PDT (History)
0 users

See Also:

Attachments
Patch (10.57 KB, patch)
2010-09-30 14:23 PDT, Anders Carlsson 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Anders Carlsson 2010-09-30 14:06:31 PDT 
WebCore uses the main thread to process sleep notifications
Comment 1 Anders Carlsson 2010-09-30 14:09:07 PDT 
<rdar://problem/8495374>
Comment 2 Anders Carlsson 2010-09-30 14:23:02 PDT 
Created attachment 69383 [details]
Patch
Comment 3 Darin Adler 2010-09-30 14:39:17 PDT 
Comment on attachment 69383 [details]
Patch

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.
Comment 4 mitz 2010-09-30 14:40:47 PDT 
Comment on attachment 69383 [details]
Patch

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

> WebCore/WebCore.xcodeproj/project.pbxproj:20875
> +			developmentRegion = English;

Please don’t commit this.

> WebCore/platform/mac/SharedTimerMac.mm:30
> +#import <IOKit/pwr_mgt/IOPMLib.h>
> +#import <IOKit/IOMessage.h>

I comes before p.

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

This is not a great name for this method.
Comment 5 Anders Carlsson 2010-09-30 15:02:07 PDT 
Committed r68836: <http://trac.webkit.org/changeset/68836>