1308600 - Remove ServiceReady, GetCurrentPositionReady, and WatchPositionReady methods from Geolocation

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Description

[Josh Matthews [:jdm]]

Geolocation::ServiceReady is no longer used by any code, so we should remove it. That means that GetCurrentPositionReady and WatchPositionReady only have one caller each, so we should merge each of those methods with GetCurrentPosition and WatchPosition. This will simplify the code in the file and make it easier to reason about.

Code: dom/geolocation/Geolocation.cpp, dom/geolocation/Geolocation.h

No need to run any tests; if the changes compile, then it's good enough to submit a patch for review.

Comment 1

[manu4vyas]

i a beginner and unable to find code , could you please specify where is dom/geolocation directory

Comment 2

[Adam Velebil]

Attachment: Removed ServiceReady(), Merged *Ready methods into GetCurrentPosition, WatchPosition

Attached patch for the suggested changes.

Comment 3

[Josh Matthews [:jdm]]

Comment on attachment 8799149 [details] [diff] [review]
Removed ServiceReady(), Merged *Ready methods into GetCurrentPosition, WatchPosition

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

This looks great! We'll need to add a commit message like "Bug 1308600 - Remove unused ServiceReady method from Geolocation. r=jdm" to the patch as well.

Comment 4

[Adam Velebil]

Attachment: Patch with commit message

Hi Josh, I added the commit message (from instructions here: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch).

Comment 5

[Josh Matthews [:jdm]]

Thanks! Do you have access to our tryserver (https://wiki.mozilla.org/ReleaseEngineering/TryServer)? If not, please apply for level 1 access by following the steps at https://www.mozilla.org/hacking/committer/ ; I will vouch for you. This will allow you to ensure that your changes build on the major supported platforms (windows, linux, mac).

Comment 6

[Adam Velebil]

Thanks for help, Josh. I am continuing with the ticket, I (hopefully) got a correct setup of the accounts to be able to push to try. This will be my next step.

Comment 7

[Adam Velebil]

I let the tryserver to build with my patch. 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f953e9e3ef2
Josh, what should be the next step?

Comment 8

[Josh Matthews [:jdm]]

Looks good! Go ahead and add the `checkin-needed` keyword to the Keywords field in this bug, and then the patch will be automatically merged for you.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c784f8b12ed
Remove unused ServiceReady method from Geolocation. r=jdm

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4c784f8b12ed