Summary: | Client-based Geolocation does not handle multiple simultaneous requests | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Steve Block <steveblock> | ||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, grahamperrin, sam, steveblock | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Bug Depends on: | |||||||
Bug Blocks: | 40374 | ||||||
Attachments: |
|
Description
Steve Block
2010-06-03 17:01:41 PDT
See also: bug 39908. I'm told the original design was that one doesn't add an observer multiple times. Created attachment 57861 [details]
Patch
> I'm told the original design was that one doesn't add an observer multiple > times. No, the original Geolocation implementation by Greg uses multiple calls to startUpdating(). This is required so that the options for each request can be passed to the provider. When client-based Geolocation was added, each call to startUpdating() adds the observer to the controller. Note that this change fixes reentrant-error.html reentrant-success.html (see Bug 39908) so my patch removes these from the mac skipped list. maximum-age.html is still broken, due to more serious problems with the client-based implementation. Once this change lands, I'll update Bug 39908 to reflect this. + on the mac skipped list. reentrant-error.html required a minor tweak as the mac + GeolocationController can only provide an error code of 2 (POSITION_UNAVAILABLE) Maybe it would be useful to also have a test for reentrant "permission denied" error, since that uses a different code path, at least with client-based Geolocation. The patch looks good to me, but it ideally, Sam should review it, since it touches one of the assumptions that were believed true when writing client based geolocation code. > Maybe it would be useful to also have a test for reentrant "permission denied" > error, since that uses a different code path, at least with client-based > Geolocation. Sure, filed Bug 40174 Sam, could you take a look at this please? Comment on attachment 57861 [details]
Patch
Sam looked at this, and agrees that this is right. Thanks for fixing the problem!
Committed r60998: <http://trac.webkit.org/changeset/60998> |