[Webkit-unassigned] [Bug 73544] [EFL] Add Web Notification feature for WebKit-EFL

[bugzilla-daemon at webkit.org]

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


Gyuyoung Kim <gyuyoung.kim at samsung.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #121398|commit-queue?               |commit-queue-
               Flag|                            |




--- Comment #25 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2012-01-05 22:43:28 PST ---
(From update of attachment 121398)
View in context: https://bugs.webkit.org/attachment.cgi?id=121398&action=review

> Source/WebKit/efl/ewk/ewk_notification.cpp:124
> +    Ewk_Notification* notification = static_cast<Ewk_Notification*>(malloc(sizeof(Ewk_Notification)));

Please use new instead of malloc.

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

> Source/WebKit/efl/ewk/ewk_notification.cpp:125
> +    if (!notification)

new never return 0. Remove this.

> Source/WebKit/efl/ewk/ewk_notification.cpp:138
> +    free(ewkNotification);

Use delete instead of free.

> Source/WebKit/efl/ewk/ewk_notification.cpp:144
> +    Ewk_Notification_Info* notification_info = static_cast<Ewk_Notification_Info*>(malloc(sizeof(Ewk_Notification_Info)));

ditto.

> Source/WebKit/efl/ewk/ewk_notification.cpp:146
> +        return 0;

ditto.

> Source/WebKit/efl/ewk/ewk_notification.h:2
> +    Copyright (C) 2011 Samsung Electronics

Change year.

> Source/WebKit/efl/ewk/ewk_notification.h:35
> + *  - "notification,show", @c Ewk_Notification_Info: requested contents to display the notification.

This signal is emitted by view. So, this comment needs to be moved to ewk_view.h

> Source/WebKit/efl/ewk/ewk_private.h:234
> +#if ENABLE(NOTIFICATIONS)

I think we can move this internal functions to ewk_notificaiton_private.h. Bug 75592 also try to add similar private header file. If the Bug's patch is landed, this patch also can has similar file.

> Source/WebKit/efl/ewk/ewk_view.cpp:3916
> +WebCore::NotificationPresenter* ewk_view_notification_presenter_get(const Evas_Object* ewkView)

Can we move this function to ewk_notification?

We can get page object as below,
WebCore::Page* page = EWKPrivate::corePage(priv->ewkView);

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