Bug 73960 - [WK2] Add permissions support to web notifications
Summary: [WK2] Add permissions support to web notifications
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh All
: P2 Normal
Assignee: Jon Lee
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-12-06 16:05 PST by Jon Lee
Modified: 2011-12-08 11:07 PST (History)
8 users (show)

See Also:


Attachments
Patch (123.88 KB, patch)
2011-12-06 17:07 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch (133.94 KB, patch)
2011-12-06 17:27 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Patch with GNU build fixes (138.15 KB, patch)
2011-12-07 10:20 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Fix gtk again (139.15 KB, patch)
2011-12-07 11:05 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Fix gtk again and adjust some xcodeproj settings and paths (142.16 KB, patch)
2011-12-07 11:51 PST, Jon Lee
no flags Details | Formatted Diff | Diff
Trying again with gtk (142.16 KB, patch)
2011-12-07 13:00 PST, Jon Lee
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jon Lee 2011-12-06 16:05:04 PST
Implement current permissions API for WK2.

<rdar://problem/10357008>
Comment 1 Jon Lee 2011-12-06 17:07:13 PST
Created attachment 118147 [details]
Patch
Comment 2 Jon Lee 2011-12-06 17:27:06 PST
Created attachment 118150 [details]
Patch

Added new files to Windows vcproj
Comment 3 Gustavo Noronha (kov) 2011-12-06 19:40:47 PST
Comment on attachment 118150 [details]
Patch

Attachment 118150 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10756001
Comment 4 Jon Lee 2011-12-07 10:20:07 PST
Created attachment 118226 [details]
Patch with GNU build fixes
Comment 5 Gustavo Noronha (kov) 2011-12-07 10:29:30 PST
Comment on attachment 118226 [details]
Patch with GNU build fixes

Attachment 118226 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10752207
Comment 6 Jon Lee 2011-12-07 11:05:21 PST
Created attachment 118234 [details]
Fix gtk again
Comment 7 Collabora GTK+ EWS bot 2011-12-07 11:43:35 PST
Comment on attachment 118234 [details]
Fix gtk again

Attachment 118234 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10748211
Comment 8 Jon Lee 2011-12-07 11:51:07 PST
Created attachment 118247 [details]
Fix gtk again and adjust some xcodeproj settings and paths
Comment 9 Gustavo Noronha (kov) 2011-12-07 12:53:07 PST
Comment on attachment 118247 [details]
Fix gtk again and adjust some xcodeproj settings and paths

Attachment 118247 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10767181
Comment 10 Jon Lee 2011-12-07 13:00:05 PST
Created attachment 118257 [details]
Trying again with gtk
Comment 11 Darin Adler 2011-12-08 10:15:31 PST
Comment on attachment 118257 [details]
Trying again with gtk

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.cpp:62
> +NotificationPermissionRequestManager::~NotificationPermissionRequestManager()
> +{
> +}

Seems we should just let the compiler generate this.
Comment 12 Jon Lee 2011-12-08 10:18:55 PST
(In reply to comment #11)
> (From update of attachment 118257 [details])
> 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.cpp:62
> > +NotificationPermissionRequestManager::~NotificationPermissionRequestManager()
> > +{
> > +}
> 
> Seems we should just let the compiler generate this.

Will do. Thank you!
Comment 13 Jon Lee 2011-12-08 11:07:16 PST
Committed r102352: <http://trac.webkit.org/changeset/102352>