[webkit-reviews] review granted: [Bug 73960] [WK2] Add permissions support to web notifications : [Attachment 118257] Trying again with gtk

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Jon Lee <jonlee at apple.com>'s request
for review:
Bug 73960: [WK2] Add permissions support to web notifications
https://bugs.webkit.org/show_bug.cgi?id=73960

Attachment 118257: Trying again with gtk
https://bugs.webkit.org/attachment.cgi?id=118257&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=118257&action=review


> Source/WebKit2/UIProcess/Notifications/WebNotification.cpp:38
> +WebNotification::WebNotification()
> +{
> +}

Shouldn’t we set m_notificationID to 0 in this constructor? I suppose maybe not
if we always just decode afterwards, but it seems slightly strange that the
strings will both be initialized and the notification will not.

> Source/WebKit2/UIProcess/Notifications/WebNotification.cpp:49
> +WebNotification::~WebNotification()
> +{
> +}

I suggest not defining or declaring this and just letting the compiler generate
it.

> Source/WebKit2/UIProcess/Notifications/WebNotificationManagerProxy.cpp:53
> +WebNotificationManagerProxy::~WebNotificationManagerProxy()
> +{
> +}

Seems we should just let the compiler generate this and need not define or
declare it.

>
Source/WebKit2/WebProcess/Notifications/NotificationPermissionRequestManager.cp
p:62
>
+NotificationPermissionRequestManager::~NotificationPermissionRequestManager()
> +{
> +}

Seems we should just let the compiler generate this.