Bug 44322
| Summary: | Client-based geolocation missing error handling | ||
|---|---|---|---|
| Product: | WebKit | Reporter: | Mahesh Kulkarni <maheshk> |
| Component: | WebKit Misc. | Assignee: | Nobody <webkit-unassigned> |
| Status: | RESOLVED INVALID | ||
| Severity: | Normal | CC: | abecsi, hausmann, jarred, laszlo.gombos, ossy, steveblock |
| Priority: | P2 | ||
| Version: | 528+ (Nightly build) | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Bug Depends on: | |||
| Bug Blocks: | 42629 | ||
Mahesh Kulkarni
GeolocationControllerClient interface missing error handling incase of unavailability of user location.
if startUpdating() returns error from platform client listeners should be notified of POSITION_UNAVAILABLE error
| Attachments | ||
|---|---|---|
| Add attachment proposed patch, testcase, etc. |
Steve Block
Can you explain exactly what error handling is missing?
I don't think there's any need for GeolocationControllerClient::startUpdating() to return a boolean indicating success. If the client fails to start the location acquisition process, it should asynchronously call GeolocationController::errorOccurred().
The fact that GeolocationService::startUpdating() returns a boolean is awkward, as in the case of an error, it requires the Geolocation object to manage making the error callback asynchronous.
Jarred Nicholls
I agree with Steve. The fact that it's client-based alone should imply that the client asynchronously informs Geolocation by calling errorOccurred(), IMHO.
Mahesh Kulkarni
(In reply to comment #1)
> Can you explain exactly what error handling is missing?
>
> I don't think there's any need for GeolocationControllerClient::startUpdating() to return a boolean indicating success. If the client fails to start the location acquisition process, it should asynchronously call GeolocationController::errorOccurred().
>
> The fact that GeolocationService::startUpdating() returns a boolean is awkward, as in the case of an error, it requires the Geolocation object to manage making the error callback asynchronous.
Makes sense. Closing the bug. Thanks Steve and Jarred.