Summary: | [WK2] Add SPI to retrieve internal IDs for web notifications | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Jon Lee <jonlee> | ||||||||
Component: | Tools / Tests | Assignee: | 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
Jon Lee
2012-08-27 10:14:57 PDT
Created attachment 160808 [details]
Patch
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. Comment on attachment 160808 [details] Patch Attachment 160808 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/13622252 Comment on attachment 160808 [details] Patch Attachment 160808 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13637046 Comment on attachment 160808 [details] Patch Attachment 160808 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13639050 Created attachment 160859 [details]
Patch
Comment on attachment 160859 [details] Patch Attachment 160859 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13645146 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? (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 \ Created attachment 161154 [details]
Patch
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. (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! Committed r127019: <http://trac.webkit.org/changeset/127019> |