[webkit-reviews] review granted: [Bug 110544] Use CFNotificationCenter instead of NSNotificationCenter for SharedTimerIOS : [Attachment 189660] Patch

[bugzilla-daemon at webkit.org]

Daniel Bates <dbates at webkit.org> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 110544: Use CFNotificationCenter instead of NSNotificationCenter for
SharedTimerIOS
https://bugs.webkit.org/show_bug.cgi?id=110544

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

------- Additional Comments from Daniel Bates <dbates at webkit.org>
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 '*'.