Bug 27690 - Don't override m_allowGeolocation set by ChromeClient::requestGeolocationPermissionForFrame
Summary: Don't override m_allowGeolocation set by ChromeClient::requestGeolocationPerm...
Status: RESOLVED DUPLICATE of bug 26993
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-26 06:22 PDT by Kwang Yul Seo
Modified: 2009-08-07 13:45 PDT (History)
4 users (show)

See Also:


Attachments
Geolocation patch (1.35 KB, patch)
2009-07-26 06:25 PDT, Kwang Yul Seo
eric: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kwang Yul Seo 2009-07-26 06:22:15 PDT
In Geolocation::requestPermission, m_allowGeolocation is overriden to InProgress after page->chrome()->requestGeolocationPermissionForFrame(m_frame, this) is called. 

In case requestGeolocationPermissionForFrame is implemented with a modal dialog and calls Geolocation::setIsAllowed(true) directly, m_allowGeolocation never becomes Yes.
Comment 1 Kwang Yul Seo 2009-07-26 06:25:38 PDT
Created attachment 33509 [details]
Geolocation patch

Don't override m_allowGeolocation.
Comment 2 Eric Seidel (no email) 2009-07-28 11:02:27 PDT
Whoever wrote the geolocation stuff should review this.

I assume this client call is expected to be async not synchronous?
Comment 3 Steve Block 2009-08-03 09:52:51 PDT
> I assume this client call is expected to be async not synchronous?
That's correct.  I'm not sure if there's any need to support synchronous client calls.

> Whoever wrote the geolocation stuff should review this.
This was written by Greg. However, I'm pretty sure that this patch doesn't fix the problem correctly.

If the supplied patch is used, and requestGeolocationPermissionsForFrame is implemented synchronously, watches will be called back twice, rather than once.

geolocationServicePositionChanged() assumes that if requestPermission() calls requestGeolocationPermissionsForFrame(), isAllowed() will return false immediately after requestPermission() returns. If requestGeolocationPermissionsForFrame() is implemented synchronously. this is not true.

So from geolocationServicePositionChanged(), requestPermission() will call requestGeolocationPermissionsForFrame(), which will synchronously call setIsAllowed(), call back into geolocationServicePositionChanged() and call back the one-shots and watchers. When requestPermission returns, isAllowed() will return true and the watchers will be called back a second time.
Comment 4 Eric Seidel (no email) 2009-08-07 13:34:05 PDT
Comment on attachment 33509 [details]
Geolocation patch

r- per above comments.
Comment 5 George Staikos 2009-08-07 13:45:16 PDT
This is identical to Yong's patch from our git repo which was checked in a while ago.

*** This bug has been marked as a duplicate of bug 26993 ***