RESOLVED FIXED 66759
Remove Android-specific modifications to non-client-based Geolocation 
https://bugs.webkit.org/show_bug.cgi?id=66759
Summary Remove Android-specific modifications to non-client-based Geolocation
Steve Block
Reported 2011-08-23 04:10:45 PDT
Remove Android-specific modifications to non-client-based Geolocation
Attachments
Patch (5.47 KB, patch)
2011-08-23 04:59 PDT, Steve Block 		tonyg: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Steve Block
Comment 1 2011-08-23 04:57:11 PDT
Geolocation::suspend()/resume() and GeolocationService::suspend()/resume()were first added in http://trac.webkit.org/changeset/38018 Geolocation::suspend()/resume() were removed in http://trac.webkit.org/changeset/56188 but reinstated for Android in http://trac.webkit.org/changeset/58061. When GeolocationClient implementations for GTK (http://trac.webkit.org/changeset/38765) and EFL (http://trac.webkit.org/changeset/81556) were added, they added empty stubs for suspend() and resume() even though these methods are only called on Android. The Android port of WebKit is being removed in Bug 66688, so it should be safe to remove the methods on Geolocation and on the clients.
Steve Block
Comment 2 2011-08-23 04:59:36 PDT
Created attachment 104823 [details] Patch
Steve Block
Comment 3 2011-08-23 05:46:24 PDT
Committed r93596: <http://trac.webkit.org/changeset/93596>
Kenneth Rohde Christiansen
Comment 4 2011-10-18 04:28:23 PDT
Well this code is used by Qt on our branch and it is used by iOS as well.
Steve Block
Comment 5 2011-10-18 04:36:23 PDT
> Well this code is used by Qt on our branch and it is used by iOS as well. Sorry, I wasn't aware of this. Wasn't it you that reviewed http://trac.webkit.org/changeset/56188 which removed them previously? Do these methods need to be present upstream? If so, we should add comments to make this clear so they're not deleted again in the future.
Kenneth Rohde Christiansen
Comment 6 2011-10-18 05:48:42 PDT
We are going to upstream all of our suspend/resume code, so they will be needed yes. We will readd them as part of that.
Steve Block
Comment 7 2011-10-18 05:52:54 PDT
> We will readd them as part of that. OK Do you have plans to switch to the client-based Geolocation implementation? There's consensus that it's the right approach and that we should remove the non-client-based implementation. It would also remove the need for add()/resume() as all this logic can be handled by the client. See Bug 40373.
Kenneth Rohde Christiansen
Comment 8 2011-10-18 05:55:36 PDT
(In reply to comment #7) > > We will readd them as part of that. > OK > > Do you have plans to switch to the client-based Geolocation implementation? There's consensus that it's the right approach and that we should remove the non-client-based implementation. It would also remove the need for add()/resume() as all this logic can be handled by the client. See Bug 40373. Yes, we will be using that, sure.
Note You need to log in before you can comment on or make changes to this bug.