Summary: | [WK2] Add permissions support to web notifications | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jon Lee <jonlee> | ||||||||||||||
Component: | WebKit2 | Assignee: | Jon Lee <jonlee> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | darin, gustavo.noronha, gustavo, sam, webkit-bug-importer, webkit.review.bot, xan.lopez, zoltan | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Mac | ||||||||||||||||
OS: | All | ||||||||||||||||
Attachments: |
|
Description
Jon Lee
2011-12-06 16:05:04 PST
Created attachment 118147 [details]
Patch
Created attachment 118150 [details]
Patch
Added new files to Windows vcproj
Comment on attachment 118150 [details] Patch Attachment 118150 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10756001 Created attachment 118226 [details]
Patch with GNU build fixes
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 Created attachment 118234 [details]
Fix gtk again
Comment on attachment 118234 [details] Fix gtk again Attachment 118234 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/10748211 Created attachment 118247 [details]
Fix gtk again and adjust some xcodeproj settings and paths
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 Created attachment 118257 [details]
Trying again with gtk
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. (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! Committed r102352: <http://trac.webkit.org/changeset/102352> |