Bug 146228 – [WK1] WebAllowDenyPolicyListener.denyOnlyThisRequest() should not start a new permission request

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

On WebKit1, WebAllowDenyPolicyListener.denyOnlyThisRequest() currently denies the pending request (as is expected) but then starts a new permission requests right away (not expected).
This means that if the client application keeps calling WebAllowDenyPolicyListener.denyOnlyThisRequest(), we'll end up in an infinite loop, even though the application did not make any new geolocation requests.

[Chris Dumez]

rdar://problem/15179262

[Chris Dumez]

Created attachment 255391 [details]
Patch

[Daniel Bates]

Comment on attachment 255391 [details]
Patch

OK.

[Chris Dumez]

Comment on attachment 255391 [details]
Patch

Clearing flags on attachment: 255391

Committed r185860: <http://trac.webkit.org/changeset/185860>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Benjamin Poulain]

This looks fishy as hell. I have a hard time believing this is correct.

You don't even need to remove the object from the GeolocationController? Can you just leave the watcher pending like that? No need to call their callback?

[Chris Dumez]

(In reply to comment #6)
> This looks fishy as hell. I have a hard time believing this is correct.

Ok, we can discuss this tomorrow. The app I was testing uses getCurrentPosition(), not watchPosition(). Your comments seem to be related to watchers so it is possible I missed something there. 

> You don't even need to remove the object from the GeolocationController?
> Can you just leave the watcher pending like that? No need to call their callback?

Maybe I misunderstand the question but the watcher's error callback *does* get called with a PermissionDenied error when Geolocation::setAllowed(false) is called.

The GeoNotifier will then call Geolocation::fatalErrorOccurred(), which will unregister the watcher or the "oneShot" from Geolocation. If there are no more listeners, Geolocation will then remove itself from the GeolocationController.

[Benjamin Poulain]

(In reply to comment #7)
> (In reply to comment #6)
> > This looks fishy as hell. I have a hard time believing this is correct.
> 
> Ok, we can discuss this tomorrow. The app I was testing uses
> getCurrentPosition(), not watchPosition(). Your comments seem to be related
> to watchers so it is possible I missed something there. 
> 
> > You don't even need to remove the object from the GeolocationController?
> > Can you just leave the watcher pending like that? No need to call their callback?
> 
> Maybe I misunderstand the question but the watcher's error callback *does*
> get called with a PermissionDenied error when Geolocation::setAllowed(false)
> is called.
> 
> The GeoNotifier will then call Geolocation::fatalErrorOccurred(), which will
> unregister the watcher or the "oneShot" from Geolocation. If there are no
> more listeners, Geolocation will then remove itself from the
> GeolocationController.

My bad, I misunderstood what this was for. I did not see this was only called after a proper deny().