27690 2009-07-26 06:22:15 -0700 Don't override m_allowGeolocation set by ChromeClient::requestGeolocationPermissionForFrame 2009-08-07 13:45:16 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED DUPLICATE 26993 P2 Normal --- 0 skyul webkit-unassigned bolsinga eric staikos steveblock oldest_to_newest 134807 0 skyul 2009-07-26 06:22:15 -0700 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. 134808 1 33509 skyul 2009-07-26 06:25:38 -0700 Created attachment 33509 Geolocation patch Don't override m_allowGeolocation. 135279 2 eric 2009-07-28 11:02:27 -0700 Whoever wrote the geolocation stuff should review this. I assume this client call is expected to be async not synchronous? 136669 3 steveblock 2009-08-03 09:52:51 -0700 > 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. 138103 4 33509 eric 2009-08-07 13:34:05 -0700 Comment on attachment 33509 Geolocation patch r- per above comments. 138114 5 staikos 2009-08-07 13:45:16 -0700 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 *** 33509 2009-07-26 06:25:38 -0700 2009-08-07 13:34:04 -0700 Geolocation patch geolocation.patch text/plain 1380 skyul SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NjQwMSkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTcgQEAKKzIwMDktMDctMjYgIEt3YW5nIFl1bCBTZW8gIDxza3l1bEBjb21wYW55 MTAwLm5ldD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBEb24ndCBvdmVycmlkZSBtX2FsbG93R2VvbG9jYXRpb24gc2V0IGJ5IENocm9tZUNsaWVudDo6 cmVxdWVzdEdlb2xvY2F0aW9uUGVybWlzc2lvbkZvckZyYW1lCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yNzY5MAorCisgICAgICAgIG1fYWxsb3dHZW9s b2NhdGlvbiBpcyBvdmVycmlkZW4gdG8KKyAgICAgICAgSW5Qcm9ncmVzcyBhZnRlciBDaHJvbWVD bGllbnQ6OnJlcXVlc3RHZW9sb2NhdGlvblBlcm1pc3Npb25Gb3JGcmFtZQorICAgICAgICBpcyBj YWxsZWQuCisKKyAgICAgICAgKiBwYWdlL0dlb2xvY2F0aW9uLmNwcDoKKyAgICAgICAgKFdlYkNv cmU6Okdlb2xvY2F0aW9uOjpyZXF1ZXN0UGVybWlzc2lvbik6CisKIDIwMDktMDctMTYgIFNoaW5p Y2hpcm8gSGFtYWppICA8aGFtYWppQGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBi eSBPbGl2ZXIgSHVudC4KSW5kZXg6IFdlYkNvcmUvcGFnZS9HZW9sb2NhdGlvbi5jcHAKPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gV2ViQ29yZS9wYWdlL0dlb2xvY2F0aW9uLmNwcAkocmV2aXNpb24gNDY0MDEpCisr KyBXZWJDb3JlL3BhZ2UvR2VvbG9jYXRpb24uY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0yNjQsMTAg KzI2NCwxMCBAQCB2b2lkIEdlb2xvY2F0aW9uOjpyZXF1ZXN0UGVybWlzc2lvbigpCiAgICAgaWYg KCFwYWdlKQogICAgICAgICByZXR1cm47CiAgICAgCisgICAgbV9hbGxvd0dlb2xvY2F0aW9uID0g SW5Qcm9ncmVzczsKKwogICAgIC8vIEFzayB0aGUgY2hyb21lOiBpdCBtYWludGFpbnMgdGhlIGdl b2xvY2F0aW9uIGNoYWxsZW5nZSBwb2xpY3kgaXRzZWxmLgogICAgIHBhZ2UtPmNocm9tZSgpLT5y ZXF1ZXN0R2VvbG9jYXRpb25QZXJtaXNzaW9uRm9yRnJhbWUobV9mcmFtZSwgdGhpcyk7Ci0gICAg Ci0gICAgbV9hbGxvd0dlb2xvY2F0aW9uID0gSW5Qcm9ncmVzczsKIH0KIAogdm9pZCBHZW9sb2Nh dGlvbjo6Z2VvbG9jYXRpb25TZXJ2aWNlUG9zaXRpb25DaGFuZ2VkKEdlb2xvY2F0aW9uU2Vydmlj ZSogc2VydmljZSkK