Bug 95100 – [WK2] Add SPI to retrieve internal IDs for web notifications

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Jon Lee]

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.

[Radar WebKit Bug Importer]

<rdar://problem/12180208>

[Jon Lee]

Created attachment 160808 [details]
Patch

[Jon Lee]

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 on attachment 160808 [details]
Patch

Attachment 160808 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13622252

[Gyuyoung Kim]

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 on attachment 160808 [details]
Patch

Attachment 160808 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13639050

[Jon Lee]

Created attachment 160859 [details]
Patch

[Early Warning System Bot]

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]

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]

(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]

Created attachment 161154 [details]
Patch

[Alexey Proskuryakov]

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]

(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]

Committed r127019: <http://trac.webkit.org/changeset/127019>