Bug 95099 – [Mac] Add SPI to WK1 to retrieve internal IDs for web notifications

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Jon Lee]

The IDs used internally by Mac WebKit to keep track of notifications need to be exposed to DRT. DRT 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/12180186>

[Jon Lee]

Created attachment 160809 [details]
Patch

[Jon Lee]

The WebCore part of the patch is the same as that in 95100. I included it in both so that I could try to parallelize the approval paths.

[WebKit Review Bot]

Comment on attachment 160809 [details]
Patch

Attachment 160809 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/13640192

New failing tests:
animations/suspend-resume-animation-events.html

[WebKit Review Bot]

Created attachment 160879 [details]
Archive of layout-test-results from gce-cr-linux-03

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: gce-cr-linux-03  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.39-gcg-201203291735-x86_64-with-Ubuntu-10.04-lucid

[Jessie Berlin]

Comment on attachment 160809 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=160809&action=review

> Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm:238
> +    return [it->second.get() notificationID];

Could this be simplified to

[m_notificationMap.get(notification).get() notificationID]

if the HashMap empty value is a RetainPtr with a nil value?

> Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm:242
> +#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)

Why do you need to duplicate the guard here? It looks like you can just remove the above #endif and this #if.

[Jon Lee]

Comment on attachment 160809 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=160809&action=review

>> Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm:238
>> +    return [it->second.get() notificationID];
> 
> Could this be simplified to
> 
> [m_notificationMap.get(notification).get() notificationID]
> 
> if the HashMap empty value is a RetainPtr with a nil value?

Indeed it can!

>> Source/WebKit/mac/WebCoreSupport/WebNotificationClient.mm:242
>> +#if ENABLE(NOTIFICATIONS) || ENABLE(LEGACY_NOTIFICATIONS)
> 
> Why do you need to duplicate the guard here? It looks like you can just remove the above #endif and this #if.

Done. I didn't want to visually implementation of two different classes within the same #if guard, but I agree it's extra code.

[Jon Lee]

Committed r126909: <http://trac.webkit.org/changeset/126909>