Bug 95099

Summary: [Mac] Add SPI to WK1 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: dglazkov, haraken, jberlin, jianli, sam, webkit-bug-importer, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.8   
Bug Depends on:    
Bug Blocks: 77969    
Attachments:
Description Flags
Patch
jberlin: review+, webkit.review.bot: commit-queue-
Archive of layout-test-results from gce-cr-linux-03 none

Jon Lee
Reported 2012-08-27 10:12:35 PDT
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.
Attachments
Patch (9.70 KB, patch)
2012-08-27 14:39 PDT, Jon Lee 		jberlin: review+
webkit.review.bot: commit-queue-
DetailsFormatted DiffDiff
Archive of layout-test-results from gce-cr-linux-03 (518.26 KB, application/zip)
2012-08-27 18:24 PDT, WebKit Review Bot 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2012-08-27 10:12:53 PDT
<rdar://problem/12180186>
Jon Lee
Comment 2 2012-08-27 14:39:05 PDT
Created attachment 160809 [details] Patch
Jon Lee
Comment 3 2012-08-27 14:40:45 PDT
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 4 2012-08-27 18:24:30 PDT
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
Comment 5 2012-08-27 18:24:32 PDT
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 6 2012-08-28 10:39:18 PDT
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 7 2012-08-28 11:59:34 PDT
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
Comment 8 2012-08-28 12:08:02 PDT
Committed r126909: <http://trac.webkit.org/changeset/126909>
Note You need to log in before you can comment on or make changes to this bug.