993041 - Create one instance of the GonkGPSGeolocationProvider

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Description

[Bhavna Sharma]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20140314220517

Steps to reproduce:

On our branch we start the GonkGPSGeoLocationProvider at boot-up, this creates one instance. 

When the first HTML5 location request comes in , that creates one more instance of the provider because of the change in 
https://bugzilla.mozilla.org/show_bug.cgi?id=977725

i.e.
#ifdef MOZ_WIDGET_GONK
-  mProvider = do_GetService(GONK_GPS_GEOLOCATION_PROVIDER_CONTRACTID);
+  mProvider = do_CreateInstance(GONK_GPS_GEOLOCATION_PROVIDER_CONTRACTID);
#endif





Actual results:

2 instance of the provider got created


Expected results:

Only 1 instance of the provider should have been created

Comment 1

[Bhavna Sharma]

Attachment: 0001-Create-1-instance-of-the-GonkGPSGeolocation-provider.patch

Comment 2

[Bhavna Sharma]

Attachment: 0001-Bug-993041-Create-one-instance-of-the-GonkGPSGeoloca.patch

Comment 3

[Doug Turner (:dougt)]

Comment on attachment 8402875 [details] [diff] [review]
0001-Bug-993041-Create-one-instance-of-the-GonkGPSGeoloca.patch

Review of attachment 8402875 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine -- could you add a comment on why we need to get service instead of get instance here?

Comment 4

[Bhavna Sharma]

GonkGPSGeolocationProvider gets created and initialized at boot time. Multiple stacks like Time download, GPS, XTRA download, Network Location Provider pipe all get initialized at boot-up time. When the first HTML5 location request comes in, all the stacks are up and ready to start receiving the requests. There is no delay in processing due to initialization reasons.

do_getService gets hold of the already initialized component and starts processing requests immediately.
do_Createinstance will create multiple location stacks which is not right.

Comment 5

[Michael Vines [:m1] [:evilmachines]]

Bhavna, I think Doug was asking you to add such a comment directly into the patch itself so that the next time somebody is tempted to make a similar change they'll have some history.  You could also reference this bug number directly in the comment as well.

Comment 6

[Bhavna Sharma]

Attachment: 0001-Bug-993041-Create-one-instance-of-the-GonkGPSGeoloca.patch

Adds a note on why do_getservice is preferred to do_createinstance

Comment 7

[Michael Vines [:m1] [:evilmachines]]

Comment on attachment 8403069 [details] [diff] [review]
0001-Bug-993041-Create-one-instance-of-the-GonkGPSGeoloca.patch

Please put the comment directly in the source file instead of the commit text

Comment 8

[Bhavna Sharma]

Attachment: 0001-Bug-993041-Create-one-instance-of-the-GonkGPSGeoloca.patch

Add comment to source file itself for history

Comment 9

[Carsten Book [:Tomcat]]

landed on mozilla-inbound - congrats for submitting the first patch and thanks for contributing to mozilla!

https://hg.mozilla.org/integration/mozilla-inbound/rev/42e3849489db

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/42e3849489db

Comment 11

[Michael Vines [:m1] [:evilmachines]]

Kewl, thanks :dougt 4 the help