Bug 95100

Summary: [WK2] Add SPI to retrieve internal IDs for web notifications
Product: WebKit Reporter: Jon Lee <jonlee>
Component: Tools / TestsAssignee: Jon Lee <jonlee>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, ap, cmarcelo, d-r, gustavo, hausmann, jberlin, kenneth, menard, ossy, philn, sam, tmpsantos, tonikitoo, vestbo, webkit-bug-importer, webkit.review.bot, xan.lopez, zherczeg, zoltan
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 77969    
Attachments:
Description Flags
Patch
none
Patch
none
Patch ap: review+

Jon Lee
Reported 2012-08-27 10:14:57 PDT
The IDs used internally by WebKit2 to keep track of notifications need to be exposed to WTR. WTR will have a notification provider, and for that provider to simulate a user click on a shown notification, it needs to be able to map the JS Notification object to an ID.
Attachments
Patch (11.55 KB, patch)
2012-08-27 14:37 PDT, Jon Lee 		no flags
DetailsFormatted DiffDiff
Patch (11.64 KB, patch)
2012-08-27 16:54 PDT, Jon Lee 		no flags
DetailsFormatted DiffDiff
Patch (7.36 KB, patch)
2012-08-29 00:24 PDT, Jon Lee 		ap: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2012-08-27 10:15:05 PDT
<rdar://problem/12180208>
Jon Lee
Comment 2 2012-08-27 14:37:42 PDT
Created attachment 160808 [details] Patch
Jon Lee
Comment 3 2012-08-27 14:40:57 PDT
The WebCore part of the patch is the same as that in 95099. I included it in both so that I could try to parallelize the approval paths.
Build Bot
Comment 4 2012-08-27 15:07:09 PDT
Comment on attachment 160808 [details] Patch Attachment 160808 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13622252
Gyuyoung Kim
Comment 5 2012-08-27 15:33:26 PDT
Comment on attachment 160808 [details] Patch Attachment 160808 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13637046
Early Warning System Bot
Comment 6 2012-08-27 15:53:07 PDT
Comment on attachment 160808 [details] Patch Attachment 160808 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13639050
Jon Lee
Comment 7 2012-08-27 16:54:17 PDT
Created attachment 160859 [details] Patch
Early Warning System Bot
Comment 8 2012-08-27 18:00:44 PDT
Comment on attachment 160859 [details] Patch Attachment 160859 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13645146
Jon Lee
Comment 9 2012-08-28 10:59:02 PDT
CC'ing a couple Qt guys for assistance with the qt-wk2 EWS error: ...ews/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:56:36: error: WebCore/JSNotification.h: No such file or directory I need to include a derived source header (JSNotification.h) as a private header in WebCore for WK2. (Which you can see reflected in the mac port in WebCore.xcodeproj/project.pbxproj.) I'm not sure how to do that for Qt. Any pointers?
Csaba Osztrogonác
Comment 10 2012-08-28 23:43:59 PDT
(In reply to comment #9) > CC'ing a couple Qt guys for assistance with the qt-wk2 EWS error: > > ...ews/Source/WebKit2/WebProcess/InjectedBundle/InjectedBundle.cpp:56:36: error: WebCore/JSNotification.h: No such file or directory > > I need to include a derived source header (JSNotification.h) as a private header in WebCore for WK2. (Which you can see reflected in the mac port in WebCore.xcodeproj/project.pbxproj.) I'm not sure how to do that for Qt. Any pointers? Including a derived source header as forwarding header is tricky a little bit on Qt. The following snippet will make the build happy: --- a/Source/WebKit2/DerivedSources.pri +++ b/Source/WebKit2/DerivedSources.pri @@ -22,6 +22,7 @@ WEBCORE_GENERATED_HEADERS_FOR_WEBKIT2 += \ $$WEBCORE_GENERATED_SOURCES_DIR/JSElement.h \ $$WEBCORE_GENERATED_SOURCES_DIR/JSHTMLElement.h \ $$WEBCORE_GENERATED_SOURCES_DIR/JSNode.h \ + $$WEBCORE_GENERATED_SOURCES_DIR/JSNotification.h \ $$WEBCORE_GENERATED_SOURCES_DIR/JSRange.h \
Jon Lee
Comment 11 2012-08-29 00:24:29 PDT
Created attachment 161154 [details] Patch
Alexey Proskuryakov
Comment 12 2012-08-29 09:49:11 PDT
Comment on attachment 161154 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=161154&action=review > Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp:115 > +uint64_t WebNotificationManager::notificationIDForTesting(WebCore::Notification* notification) No need for WebCore prefix - there is a using directive in this file. > Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h:75 > // For testing purposes only. > void removeAllPermissionsForTesting(); > + uint64_t notificationIDForTesting(WebCore::Notification*); The comment looks redundant.
Jon Lee
Comment 13 2012-08-29 10:43:31 PDT
(In reply to comment #12) > (From update of attachment 161154 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=161154&action=review > > > Source/WebKit2/WebProcess/Notifications/WebNotificationManager.cpp:115 > > +uint64_t WebNotificationManager::notificationIDForTesting(WebCore::Notification* notification) > > No need for WebCore prefix - there is a using directive in this file. > > > Source/WebKit2/WebProcess/Notifications/WebNotificationManager.h:75 > > // For testing purposes only. > > void removeAllPermissionsForTesting(); > > + uint64_t notificationIDForTesting(WebCore::Notification*); > > The comment looks redundant. Fixed. Thanks!
Jon Lee
Comment 14 2012-08-29 10:47:25 PDT
Committed r127019: <http://trac.webkit.org/changeset/127019>
Note You need to log in before you can comment on or make changes to this bug.