Bug 66759 - Remove Android-specific modifications to non-client-based Geolocation
Summary: Remove Android-specific modifications to non-client-based Geolocation
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Steve Block
URL:
Keywords:
Depends on:
Blocks: 40373 66688
  Show dependency tree / graph
 
Reported: 2011-08-23 04:10 PDT by Steve Block
Modified: 2011-10-18 05:55 PDT (History)
6 users (show)

See Also:

Attachments
Patch (5.47 KB, patch)
2011-08-23 04:59 PDT, Steve Block 		tonyg: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Block 2011-08-23 04:10:45 PDT 
Remove Android-specific modifications to non-client-based Geolocation
Comment 1 Steve Block 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.
Comment 2 Steve Block 2011-08-23 04:59:36 PDT 
Created attachment 104823 [details]
Patch
Comment 3 Steve Block 2011-08-23 05:46:24 PDT 
Committed r93596: <http://trac.webkit.org/changeset/93596>
Comment 4 Kenneth Rohde Christiansen 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.
Comment 5 Steve Block 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.
Comment 6 Kenneth Rohde Christiansen 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.
Comment 7 Steve Block 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.
Comment 8 Kenneth Rohde Christiansen 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.