Bug 110544 - Use CFNotificationCenter instead of NSNotificationCenter for SharedTimerIOS
Summary: Use CFNotificationCenter instead of NSNotificationCenter for SharedTimerIOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-02-21 19:01 PST by Benjamin Poulain
Modified: 2013-02-22 14:17 PST (History)
2 users (show)

See Also:

Attachments
Patch (3.25 KB, patch)
2013-02-21 19:04 PST, Benjamin Poulain 		dbates: 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 Benjamin Poulain 2013-02-21 19:01:41 PST 
Use CFNotificationCenter instead of NSNotificationCenter for SharedTimerIOS
Comment 1 Benjamin Poulain 2013-02-21 19:04:48 PST 
Created attachment 189660 [details]
Patch
Comment 2 Daniel Bates 2013-02-22 11:52:23 PST 
Comment on attachment 189660 [details]
Patch

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

This patch looks straightforward to me. You may want to consider adding a remark that we never remove the observer for notification UIApplicationDidBecomeActiveNotification. (This behavior is consistent with the behavior we had when we used WebCoreResumeNotifierIOS). If you are looking for a more thorough review then feel free to have someone more familiar with {CF, NS}NotificationCenter review this patch.

> Source/WebCore/ChangeLog:8
> +        Previously, we were instanciating the Obj-C object WebCoreResumeNotifierIOS

Nit: instanciating => instantiating

> Source/WebCore/ChangeLog:12
> +        without the itermediary object.

Nit: itermediary => intermediary

> Source/WebCore/platform/ios/SharedTimerIOS.mm:38
> +static void applicationDidBecomeActive(CFNotificationCenterRef, void*, CFStringRef, const void *, CFDictionaryRef)

Nit: "void *" => "void*"

Notice that lack of a space character between "void" and the '*'.
Comment 3 Benjamin Poulain 2013-02-22 14:17:20 PST 
Committed r143795: <http://trac.webkit.org/changeset/143795>
Comment 4 Benjamin Poulain 2013-02-22 14:17:49 PST 
Thank you for the review.