[webkit-reviews] review requested: [Bug 59199] [Qt][WK2] Implement geolocation provider for qt port : [Attachment 99976] patch

[bugzilla-daemon at webkit.org]

Mahesh Kulkarni <mahesh.kulkarni at nokia.com> has asked  for review:
Bug 59199: [Qt][WK2] Implement geolocation provider for qt port
https://bugs.webkit.org/show_bug.cgi?id=59199

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

------- Additional Comments from Mahesh Kulkarni <mahesh.kulkarni at nokia.com>
Thanks Yael for review. Updated as per comments from Yael.


> > Source/WebKit2/UIProcess/API/C/WKGeolocationManager.h:38
> > -typedef void
(*WKGeolocationProviderStartUpdatingCallback)(WKGeolocationManagerRef
geolocationManager, const void* clientInfo);
> > -typedef void
(*WKGeolocationProviderStopUpdatingCallback)(WKGeolocationManagerRef
geolocationManager, const void* clientInfo);
> > +typedef void
(*WKGeolocationProviderStartUpdatingCallback)(WKGeolocationManagerRef
geolocationManager, void* clientInfo);
> > +typedef void
(*WKGeolocationProviderStopUpdatingCallback)(WKGeolocationManagerRef
geolocationManager, void* clientInfo);
> >  
> 
> Wouldn't removing the const be a problem for other ports?

No, because no other ports have geolocation provider implemented yet.

> > Source/WebKit2/UIProcess/qt/WebGeolocationProviderQt.h:57
> > +	 WKRetainPtr<WKGeolocationPositionRef> m_lastPosition;
> 
> Can you use RefPtr? Qt is the only port to use WKRetainPtr, and it should
probably go away.

The class isn't RefCounted yet. Shall take up this separately.