125308 2013-12-05 10:42:02 -0800 AX: WK2: Safari extension installation hangs Safari and VoiceOver 2013-12-05 15:30:21 -0800 1 1 1 Unclassified WebKit Accessibility 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 cfleizach cfleizach andersca ap beidson commit-queue jberlin webkit-bug-importer oldest_to_newest 956957 0 cfleizach 2013-12-05 10:42:02 -0800 1-open safari and search for safari extension (any so far) 2-once downloaded, go to finder and open the ext file. * EXPECTED RESULTS: extension to install with the dialog in safari asking to confirm installation of the extension. * ACTUAL RESULTS: safari and voice over freeze. Difficult to get out 956958 1 cfleizach 2013-12-05 10:42:11 -0800 <rdar://problem/15594097> 956960 2 218524 cfleizach 2013-12-05 10:43:32 -0800 Created attachment 218524 patch 956965 3 ap 2013-12-05 11:02:41 -0800 This is super scary. With this patch, dispatchDecidePolicyForResponse will have entirely different behavior when accessibility is enabled. 956966 4 ap 2013-12-05 11:04:38 -0800 If you can prove that it is safe to spin run loop here, I suggest making it unconditionally. Otherwise, we might need a deeper change to ho extensions are installed. 956974 5 cfleizach 2013-12-05 11:23:24 -0800 (In reply to comment #4) > If you can prove that it is safe to spin run loop here, I suggest making it unconditionally. > > Otherwise, we might need a deeper change to ho extensions are installed. This seems not that much different from javascript alerts where we do unsigned syncSendFlags = (WebCore::AXObjectCache::accessibilityEnabled()) ? CoreIPC::SpinRunLoopWhileWaitingForReply : 0; WebProcess::shared().parentProcessConnection()->sendSync(Messages::WebPageProxy::RunJavaScriptAlert(webFrame->frameID(), alertText), Messages::WebPageProxy::RunJavaScriptAlert::Reply(), m_page->pageID(), CoreIPC::Connection::NoTimeout, syncSendFlags); That code has been in for 2 years, and we do have one case where it has proven to be unsafe (https://bugs.webkit.org/show_bug.cgi?id=123828) which I am very close to having a fix for. 956977 6 ap 2013-12-05 11:28:13 -0800 Should we make it unconditional in that case? Anders and Brady are really the ones who should make the call on whether it's acceptable design to spin the run loop here. 956979 7 cfleizach 2013-12-05 11:30:45 -0800 (In reply to comment #6) > Should we make it unconditional in that case? > Not sure. I think Anders and Darin helped me resolve the javascript case in the past, so (echoing you) they'll probably be able to speak to that more. Thanks > Anders and Brady are really the ones who should make the call on whether it's acceptable design to spin the run loop here. 957060 8 218524 andersca 2013-12-05 15:00:02 -0800 Comment on attachment 218524 patch I think this looks good. 957072 9 218524 commit-queue 2013-12-05 15:30:18 -0800 Comment on attachment 218524 patch Clearing flags on attachment: 218524 Committed r160197: <http://trac.webkit.org/changeset/160197> 957073 10 commit-queue 2013-12-05 15:30:21 -0800 All reviewed patches have been landed. Closing bug. 218524 2013-12-05 10:43:32 -0800 2013-12-05 15:30:18 -0800 patch patch text/plain 4095 cfleizach SW5kZXg6IFNvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi S2l0Mi9DaGFuZ2VMb2cJKHJldmlzaW9uIDE2MDE4MCkKKysrIFNvdXJjZS9XZWJLaXQyL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE4IEBACisyMDEzLTEyLTA1ICBDaHJpcyBG bGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CisKKyAgICAgICAgQVg6IFNlZWQ6IHNhZmFy aSBleHRlbnNpb24gaW5zdGFsbGF0aW9uIGNyYXNoZXMgc2FmYXJpIHVuZGVyIHZvaWNlIG92ZXIg YW5kIGZyZWV6ZXMgdm9pY2Ugb3ZlcgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9z aG93X2J1Zy5jZ2k/aWQ9MTI1MzA4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgTXVjaCBsaWtlIEphdmFzY3JpcHQgYWxlcnRzLCB3ZSBuZWVkIHRvIGFs bG93IGFjY2Vzc2liaWxpdHkgY2xpZW50cyB0byBjb250aW51ZSB0byBpbnRlcmFjdCB3aXRoIHRo ZSBXZWJQcm9jZXNzIHRocmVhZAorICAgICAgICB3aGVuIHVzaW5nIGRpc3BhdGNoRGVjaWRlUG9s aWN5UmVzcG9uc2VzLgorCisgICAgICAgICogUGxhdGZvcm0vQ29yZUlQQy9NZXNzYWdlU2VuZGVy Lmg6CisgICAgICAgIChDb3JlSVBDOjpNZXNzYWdlU2VuZGVyOjpzZW5kU3luYyk6CisgICAgICAg ICogV2ViUHJvY2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJGcmFtZUxvYWRlckNsaWVudC5jcHA6Cisg ICAgICAgIChXZWJLaXQ6OldlYkZyYW1lTG9hZGVyQ2xpZW50OjpkaXNwYXRjaERlY2lkZVBvbGlj eUZvclJlc3BvbnNlKToKKwogMjAxMy0xMi0wNSAgWmFuIERvYmVyc2VrICA8emRvYmVyc2VrQGln YWxpYS5jb20+CiAKICAgICAgICAgW0dUS11bV0syXSBDbGVhbiB1cCBXb3JrUXVldWVHdGsKSW5k ZXg6IFNvdXJjZS9XZWJLaXQyL1BsYXRmb3JtL0NvcmVJUEMvTWVzc2FnZVNlbmRlci5oCj09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQyL1BsYXRmb3JtL0NvcmVJUEMvTWVzc2FnZVNlbmRlci5o CShyZXZpc2lvbiAxNjAwMTEpCisrKyBTb3VyY2UvV2ViS2l0Mi9QbGF0Zm9ybS9Db3JlSVBDL01l c3NhZ2VTZW5kZXIuaAkod29ya2luZyBjb3B5KQpAQCAtNTEsMTkgKzUxLDE5IEBACiAgICAgfQog CiAgICAgdGVtcGxhdGU8dHlwZW5hbWUgVD4KLSAgICBib29sIHNlbmRTeW5jKFQmJiBtZXNzYWdl LCB0eXBlbmFtZSBUOjpSZXBseSYmIHJlcGx5LCBkb3VibGUgdGltZW91dCA9IENvbm5lY3Rpb246 Ok5vVGltZW91dCkKKyAgICBib29sIHNlbmRTeW5jKFQmJiBtZXNzYWdlLCB0eXBlbmFtZSBUOjpS ZXBseSYmIHJlcGx5LCBkb3VibGUgdGltZW91dCA9IENvbm5lY3Rpb246Ok5vVGltZW91dCwgdW5z aWduZWQgc3luY1NlbmRGbGFncyA9IDApCiAgICAgewogICAgICAgICBzdGF0aWNfYXNzZXJ0KFQ6 OmlzU3luYywgIk1lc3NhZ2UgaXMgbm90IHN5bmMhIik7CiAKLSAgICAgICAgcmV0dXJuIHNlbmRT eW5jKHN0ZDo6Zm9yd2FyZDxUPihtZXNzYWdlKSwgc3RkOjptb3ZlKHJlcGx5KSwgbWVzc2FnZVNl bmRlckRlc3RpbmF0aW9uSUQoKSwgdGltZW91dCk7CisgICAgICAgIHJldHVybiBzZW5kU3luYyhz dGQ6OmZvcndhcmQ8VD4obWVzc2FnZSksIHN0ZDo6bW92ZShyZXBseSksIG1lc3NhZ2VTZW5kZXJE ZXN0aW5hdGlvbklEKCksIHRpbWVvdXQsIHN5bmNTZW5kRmxhZ3MpOwogICAgIH0KIAogICAgIHRl bXBsYXRlPHR5cGVuYW1lIFQ+Ci0gICAgYm9vbCBzZW5kU3luYyhUJiYgbWVzc2FnZSwgdHlwZW5h bWUgVDo6UmVwbHkmJiByZXBseSwgdWludDY0X3QgZGVzdGluYXRpb25JRCwgZG91YmxlIHRpbWVv dXQgPSBDb25uZWN0aW9uOjpOb1RpbWVvdXQpCisgICAgYm9vbCBzZW5kU3luYyhUJiYgbWVzc2Fn ZSwgdHlwZW5hbWUgVDo6UmVwbHkmJiByZXBseSwgdWludDY0X3QgZGVzdGluYXRpb25JRCwgZG91 YmxlIHRpbWVvdXQgPSBDb25uZWN0aW9uOjpOb1RpbWVvdXQsIHVuc2lnbmVkIHN5bmNTZW5kRmxh Z3MgPSAwKQogICAgIHsKICAgICAgICAgQVNTRVJUKG1lc3NhZ2VTZW5kZXJDb25uZWN0aW9uKCkp OwogCi0gICAgICAgIHJldHVybiBtZXNzYWdlU2VuZGVyQ29ubmVjdGlvbigpLT5zZW5kU3luYyhz dGQ6Om1vdmUobWVzc2FnZSksIHN0ZDo6bW92ZShyZXBseSksIGRlc3RpbmF0aW9uSUQsIHRpbWVv dXQpOworICAgICAgICByZXR1cm4gbWVzc2FnZVNlbmRlckNvbm5lY3Rpb24oKS0+c2VuZFN5bmMo c3RkOjptb3ZlKG1lc3NhZ2UpLCBzdGQ6Om1vdmUocmVwbHkpLCBkZXN0aW5hdGlvbklELCB0aW1l b3V0LCBzeW5jU2VuZEZsYWdzKTsKICAgICB9CiAKICAgICBib29sIHNlbmRNZXNzYWdlKHN0ZDo6 dW5pcXVlX3B0cjxNZXNzYWdlRW5jb2Rlcj4pOwpJbmRleDogU291cmNlL1dlYktpdDIvV2ViUHJv Y2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJGcmFtZUxvYWRlckNsaWVudC5jcHAKPT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQot LS0gU291cmNlL1dlYktpdDIvV2ViUHJvY2Vzcy9XZWJDb3JlU3VwcG9ydC9XZWJGcmFtZUxvYWRl ckNsaWVudC5jcHAJKHJldmlzaW9uIDE2MDAxMSkKKysrIFNvdXJjZS9XZWJLaXQyL1dlYlByb2Nl c3MvV2ViQ29yZVN1cHBvcnQvV2ViRnJhbWVMb2FkZXJDbGllbnQuY3BwCSh3b3JraW5nIGNvcHkp CkBAIC01MSw2ICs1MSw3IEBACiAjaW5jbHVkZSAiV2ViUHJvY2Vzc1Byb3h5TWVzc2FnZXMuaCIK ICNpbmNsdWRlIDxKYXZhU2NyaXB0Q29yZS9BUElDYXN0Lmg+CiAjaW5jbHVkZSA8SmF2YVNjcmlw dENvcmUvSlNPYmplY3QuaD4KKyNpbmNsdWRlIDxXZWJDb3JlL0FYT2JqZWN0Q2FjaGUuaD4KICNp bmNsdWRlIDxXZWJDb3JlL0Nocm9tZS5oPgogI2luY2x1ZGUgPFdlYkNvcmUvRE9NV3JhcHBlcldv cmxkLmg+CiAjaW5jbHVkZSA8V2ViQ29yZS9Eb2N1bWVudExvYWRlci5oPgpAQCAtNjQ2LDcgKzY0 Nyw4IEBACiAgICAgdWludDY0X3QgZG93bmxvYWRJRDsKIAogICAgIC8vIE5vdGlmeSB0aGUgVUlQ cm9jZXNzLgotICAgIGlmICghd2ViUGFnZS0+c2VuZFN5bmMoTWVzc2FnZXM6OldlYlBhZ2VQcm94 eTo6RGVjaWRlUG9saWN5Rm9yUmVzcG9uc2VTeW5jKG1fZnJhbWUtPmZyYW1lSUQoKSwgcmVzcG9u c2UsIHJlcXVlc3QsIGNhblNob3dNSU1FVHlwZSwgbGlzdGVuZXJJRCwgSW5qZWN0ZWRCdW5kbGVV c2VyTWVzc2FnZUVuY29kZXIodXNlckRhdGEuZ2V0KCkpKSwgTWVzc2FnZXM6OldlYlBhZ2VQcm94 eTo6RGVjaWRlUG9saWN5Rm9yUmVzcG9uc2VTeW5jOjpSZXBseShyZWNlaXZlZFBvbGljeUFjdGlv biwgcG9saWN5QWN0aW9uLCBkb3dubG9hZElEKSkpCisgICAgdW5zaWduZWQgc3luY1NlbmRGbGFn cyA9IChXZWJDb3JlOjpBWE9iamVjdENhY2hlOjphY2Nlc3NpYmlsaXR5RW5hYmxlZCgpKSA/IENv cmVJUEM6OlNwaW5SdW5Mb29wV2hpbGVXYWl0aW5nRm9yUmVwbHkgOiAwOworICAgIGlmICghd2Vi UGFnZS0+c2VuZFN5bmMoTWVzc2FnZXM6OldlYlBhZ2VQcm94eTo6RGVjaWRlUG9saWN5Rm9yUmVz cG9uc2VTeW5jKG1fZnJhbWUtPmZyYW1lSUQoKSwgcmVzcG9uc2UsIHJlcXVlc3QsIGNhblNob3dN SU1FVHlwZSwgbGlzdGVuZXJJRCwgSW5qZWN0ZWRCdW5kbGVVc2VyTWVzc2FnZUVuY29kZXIodXNl ckRhdGEuZ2V0KCkpKSwgTWVzc2FnZXM6OldlYlBhZ2VQcm94eTo6RGVjaWRlUG9saWN5Rm9yUmVz cG9uc2VTeW5jOjpSZXBseShyZWNlaXZlZFBvbGljeUFjdGlvbiwgcG9saWN5QWN0aW9uLCBkb3du bG9hZElEKSwgQ29yZUlQQzo6Q29ubmVjdGlvbjo6Tm9UaW1lb3V0LCBzeW5jU2VuZEZsYWdzKSkK ICAgICAgICAgcmV0dXJuOwogCiAgICAgLy8gV2UgY2FsbCB0aGlzIHN5bmNocm9ub3VzbHkgYmVj YXVzZSBDRk5ldHdvcmsgY2FuIG9ubHkgY29udmVydCBhIGxvYWRpbmcgY29ubmVjdGlvbiB0byBh IGRvd25sb2FkIGZyb20gaXRzIGRpZFJlY2VpdmVSZXNwb25zZSBjYWxsYmFjay4K