Hook up the code for QtWebKit for ENABLE_NOTIFICAITONS build option.
Created attachment 49705 [details] proposed patch Map WebKit notifications to Qt's SystemTray API and implement DRT tracing. This patch does not implement the security part of WebKit notifications.
The first patch provides a very basic implementation and satisfies the build scripts. The complete solution (following additional patches) would need to address: - DRT tests for Qt port (make sure tests not executed for other ports as most ports do not implement this feature yet). - implement security - possible provide a Delegate API for QtWebKit to delegate the notification mechanism. To keep the patch size low, I figured the patch is complete enough for a review round.
Comment on attachment 49705 [details] proposed patch Is this really supposed to return a new one? #if ENABLE(NOTIFICATIONS) 436 NotificationPresenter* ChromeClientQt::notificationPresenter() const 437 { 438 return new NotificationPresenterClientQt; 439 } 440 #endif based on that method signature, I woudl expect it to return a reference to one. I expect you're leaking here. Spacing: void NotificationPresenterClientQt::notificationObjectDestroyed(Notification* notification) 92 { 93 notImplemented(); 94 95 } Spacing: void NotificationPresenterClientQt::requestPermission(SecurityOrigin* origin, PassRefPtr<VoidCallback> callback) 98 { 99 100 if (dumpNotification) r- for what appears to be a leak (or at least would confuse other developers into causing a leak).
Created attachment 51906 [details] 2nd try Thanks Eric for the review. Good catch on the leak. I changed the patch so that now each instance QWebPage of owns (creates and destroys) an instance of NotificationPresenterClientQt.
Comment on attachment 51906 [details] 2nd try Fine with me, but you have a spelling error in the ChangeLog "notificationa DRT"
Comment on attachment 51906 [details] 2nd try Committed as http://trac.webkit.org/changeset/57408 with the change in the ChangeLog suggested by Kenneth.
Next time please keep your eyes on bots or don't commit before you go offline! First I had to kick (make clean build) our bots because build system modifications usually make all test crash. Makefiles can't handle only build flag modifications. Second I had to modify 3 expected files (which you forgot) to make our very sad bots happy: http://trac.webkit.org/changeset/57412
http://trac.webkit.org/changeset/57412 might have broken SnowLeopard Intel Release (Tests)
(In reply to comment #8) > http://trac.webkit.org/changeset/57412 might have broken SnowLeopard Intel > Release (Tests) false positive alarm, it was an absolutely unrelated patch.
http://trac.webkit.org/changeset/57413 might have broken SnowLeopard Intel Release (Tests)
(In reply to comment #7) Ossy, thanks for you help ! > Next time please keep your eyes on bots or don't commit before you go offline! I did just that but I obviously missed the issues with the Qt bot. > First I had to kick (make clean build) our bots because build system > modifications usually make all test crash. Makefiles can't handle > only build flag modifications. First, the build should fail instead of creating an unusable binary. Had the build fail, I would have caught it before even committing. Any idea how to fix this ? Second, there should be a way for committees to kick the bot. > Second I had to modify 3 expected files (which you forgot) to make > our very sad bots happy: http://trac.webkit.org/changeset/57412 Sorry I missed that, thanks for your help.