[webkit-reviews] review denied: [Bug 73194] [BlackBerry] Add notification support for the BlackBerry port : [Attachment 117136] Patch for notification support. V4.

[bugzilla-daemon at webkit.org]

Rob Buis <rwlbuis at gmail.com> has denied Robin Qiu
<robin.qiu at torchmobile.com.cn>'s request for review:
Bug 73194: [BlackBerry] Add notification support for the BlackBerry port
https://bugs.webkit.org/show_bug.cgi?id=73194

Attachment 117136: Patch for notification support. V4.
https://bugs.webkit.org/attachment.cgi?id=117136&action=review

------- Additional Comments from Rob Buis <rwlbuis at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=117136&action=review


Looks quite close to being finished, some nits, so r- for now :)

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.cpp:52
> +bool NotificationPresenterImpl::show(Notification* n)

For consistency needs (Notification* notification), see
notificationObjectDestroyed.

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.cpp:77
> +void NotificationPresenterImpl::cancel(Notification* n)

Ditto.

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.cpp:126
> +    // FIXME: Should store the permission information into file permenently
instead of in m_allowedDomains.

Typo -> permanently

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.h:56
> +    // Cancel all outstanding requests for the ScriptExecutionContext

misses '.' at the end.