[webkit-reviews] review granted: [Bug 40002] Need Geolocation LayoutTest to test case where permission has neither been granted nor denied : [Attachment 63340] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Steve Block
<steveblock at google.com>'s request for review:
Bug 40002: Need Geolocation LayoutTest to test case where permission has
neither been granted nor denied
https://bugs.webkit.org/show_bug.cgi?id=40002

Attachment 63340: Patch
https://bugs.webkit.org/attachment.cgi?id=63340&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+#import <Foundation/NSSet.h>

This isn't needed, please don't add this line.

+    if (m_pendingGeolocationPermissionListeners != nil && !m_timer)

We don't compare to null, so this can be just
(m_pendingGeolocationPermissionListeners && !m_timer)

+	 if (!m_pendingGeolocationPermissionListeners)
+	     m_pendingGeolocationPermissionListeners = [[NSMutableSet set]
retain];

I see that m_pendingGeolocationPermissionListeners is released in timerFired,
but shouldn't it be also released in dealloc method - in case the timer doesn't
get a chance to fire?

+    NSEnumerator* enumerator = [m_pendingGeolocationPermissionListeners
objectEnumerator];

Should we be iterating a copy? It seems that a client might add listeners when
handling allow/deny.

r=me