[webkit-reviews] review denied: [Bug 27944] Geolocation error callback not called if permissions have already been denied : [Attachment 34779] Patch 1 for bug 27944

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied steveblock at google.com's request for
review:
Bug 27944: Geolocation error callback not called if permissions have already
been denied
https://bugs.webkit.org/show_bug.cgi?id=27944

Attachment 34779: Patch 1 for bug 27944
https://bugs.webkit.org/attachment.cgi?id=34779&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
We don't have a find method for this type?
126	for (GeoNotifierMap::iterator iter = m_watchers.begin(); iter !=
m_watchers.end(); ++iter) {
 127	     if (iter->second == notifier) {
 128		 m_watchers.remove(iter);
 129		 break;
 130	     }
 131	 }

Style:
67	   if (m_errorCallback) {
 68		m_errorCallback->handleEvent(m_fatalError.get());
 69	    }

No need to store it locally first:
 106	     RefPtr<PositionError> error =
PositionError::create(PositionError::PERMISSION_DENIED,
permissionDeniedErrorMessage);
 107	     notifier->setFatalError(error.release());

Aren't the two if (isDenied()) blocks your editing identical?  Cant' we share
code there?

Arguments are only named when the name adds clarity:
 92	    GeoNotifier(Geolocation* geolocation, PassRefPtr<PositionCallback>,
PassRefPtr<PositionErrorCallback>, PassRefPtr<PositionOptions>);
 119	 void fatalErrorOccurred(GeoNotifier* notifier);


Who would know about this code?  I certainly don't.

r- for the seemingly duplicated code.