Closed Bug 993041 Opened 10 years ago Closed 10 years ago

Create one instance of the GonkGPSGeolocationProvider

Categories

(Core :: DOM: Geolocation, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: sbhavna, Assigned: sbhavna)

Details

Attachments

(1 file, 3 obsolete files)

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
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #8402856 - Flags: checkin?(dougt)
Attachment #8402875 - Flags: review?(dougt)
Attachment #8402856 - Attachment is obsolete: true
Attachment #8402856 - Flags: checkin?(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?
Attachment #8402875 - Flags: review?(dougt) → review-
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.
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.
Adds a note on why do_getservice is preferred to do_createinstance
Attachment #8402875 - Attachment is obsolete: true
Attachment #8403069 - Flags: review?(dougt)
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
Attachment #8403069 - Flags: review?(dougt)
Add comment to source file itself for history
Attachment #8403069 - Attachment is obsolete: true
Attachment #8403417 - Flags: review?(dougt)
Attachment #8403417 - Flags: review?(dougt) → review+
Keywords: checkin-needed
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
Assignee: nobody → sbhavna
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/42e3849489db
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Kewl, thanks :dougt 4 the help
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: