Bug 31554 - [Android] Android is missing the implementation of the GeolocationService iface.
Summary: [Android] Android is missing the implementation of the GeolocationService iface.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other Other
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-11-16 08:06 PST by Andrei Popescu
Modified: 2009-11-17 05:13 PST (History)
2 users (show)

See Also:


Attachments
Android implementation of the GeolocationService implementation (22.59 KB, patch)
2009-11-16 08:16 PST, Andrei Popescu
dglazkov: review-
Details | Formatted Diff | Diff
Android implementation of the GeolocationService implementation v2 (26.23 KB, patch)
2009-11-16 10:48 PST, Andrei Popescu
no flags Details | Formatted Diff | Diff
Android implementation of the GeolocationService implementation v2 (26.17 KB, patch)
2009-11-16 10:52 PST, Andrei Popescu
dglazkov: review+
benm: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrei Popescu 2009-11-16 08:06:13 PST
Each port of WebKit that wants to support Geolocation must implement the GeolocationService interface. So Android needs one too.
Comment 1 Andrei Popescu 2009-11-16 08:16:16 PST
Created attachment 43308 [details]
Android implementation of the GeolocationService implementation

The corresponding code in the Android source tree can be found at

http://android.git.kernel.org/?p=platform/external/webkit.git;a=tree;f=WebCore/platform/android;h=b66c3277d6de15bd67218256deeafa72925b557d;hb=refs/heads/eclair
Comment 2 Dimitri Glazkov (Google) 2009-11-16 09:15:04 PST
Comment on attachment 43308 [details]
Android implementation of the GeolocationService implementation

> +// GeolocationServiceBridge is the bridge to the Java implementation. It manages
> +// the lifetime of the Java object. It is an implementation detail of
> +// GeolocationServiceAndroid.
> +class GeolocationServiceBridge {

Can you split this out into a separate file?

> +    static PassRefPtr<Geoposition> convertLocationToGeoposition(JNIEnv *env, const jobject &location);

toGeoposition is shorter and more inline with the WebKit convention.

> +static const char* kJavaGeolocationServiceClass = "android/webkit/GeolocationService";

prefix k* is not necessary, just use standard camelCase.

> +enum kJavaGeolocationServiceClassMethods {
> +    GEOLOCATION_SERVICE_METHOD_INIT = 0,

GeolocationServiceMethodInit <-- casing here and elsewhere

> +    GEOLOCATION_SERVICE_METHOD_START,
> +    GEOLOCATION_SERVICE_METHOD_STOP,
> +    GEOLOCATION_SERVICE_METHOD_SET_ENABLE_GPS,
> +    GEOLOCATION_SERVICE_METHOD_COUNT,
> +};
Comment 3 Andrei Popescu 2009-11-16 10:48:53 PST
Created attachment 43314 [details]
Android implementation of the GeolocationService implementation v2
Comment 4 Andrei Popescu 2009-11-16 10:52:37 PST
Created attachment 43315 [details]
Android implementation of the GeolocationService implementation v2

The previous patch was missing new lines at the end of the new files.
Comment 5 Andrei Popescu 2009-11-16 10:54:59 PST
Thanks Dimitri,

(In reply to comment #2)
> (From update of attachment 43308 [details])
> > +// GeolocationServiceBridge is the bridge to the Java implementation. It manages
> > +// the lifetime of the Java object. It is an implementation detail of
> > +// GeolocationServiceAndroid.
> > +class GeolocationServiceBridge {
> 
> Can you split this out into a separate file?
> 

Sure, done.

> > +    static PassRefPtr<Geoposition> convertLocationToGeoposition(JNIEnv *env, const jobject &location);
> 
> toGeoposition is shorter and more inline with the WebKit convention.
>

Indeed. Changed.
 
> > +static const char* kJavaGeolocationServiceClass = "android/webkit/GeolocationService";
> 
> prefix k* is not necessary, just use standard camelCase.
> 

Done.

> > +enum kJavaGeolocationServiceClassMethods {
> > +    GEOLOCATION_SERVICE_METHOD_INIT = 0,
> 
> GeolocationServiceMethodInit <-- casing here and elsewhere
>

Fixed.
 
Andrei
Comment 6 Dimitri Glazkov (Google) 2009-11-16 11:09:48 PST
Comment on attachment 43315 [details]
Android implementation of the GeolocationService implementation v2

looks good.
Comment 7 Ben Murdoch 2009-11-17 04:28:20 PST
Comment on attachment 43315 [details]
Android implementation of the GeolocationService implementation v2

Will land this manually as the commit queue is currently disabled.
Comment 8 Ben Murdoch 2009-11-17 05:13:32 PST
Landed as r51071.