146228 2015-06-22 19:46:59 -0700 [WK1] WebAllowDenyPolicyListener.denyOnlyThisRequest() should not start a new permission request 2015-06-22 22:24:25 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 cdumez cdumez aestes benjamin dbates oldest_to_newest 1104003 0 cdumez 2015-06-22 19:46:59 -0700 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. 1104004 1 cdumez 2015-06-22 19:47:07 -0700 rdar://problem/15179262 1104007 2 255391 cdumez 2015-06-22 19:58:54 -0700 Created attachment 255391 Patch 1104012 3 255391 dbates 2015-06-22 20:54:02 -0700 Comment on attachment 255391 Patch OK. 1104013 4 255391 cdumez 2015-06-22 20:55:15 -0700 Comment on attachment 255391 Patch Clearing flags on attachment: 255391 Committed r185860: <http://trac.webkit.org/changeset/185860> 1104014 5 cdumez 2015-06-22 20:55:18 -0700 All reviewed patches have been landed. Closing bug. 1104017 6 benjamin 2015-06-22 21:25:51 -0700 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? 1104021 7 cdumez 2015-06-22 22:18:48 -0700 (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. 1104022 8 benjamin 2015-06-22 22:24:25 -0700 (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(). 255391 2015-06-22 19:58:54 -0700 2015-06-22 20:55:15 -0700 Patch bug-146228-20150622195811.patch text/plain 3867 cdumez U3VidmVyc2lvbiBSZXZpc2lvbjogMTg1ODI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNzE2YWU3ODVlNGRjM2U5 NGRmZTM4ODkxZTE4NjgxMzE2NTBiZDZjZS4uMWNjZjgyZTA0MmVmN2FmNjQyYWZkZDJjMDAxMjg5 MTUyZDlmOTBiNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE4IEBACisyMDE1LTA2LTIyICBDaHJp cyBEdW1leiAgPGNkdW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgW1dLMV0gV2ViQWxsb3dEZW55 UG9saWN5TGlzdGVuZXIuZGVueU9ubHlUaGlzUmVxdWVzdCgpIHNob3VsZCBub3Qgc3RhcnQgYSBu ZXcgcGVybWlzc2lvbiByZXF1ZXN0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0xNDYyMjgKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzE1MTc5MjYyPgor CisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEFkZCBHZW9s b2NhdGlvbjo6cmVzZXRJc0FsbG93ZWQoKSBBUEkgdGhhdCBtZXJlbHkgcmVzZXRzCisgICAgICAg IG1fYWxsb3dHZW9sb2NhdGlvbiB0byBVbmtub3duLCBzbyB0aGF0IHdlIHdpbGwgcmVxdWVzdCB0 aGUgcGVybWlzc2lvbgorICAgICAgICBhZ2FpbiB0aGUgbmV4dCB0aW1lIGEgcG9zaXRpb24gaXMg cmVxdWVzdGVkLgorCisgICAgICAgICogTW9kdWxlcy9nZW9sb2NhdGlvbi9HZW9sb2NhdGlvbi5o OgorICAgICAgICAoV2ViQ29yZTo6R2VvbG9jYXRpb246OnJlc2V0SXNBbGxvd2VkKToKKwogMjAx NS0wNi0yMiAgRGFuaWVsIEJhdGVzICA8ZGFiYXRlc0BhcHBsZS5jb20+CiAKICAgICAgICAgQVg6 IFVJIEF1dG9tYXRpb24gY2Fubm90IGZpbmQgQXV0b0ZpbGwgb3Igc2VhcmNoIGNhbmNlbCBidXR0 b25zCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L21hYy9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi S2l0L21hYy9DaGFuZ2VMb2cKaW5kZXggZTc5Y2QwOWUxY2MwNmRjYTc4NjVkMTgyM2MzMzBlZjYz NjZiZDljNC4uOTkyNTM4YzEzN2QwZjBjMzQ0NTE3MTUwMGVlMmYyYWYwMTM2NmUwNCAxMDA2NDQK LS0tIGEvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvbWFj L0NoYW5nZUxvZwpAQCAtMSwzICsxLDI5IEBACisyMDE1LTA2LTIyICBDaHJpcyBEdW1leiAgPGNk dW1lekBhcHBsZS5jb20+CisKKyAgICAgICAgW1dLMV0gV2ViQWxsb3dEZW55UG9saWN5TGlzdGVu ZXIuZGVueU9ubHlUaGlzUmVxdWVzdCgpIHNob3VsZCBub3Qgc3RhcnQgYSBuZXcgcGVybWlzc2lv biByZXF1ZXN0CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xNDYyMjgKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzE1MTc5MjYyPgorCisgICAgICAgIFJl dmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIENhbGwgdGhlIG5ldyBHZW9sb2Nh dGlvbjo6cmVzZXRJc0FsbG93ZWQoKSBBUEkgYWZ0ZXIgZGVueWluZyB0aGUKKyAgICAgICAgY3Vy cmVudCByZXF1ZXN0LCBpbnN0ZWFkIG9mIGNhbGxpbmcKKyAgICAgICAgR2VvbG9jYXRpb246OnJl c2V0QWxsR2VvbG9jYXRpb25QZXJtaXNzaW9uKCkuIEluIGFkZGl0aW9uIHRvIHJlc2V0dGluZwor ICAgICAgICBtX2FsbG93R2VvbG9jYXRpb24gdG8gVW5rbm93biwgdGhlIGxhdHRlciB3b3VsZCBh bHNvIHN0YXJ0IGEgbmV3CisgICAgICAgIHBlcm1pc3Npb24gcmVxdWVzdC4gSG93ZXZlciwgZm9y CisgICAgICAgIFdlYkFsbG93RGVueVBvbGljeUxpc3RlbmVyLmRlbnlPbmx5VGhpc1JlcXVlc3Qo KSwgd2UgcmVhbGx5IG9ubHkgd2FudAorICAgICAgICB0byBkZW55IHRoZSBjdXJyZW50IHJlcXVl c3QgYW5kIHRoZW4gcmVzZXQgbV9hbGxvd0dlb2xvY2F0aW9uIHRvCisgICAgICAgIFVua25vd24g c28gdGhhdCBwZXJtaXNzaW9uIGlzIHJlcXVlc3RlZCBhZ2FpbiBsYXRlciBpZiB0aGUgYXBwIHJl cXVlc3RzCisgICAgICAgIGEgcG9zaXRpb24gYWdhaW4uCisKKyAgICAgICAgVGhlIHByZXZpb3Vz IGltcGxlbWVudGF0aW9uIG1lYW50IHRoYXQgaWYgdGhlIGNsaWVudCBhcHBsaWNhdGlvbiBrZWVw cworICAgICAgICBjYWxsaW5nIFdlYkFsbG93RGVueVBvbGljeUxpc3RlbmVyLmRlbnlPbmx5VGhp c1JlcXVlc3QoKSwgd2Ugd291bGQgZW5kCisgICAgICAgIHVwIGluIGFuIGluZmluaXRlIGxvb3Ag KHJlcXVlc3RpbmcgZm9yIHBlcm1pc3Npb24pLCBldmVuIHRob3VnaCB0aGUKKyAgICAgICAgYXBw bGljYXRpb24gZGlkIG5vdCBtYWtlIGFueSBuZXcgZ2VvbG9jYXRpb24gcmVxdWVzdHMuCisKKyAg ICAgICAgKiBXZWJDb3JlU3VwcG9ydC9XZWJHZW9sb2NhdGlvbkNsaWVudC5tbToKKyAgICAgICAg KC1bV2ViR2VvbG9jYXRpb25Qb2xpY3lMaXN0ZW5lciBkZW55T25seVRoaXNSZXF1ZXN0XSk6CisK IDIwMTUtMDYtMjAgIFRpbSBIb3J0b24gIDx0aW1vdGh5X2hvcnRvbkBhcHBsZS5jb20+CiAKICAg ICAgICAgRGVzZWxlY3Rpb24gb2YgdGV4dCBjYXVzZXMgYSBub3RpY2VhYmxlIGp1bXAgb24gZm9y Y2UgdG91Y2ggbWFjaGluZXMKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvZ2Vv bG9jYXRpb24vR2VvbG9jYXRpb24uaCBiL1NvdXJjZS9XZWJDb3JlL01vZHVsZXMvZ2VvbG9jYXRp b24vR2VvbG9jYXRpb24uaAppbmRleCA4ZmY5ZWUzYTBkNzcwMTZkOGY0OTBjMTJmZWVjNWIyNmRh NTZhYTdjLi44N2Q0MjY1M2FiNmNhNzYzOWRkN2E1NDE0YTk1MWM0YTdmYTNlNGI4IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9Nb2R1bGVzL2dlb2xvY2F0aW9uL0dlb2xvY2F0aW9uLmgKKysr IGIvU291cmNlL1dlYkNvcmUvTW9kdWxlcy9nZW9sb2NhdGlvbi9HZW9sb2NhdGlvbi5oCkBAIC02 Niw2ICs2Niw3IEBAIHB1YmxpYzoKICAgICB2b2lkIGNsZWFyV2F0Y2goaW50IHdhdGNoSUQpOwog CiAgICAgV0VCQ09SRV9FWFBPUlQgdm9pZCBzZXRJc0FsbG93ZWQoYm9vbCk7CisgICAgdm9pZCBy ZXNldElzQWxsb3dlZCgpIHsgbV9hbGxvd0dlb2xvY2F0aW9uID0gVW5rbm93bjsgfQogICAgIGJv b2wgaXNBbGxvd2VkKCkgY29uc3QgeyByZXR1cm4gbV9hbGxvd0dlb2xvY2F0aW9uID09IFllczsg fQogCiAgICAgdm9pZCBwb3NpdGlvbkNoYW5nZWQoKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQvbWFjL1dlYkNvcmVTdXBwb3J0L1dlYkdlb2xvY2F0aW9uQ2xpZW50Lm1tIGIvU291cmNlL1dl YktpdC9tYWMvV2ViQ29yZVN1cHBvcnQvV2ViR2VvbG9jYXRpb25DbGllbnQubW0KaW5kZXggMmYy YzA1NmE0NWM0YWNlN2YzYmQzMWI5NzA0MTI5NzE2ODdmYzVlNy4uYTYyMzlmZmI4Y2VmNDEyODlk ZjRlYTZkOTMzMTJjZmEzYTA0YWY4NSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvV2Vi Q29yZVN1cHBvcnQvV2ViR2VvbG9jYXRpb25DbGllbnQubW0KKysrIGIvU291cmNlL1dlYktpdC9t YWMvV2ViQ29yZVN1cHBvcnQvV2ViR2VvbG9jYXRpb25DbGllbnQubW0KQEAgLTE5MSw3ICsxOTEs NyBAQCAtICh2b2lkKWRlbnlPbmx5VGhpc1JlcXVlc3QKICAgICBXZWJUaHJlYWRSdW4oXnsKICAg ICAgICAgLy8gQSBzb2Z0IGRlbnkgZG9lcyBub3QgcHJldmVudCBzdWJzZXF1ZW50IHJlcXVlc3Qg ZnJvbSB0aGUgR2VvbG9jYXRpb24gb2JqZWN0LgogICAgICAgICBbc2VsZiBkZW55XTsKLSAgICAg ICAgX2dlb2xvY2F0aW9uLT5yZXNldEFsbEdlb2xvY2F0aW9uUGVybWlzc2lvbigpOworICAgICAg ICBfZ2VvbG9jYXRpb24tPnJlc2V0SXNBbGxvd2VkKCk7CiAgICAgfSk7CiB9CiAK