[Webkit-unassigned] [Bug 83877] [GTK][WK2] Implement geolocation provider for the GTK port

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu May 31 10:22:10 PDT 2012


https://bugs.webkit.org/show_bug.cgi?id=83877


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #145056|review?                     |review-
               Flag|                            |




--- Comment #22 from Martin Robinson <mrobinson at webkit.org>  2012-05-31 10:22:10 PST ---
(From update of attachment 145056)
View in context: https://bugs.webkit.org/attachment.cgi?id=145056&action=review

Looks good, though I've listed a few issues below and it looks like the build failed.

> Source/WebKit2/UIProcess/API/gtk/WebKitGeolocationProvider.cpp:71
> +        WKGeolocationManagerRef wkGeolocationManager(WKContextGetGeolocationManager(m_wkContext.get()));
> +        WKGeolocationPositionRef wkGeolocationPosition(WKGeolocationPositionCreate(timestamp, latitude, longitude, accuracy));

I think you are leaking the position here. These are just pointers, so you should use the assignment style, not the constructor style:

WKGeolocationManagerRef wkGeolocationManager = WKContextGetGeolocationManager(m_wkContext.get());

> Source/WebKit2/UIProcess/API/gtk/WebKitGeolocationProvider.cpp:80
> +        WKGeolocationManagerRef wkGeolocationManager(WKContextGetGeolocationManager(m_wkContext.get()));

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitGeolocationProvider.cpp:108
> +    static GeolocationProviderClientInfo clientInfo;

I don't think you want to make this static. Theoretically there may be more than one WebKitWebContext.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list