130898 2014-03-28 07:00:56 -0700 [GTK] Geoclue2 providers won't work after reloading 2014-03-28 08:34:32 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) All Linux RESOLVED FIXED Gtk P2 Normal --- 1 mario webkit-unassigned cgarcia commit-queue oldest_to_newest 995314 0 mario 2014-03-28 07:00:56 -0700 There is a bug in the code of the Geoclue2 geolocation provider that prevents geolocation from working if you try to reload a page after having started using the geolocation API previously. Steps to reproduce: 1. Open http://html5demos.com/geo in a geolocation in Minibrowser 2. Allow to know the location when requested through a dialog 3. Wait until it says "Found you!" 4. Reload the page and repeat step 2 Expected outcome: You should see the found location once again Actual: You will see "Failed" instead, because the geolocation provider is trying to use the same client proxy for geoclue2 service, that is no longer available after having called geoclue_client_call_stop() in GeolocarionProviderGeoclue::stopUpdating(), since apparently it can be reused from previous times. 995317 1 228049 mario 2014-03-28 07:14:53 -0700 Created attachment 228049 Patch proposal The fix is quite simple. Please consider reviewing it quickly so we can request integrating it into the stable branch before the next release. Thanks! 995320 2 228049 mrobinson 2014-03-28 07:34:03 -0700 Comment on attachment 228049 Patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=228049&action=review > Source/WebCore/platform/geoclue/GeolocationProviderGeoclue2.cpp:76 > - startGeoclueClient(); > + geoclue_manager_call_get_client(m_managerProxy.get(), nullptr, reinterpret_cast<GAsyncReadyCallback>(getGeoclueClientCallback), this); You don't mention in the changelog why you no longer call startGeoclueClient. 995321 3 mario 2014-03-28 07:46:49 -0700 (In reply to comment #2) > (From update of attachment 228049 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=228049&action=review > > > Source/WebCore/platform/geoclue/GeolocationProviderGeoclue2.cpp:76 > > - startGeoclueClient(); > > + geoclue_manager_call_get_client(m_managerProxy.get(), nullptr, reinterpret_cast<GAsyncReadyCallback>(getGeoclueClientCallback), this); > > You don't mention in the changelog why you no longer call startGeoclueClient. I no longer call him here because it will be called later on, once the client proxy is created, down in createGeoclueClientProxyCallback(). Not sure whether it's worth mentioning that in the ChangeLog? 995324 4 228049 mario 2014-03-28 08:05:18 -0700 Comment on attachment 228049 Patch proposal Thanks for the review! 995331 5 228049 commit-queue 2014-03-28 08:34:29 -0700 Comment on attachment 228049 Patch proposal Clearing flags on attachment: 228049 Committed r166406: <http://trac.webkit.org/changeset/166406> 995332 6 commit-queue 2014-03-28 08:34:32 -0700 All reviewed patches have been landed. Closing bug. 228049 2014-03-28 07:14:53 -0700 2014-03-28 08:34:29 -0700 Patch proposal 0001-2014-03-28-Mario-Sanchez-Prada-mario.prada-samsung.c.patch text/plain 3267 mario RnJvbSA0MjE1NzdmOTVhYjBiNzE2Y2Y0YjRmNjE2NDA4YTA4YzNjMDRhZmJjIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBNYXJpbyBTYW5jaGV6IFByYWRhIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KRGF0ZTogRnJpLCAyOCBNYXIgMjAxNCAxMjowOTowNSArMDAwMApTdWJqZWN0OiBb UEFUQ0hdIDIwMTQtMDMtMjggIE1hcmlvIFNhbmNoZXogUHJhZGEgIDxtYXJpby5wcmFkYUBzYW1z dW5nLmNvbT4KCiAgICAgICAgW0dUS10gR2VvY2x1ZTIgcHJvdmlkZXJzIHdvbid0IHdvcmsgYWZ0 ZXIgcmVsb2FkaW5nCiAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dp P2lkPTEzMDg5OAoKICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KCiAgICAgICAg RG9uJ3QgcmV1c2UgdGhlIEdlb2NsdWUyIGNsaWVudCBwcm94eSBiZXR3ZWVuIGRpZmZlcmVudCBj YWxscyB0bwogICAgICAgIHN0YXJ0UG9zaXRpb24oKSwgYW5kIGNyZWF0ZSBhIG5ldyBjbGllbnQg cHJveHkgZWFjaCB0aW1lIGluc3RlYWQuCgogICAgICAgICogcGxhdGZvcm0vZ2VvY2x1ZS9HZW9s b2NhdGlvblByb3ZpZGVyR2VvY2x1ZTIuY3BwOgogICAgICAgIChHZW9sb2NhdGlvblByb3ZpZGVy R2VvY2x1ZTo6c3RhcnRVcGRhdGluZyk6IERvbid0IHJldXNlIHRoZQogICAgICAgIGNsaWVudCBw cm94eSwgYnkgYWx3YXlzIGNhbGxpbmcgZ2VvY2x1ZV9tYW5hZ2VyX2NhbGxfZ2V0X2NsaWVudCgp LgogICAgICAgIChHZW9sb2NhdGlvblByb3ZpZGVyR2VvY2x1ZTo6c3RvcFVwZGF0aW5nKTogRGlz Y29ubmVjdCBmcm9tIHRoZQogICAgICAgICdsb2NhdGlvbi11cGRhdGVkJyBzaWduYWwgYW5kIGRp c3Bvc2UgdGhlIGNsaWVudCBwcm94eS4KLS0tCiBTb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cgICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICB8IDE2ICsrKysrKysrKysrKysrKysKIC4uLi9w bGF0Zm9ybS9nZW9jbHVlL0dlb2xvY2F0aW9uUHJvdmlkZXJHZW9jbHVlMi5jcHAgICAgIHwgMTIg KysrKystLS0tLS0tCiAyIGZpbGVzIGNoYW5nZWQsIDIxIGluc2VydGlvbnMoKyksIDcgZGVsZXRp b25zKC0pCgpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nIGIvU291cmNlL1dl YkNvcmUvQ2hhbmdlTG9nCmluZGV4IGIyNDEyMTcuLjVmZDExNjEgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJDb3JlL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOSBAQAorMjAxNC0wMy0yOCAgTWFyaW8gU2FuY2hleiBQcmFkYSAgPG1hcmlvLnByYWRh QHNhbXN1bmcuY29tPgorCisgICAgICAgIFtHVEtdIEdlb2NsdWUyIHByb3ZpZGVycyB3b24ndCB3 b3JrIGFmdGVyIHJlbG9hZGluZworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MTMwODk4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISku CisKKyAgICAgICAgRG9uJ3QgcmV1c2UgdGhlIEdlb2NsdWUyIGNsaWVudCBwcm94eSBiZXR3ZWVu IGRpZmZlcmVudCBjYWxscyB0bworICAgICAgICBzdGFydFBvc2l0aW9uKCksIGFuZCBjcmVhdGUg YSBuZXcgY2xpZW50IHByb3h5IGVhY2ggdGltZSBpbnN0ZWFkLgorCisgICAgICAgICogcGxhdGZv cm0vZ2VvY2x1ZS9HZW9sb2NhdGlvblByb3ZpZGVyR2VvY2x1ZTIuY3BwOgorICAgICAgICAoR2Vv bG9jYXRpb25Qcm92aWRlckdlb2NsdWU6OnN0YXJ0VXBkYXRpbmcpOiBEb24ndCByZXVzZSB0aGUK KyAgICAgICAgY2xpZW50IHByb3h5LCBieSBhbHdheXMgY2FsbGluZyBnZW9jbHVlX21hbmFnZXJf Y2FsbF9nZXRfY2xpZW50KCkuCisgICAgICAgIChHZW9sb2NhdGlvblByb3ZpZGVyR2VvY2x1ZTo6 c3RvcFVwZGF0aW5nKTogRGlzY29ubmVjdCBmcm9tIHRoZQorICAgICAgICAnbG9jYXRpb24tdXBk YXRlZCcgc2lnbmFsIGFuZCBkaXNwb3NlIHRoZSBjbGllbnQgcHJveHkuCisKIDIwMTQtMDMtMjcg IEtyenlzenRvZiBDemVjaCAgPGsuY3plY2hAc2Ftc3VuZy5jb20+CiAKICAgICAgICAgQVg6IFJl dHVybnMgY29uc3QgcmVmZXJlbmNlIHRvIHN0YXRpYyBzdHJpbmcuCmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS9nZW9jbHVlL0dlb2xvY2F0aW9uUHJvdmlkZXJHZW9jbHVlMi5j cHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9nZW9jbHVlL0dlb2xvY2F0aW9uUHJvdmlkZXJH ZW9jbHVlMi5jcHAKaW5kZXggNzQ4NzE5NS4uY2VhNDhmMyAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vZ2VvY2x1ZS9HZW9sb2NhdGlvblByb3ZpZGVyR2VvY2x1ZTIuY3BwCisr KyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2dlb2NsdWUvR2VvbG9jYXRpb25Qcm92aWRlckdl b2NsdWUyLmNwcApAQCAtNzMsMTggKzczLDE2IEBAIHZvaWQgR2VvbG9jYXRpb25Qcm92aWRlckdl b2NsdWU6OnN0YXJ0VXBkYXRpbmcoKQogICAgICAgICByZXR1cm47CiAgICAgfQogCi0gICAgaWYg KCFtX2NsaWVudFByb3h5KSB7Ci0gICAgICAgIGdlb2NsdWVfbWFuYWdlcl9jYWxsX2dldF9jbGll bnQobV9tYW5hZ2VyUHJveHkuZ2V0KCksIG51bGxwdHIsIHJlaW50ZXJwcmV0X2Nhc3Q8R0FzeW5j UmVhZHlDYWxsYmFjaz4oZ2V0R2VvY2x1ZUNsaWVudENhbGxiYWNrKSwgdGhpcyk7Ci0gICAgICAg IHJldHVybjsKLSAgICB9Ci0KLSAgICBzdGFydEdlb2NsdWVDbGllbnQoKTsKKyAgICBnZW9jbHVl X21hbmFnZXJfY2FsbF9nZXRfY2xpZW50KG1fbWFuYWdlclByb3h5LmdldCgpLCBudWxscHRyLCBy ZWludGVycHJldF9jYXN0PEdBc3luY1JlYWR5Q2FsbGJhY2s+KGdldEdlb2NsdWVDbGllbnRDYWxs YmFjayksIHRoaXMpOwogfQogCiB2b2lkIEdlb2xvY2F0aW9uUHJvdmlkZXJHZW9jbHVlOjpzdG9w VXBkYXRpbmcoKQogewotICAgIGlmIChtX2NsaWVudFByb3h5KQorICAgIGlmIChtX2NsaWVudFBy b3h5KSB7CiAgICAgICAgIGdlb2NsdWVfY2xpZW50X2NhbGxfc3RvcChtX2NsaWVudFByb3h5Lmdl dCgpLCBudWxscHRyLCBudWxscHRyLCBudWxscHRyKTsKKyAgICAgICAgZ19zaWduYWxfaGFuZGxl cnNfZGlzY29ubmVjdF9ieV9mdW5jKG1fY2xpZW50UHJveHkuZ2V0KCksIHJlaW50ZXJwcmV0X2Nh c3Q8Z3BvaW50ZXI+KGxvY2F0aW9uVXBkYXRlZENhbGxiYWNrKSwgdGhpcyk7CisgICAgICAgIG1f Y2xpZW50UHJveHkgPSBudWxscHRyOworICAgIH0KICAgICBtX2lzVXBkYXRpbmcgPSBmYWxzZTsK IH0KIAotLSAKMS44LjUuMwoK