[Webkit-unassigned] [Bug 73194] [BlackBerry] Upstream BlackBerry porting of NotificationPresenter

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73194





--- Comment #2 from Leo Yang <leo.yang at torchmobile.com.cn>  2011-11-29 17:57:27 PST ---
(From update of attachment 116922)
View in context: https://bugs.webkit.org/attachment.cgi?id=116922&action=review

This is an informal review.

> Source/WebKit/ChangeLog:3
> +2011-11-29  Robin Qiu  <robin.qiu at torchmobile.com.cn>
> +
> +        [BlackBerry] Add notification support for the BlackBerry port.

As usual this should be the same as bug title.

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.cpp:55
> +NotificationPresenterImpl::~NotificationPresenterImpl()
> +{
> +}
> +
> +

Extra blank line, please remove one.

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

Should it ASSERT(n) here?

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

Ditto.

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.cpp:102
> +void NotificationPresenterImpl::onPermission(const std::string& domainStr, bool isAllowed)
> +{
> +    String domain = String::fromUTF8(domainStr.c_str());

Better ASSERT(!domainStr.empty()) before the converting?

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.cpp:137
> +void NotificationPresenterImpl::notificationClicked(const std::string& idStr)
> +{
> +    String id = String::fromUTF8(idStr.c_str());

Ditto

> Source/WebKit/blackberry/WebCoreSupport/NotificationPresenterImpl.h:64
> +    // Checks the current level of permission.
> +    virtual Permission checkPermission(WebCore::ScriptExecutionContext*);
> +
> +

Extra blank line, please remove one.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.