161502 2016-09-01 12:19:45 -0700 Web Inspector: If inspector process crashes, re-inspecting the page does not work 2016-09-02 14:01:56 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck bburg joepeck mattbaker nvasilyev timothy webkit-bug-importer oldest_to_newest 1225455 0 joepeck 2016-09-01 12:19:45 -0700 Summary: If inspector process crashes, re-inspecting the page does not work immediately. Steps to Reproduce: 1. Inspect this page 2. shell> kill -9 <pid-of-InspectorProcess> 3. Inspect this page => Nothing 4. Inspect this page again => Inspector shows but is broken Notes: - WebInspectorProxy has code to handle webProcessDidCrash. It must not be cleaning up everything. 1225456 1 webkit-bug-importer 2016-09-01 12:20:01 -0700 <rdar://problem/28120368> 1225674 2 joepeck 2016-09-01 20:15:13 -0700 This happens only on Mac platforms, because WebInspectorProxyMac's platformDidClose doesn't immediately close the WebPage that crashed. It just sets a timer to close it in 60 seconds. We need a stronger "close because it crashed" case. 1225676 3 287720 joepeck 2016-09-01 20:21:38 -0700 Created attachment 287720 [PATCH] Proposed Fix 1225800 4 287720 bburg 2016-09-02 10:02:18 -0700 Comment on attachment 287720 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=287720&action=review r=me but please re-run through EWS when our lab is not on fire > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:171 > + if (m_inspectorProxyObjCAdapter) { It would be kind of worrying if this were nil. Is this possible if the process crashes prior to creating the adapter (like right after we spawn it i guess) ? 1225825 5 joepeck 2016-09-02 10:57:22 -0700 Well this won't apply until bug 161515. I'll just wait for that. 1225924 6 287720 joepeck 2016-09-02 13:40:50 -0700 Comment on attachment 287720 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=287720&action=review >> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:171 >> + if (m_inspectorProxyObjCAdapter) { > > It would be kind of worrying if this were nil. Is this possible if the process crashes prior to creating the adapter (like right after we spawn it i guess) ? I don't think it is nil, but its much easier to reason about this method when it just does the right thing regardless of the state of members. 1225926 7 joepeck 2016-09-02 13:43:25 -0700 (In reply to comment #6) > Comment on attachment 287720 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=287720&action=review > > >> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:171 > >> + if (m_inspectorProxyObjCAdapter) { > > > > It would be kind of worrying if this were nil. Is this possible if the process crashes prior to creating the adapter (like right after we spawn it i guess) ? > > I don't think it is nil, but its much easier to reason about this method > when it just does the right thing regardless of the state of members. I mean to say, I made these changes not because they were required, but because it just made more sense to me. Just cleanup 1225942 8 joepeck 2016-09-02 14:01:56 -0700 <https://trac.webkit.org/changeset/205370> 287720 2016-09-01 20:21:38 -0700 2016-09-02 10:02:18 -0700 [PATCH] Proposed Fix crash-fix-1.patch text/plain 6501 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCA1M2FjOTU1Li4zNThmNmMwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDUgKzEsNDIg QEAKIDIwMTYtMDktMDEgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KIAor ICAgICAgICBXZWIgSW5zcGVjdG9yOiBJZiBpbnNwZWN0b3IgcHJvY2VzcyBjcmFzaGVzLCByZS1p bnNwZWN0aW5nIHRoZSBwYWdlIGRvZXMgbm90IHdvcmsKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2MTUwMgorICAgICAgICA8cmRhcjovL3Byb2JsZW0v MjgxMjAzNjg+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAg ICAgVGhpcyBpcyBhbiBpc3N1ZSBvbiBNYWMgYmVjYXVzZSBXZWJJbnNwZWN0b3JQcm94eU1hYydz IHBsYXRmb3JtRGlkQ2xvc2UKKyAgICAgICAgZG9lc24ndCBpbW1lZGlhdGVseSBnZXQgcmlkIG9m IHRoZSBXS1dlYlZpZXcgZm9yIHRoZSBpbnNwZWN0b3IuIFRoaXMgaXMKKyAgICAgICAgaW50ZW5k ZWQgc28gdGhhdCBhIHF1aWNrIGNsb3NlIGFuZCByZW9wZW4gb2YgV2ViIEluc3BlY3RvciBpcyBm YXN0ZXIsCisgICAgICAgIGhvd2V2ZXIgaW4gdGhlIGNhc2Ugd2hlcmUgdGhlIFdlYlBhZ2UgdW5k ZXIgdGhlIFdLV2ViVmlldyBjcmFzaGVkLCB3ZQorICAgICAgICBhY3R1YWxseSBzaG91bGQgY2xl YXIgdGhpbmdzIGluc3RlYWQgb2YgdXNpbmcgdGhlIHRpbWVyIHBhdGguIFByb3ZpZGUKKyAgICAg ICAgYSBzdHJvbmdlciBwbGF0Zm9ybSBjbG9zZSBoYW5kbGVyIHdoZW4gdGhlIGluc3BlY3RvciBw YWdlIGNyYXNoZWQuCisKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViSW5zcGVjdG9yUHJveHkuY3Bw OgorICAgICAgICAoV2ViS2l0OjpXZWJJbnNwZWN0b3JQcm94eTo6Y2xvc2VGb3JDcmFzaCk6Cisg ICAgICAgIChXZWJLaXQ6OndlYlByb2Nlc3NEaWRDcmFzaCk6CisgICAgICAgIChXZWJLaXQ6Oldl Ykluc3BlY3RvclByb3h5OjpwbGF0Zm9ybURpZENsb3NlRm9yQ3Jhc2gpOgorICAgICAgICBHbyB0 aHJvdWdoIGEgc3Ryb25nZXIgcGF0aCB3aGVuIHRoZSB3ZWIgcHJvY2VzcyBjcmFzaGVzLgorICAg ICAgICBTb21lIHBsYXRmb3JtcyBtYXkgd2FudCB0byBoYW5kbGUgdGhpcyBkaWZmZXJlbnRseSB0 aGVuIHRoZSB1c2VyCisgICAgICAgIGNsb3NpbmcgYSB3ZWIgaW5zcGVjdG9yIHdpbmRvdy4KKyAg ICAgICAgCisgICAgICAgICogVUlQcm9jZXNzL1dlYkluc3BlY3RvclByb3h5Lmg6CisgICAgICAg ICogVUlQcm9jZXNzL2VmbC9XZWJJbnNwZWN0b3JQcm94eUVmbC5jcHA6CisgICAgICAgIChXZWJL aXQ6OldlYkluc3BlY3RvclByb3h5OjpwbGF0Zm9ybURpZENsb3NlRm9yQ3Jhc2gpOgorICAgICAg ICAqIFVJUHJvY2Vzcy9ndGsvV2ViSW5zcGVjdG9yUHJveHlHdGsuY3BwOgorICAgICAgICAoV2Vi S2l0OjpXZWJJbnNwZWN0b3JQcm94eTo6cGxhdGZvcm1EaWRDbG9zZUZvckNyYXNoKToKKyAgICAg ICAgVGhlc2UgcGxhdGZvcm1zIGRvIG5vdCBuZWVkIHRvIGRvIGFueXRoaW5nIHNwZWNpYWwuCisK KyAgICAgICAgKiBVSVByb2Nlc3MvbWFjL1dlYkluc3BlY3RvclByb3h5TWFjLm1tOgorICAgICAg ICAoV2ViS2l0OjpXZWJJbnNwZWN0b3JQcm94eTo6Y2xvc2VUaW1lckZpcmVkKToKKyAgICAgICAg TG9vc2VuIHRoaXMgY29kZS4gVGhlcmUgaXMgbm8gbmVlZCBmb3IgaXQgdG8gYmUgc28gc3RyaWN0 LgorCisgICAgICAgIChXZWJLaXQ6OldlYkluc3BlY3RvclByb3h5OjpwbGF0Zm9ybURpZENsb3Nl Rm9yQ3Jhc2gpOgorICAgICAgICBJZiB0aGUgaW5zcGVjdG9yIHBhZ2UgY3Jhc2hlZCwgY2xvc2Ug b3VyIGhhbmRsZXMgaW1tZWRpYXRlbHkuCisKKzIwMTYtMDktMDEgIEpvc2VwaCBQZWNvcmFybyAg PHBlY29yYXJvQGFwcGxlLmNvbT4KKwogICAgICAgICBXZWIgSW5zcGVjdG9yOiBQcm92aWRlIGEg d2F5IHRvIG9wZW4gYW4gaW5zcGVjdG9yIGZyb250ZW5kIGZvciBhIHJlbW90ZSB0YXJnZXQKICAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE2MTUxNQogICAg ICAgICA8cmRhcjovL3Byb2JsZW0vMTMxODIxMjc+CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0 Mi9VSVByb2Nlc3MvV2ViSW5zcGVjdG9yUHJveHkuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9j ZXNzL1dlYkluc3BlY3RvclByb3h5LmNwcAppbmRleCAzNjdiZWIwLi5kYzkwMGE1IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViSW5zcGVjdG9yUHJveHkuY3BwCisrKyBi L1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJJbnNwZWN0b3JQcm94eS5jcHAKQEAgLTE2MCw2 ICsxNjAsMTMgQEAgdm9pZCBXZWJJbnNwZWN0b3JQcm94eTo6Y2xvc2UoKQogICAgIGRpZENsb3Nl KCk7CiB9CiAKK3ZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OmNsb3NlRm9yQ3Jhc2goKQoreworICAg IGNsb3NlKCk7CisKKyAgICBwbGF0Zm9ybURpZENsb3NlRm9yQ3Jhc2goKTsKK30KKwogdm9pZCBX ZWJJbnNwZWN0b3JQcm94eTo6c2hvd0NvbnNvbGUoKQogewogICAgIGlmICghbV9pbnNwZWN0ZWRQ YWdlKQpAQCAtMzE5LDcgKzMyNiw3IEBAIHN0YXRpYyB2b2lkIHdlYlByb2Nlc3NEaWRDcmFzaChX S1BhZ2VSZWYsIGNvbnN0IHZvaWQqIGNsaWVudEluZm8pCiB7CiAgICAgV2ViSW5zcGVjdG9yUHJv eHkqIHdlYkluc3BlY3RvclByb3h5ID0gc3RhdGljX2Nhc3Q8V2ViSW5zcGVjdG9yUHJveHkqPihj b25zdF9jYXN0PHZvaWQqPihjbGllbnRJbmZvKSk7CiAgICAgQVNTRVJUKHdlYkluc3BlY3RvclBy b3h5KTsKLSAgICB3ZWJJbnNwZWN0b3JQcm94eS0+Y2xvc2UoKTsKKyAgICB3ZWJJbnNwZWN0b3JQ cm94eS0+Y2xvc2VGb3JDcmFzaCgpOwogfQogCiBzdGF0aWMgdm9pZCBkZWNpZGVQb2xpY3lGb3JO YXZpZ2F0aW9uQWN0aW9uKFdLUGFnZVJlZiBwYWdlUmVmLCBXS05hdmlnYXRpb25BY3Rpb25SZWYg bmF2aWdhdGlvbkFjdGlvblJlZiwgV0tGcmFtZVBvbGljeUxpc3RlbmVyUmVmIGxpc3RlbmVyUmVm LCBXS1R5cGVSZWYsIGNvbnN0IHZvaWQqIGNsaWVudEluZm8pCkBAIC02MzgsNiArNjQ1LDExIEBA IHZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtRGlkQ2xvc2UoKQogICAgIG5vdEltcGxl bWVudGVkKCk7CiB9CiAKK3ZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtRGlkQ2xvc2VG b3JDcmFzaCgpCit7CisgICAgbm90SW1wbGVtZW50ZWQoKTsKK30KKwogdm9pZCBXZWJJbnNwZWN0 b3JQcm94eTo6cGxhdGZvcm1JbnZhbGlkYXRlKCkKIHsKICAgICBub3RJbXBsZW1lbnRlZCgpOwpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYkluc3BlY3RvclByb3h5Lmgg Yi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViSW5zcGVjdG9yUHJveHkuaAppbmRleCA3MTIy NTBmLi5kNjY4ODRkIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViSW5z cGVjdG9yUHJveHkuaAorKysgYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViSW5zcGVjdG9y UHJveHkuaApAQCAtOTcsNiArOTcsNyBAQCBwdWJsaWM6CiAgICAgdm9pZCBzaG93KCk7CiAgICAg dm9pZCBoaWRlKCk7CiAgICAgdm9pZCBjbG9zZSgpOworICAgIHZvaWQgY2xvc2VGb3JDcmFzaCgp OwogCiAjaWYgUExBVEZPUk0oTUFDKSAmJiBXS19BUElfRU5BQkxFRAogICAgIHN0YXRpYyBSZXRh aW5QdHI8V0tXZWJWaWV3Q29uZmlndXJhdGlvbj4gY3JlYXRlRnJvbnRlbmRDb25maWd1cmF0aW9u KFdlYlBhZ2VQcm94eSosIGJvb2wgdW5kZXJUZXN0KTsKQEAgLTE3Niw2ICsxNzcsNyBAQCBwcml2 YXRlOgogICAgIFdlYlBhZ2VQcm94eSogcGxhdGZvcm1DcmVhdGVJbnNwZWN0b3JQYWdlKCk7CiAg ICAgdm9pZCBwbGF0Zm9ybU9wZW4oKTsKICAgICB2b2lkIHBsYXRmb3JtRGlkQ2xvc2UoKTsKKyAg ICB2b2lkIHBsYXRmb3JtRGlkQ2xvc2VGb3JDcmFzaCgpOwogICAgIHZvaWQgcGxhdGZvcm1JbnZh bGlkYXRlKCk7CiAgICAgdm9pZCBwbGF0Zm9ybUJyaW5nVG9Gcm9udCgpOwogICAgIHZvaWQgcGxh dGZvcm1IaWRlKCk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZWZsL1dl Ykluc3BlY3RvclByb3h5RWZsLmNwcCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvV2Vi SW5zcGVjdG9yUHJveHlFZmwuY3BwCmluZGV4IGU0NzliZjkuLmY1NzRlMTUgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvV2ViSW5zcGVjdG9yUHJveHlFZmwuY3BwCisr KyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9lZmwvV2ViSW5zcGVjdG9yUHJveHlFZmwuY3Bw CkBAIC0xNDYsNiArMTQ2LDEwIEBAIHZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtRGlk Q2xvc2UoKQogICAgIH0KIH0KIAordm9pZCBXZWJJbnNwZWN0b3JQcm94eTo6cGxhdGZvcm1EaWRD bG9zZUZvckNyYXNoKCkKK3sKK30KKwogdm9pZCBXZWJJbnNwZWN0b3JQcm94eTo6cGxhdGZvcm1J bnZhbGlkYXRlKCkKIHsKIH0KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9n dGsvV2ViSW5zcGVjdG9yUHJveHlHdGsuY3BwIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2d0 ay9XZWJJbnNwZWN0b3JQcm94eUd0ay5jcHAKaW5kZXggNGQzODQ5NC4uYTYyMzA1MiAxMDA2NDQK LS0tIGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2d0ay9XZWJJbnNwZWN0b3JQcm94eUd0ay5j cHAKKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2d0ay9XZWJJbnNwZWN0b3JQcm94eUd0 ay5jcHAKQEAgLTIxNCw2ICsyMTQsMTAgQEAgdm9pZCBXZWJJbnNwZWN0b3JQcm94eTo6cGxhdGZv cm1EaWRDbG9zZSgpCiAgICAgbV9pbnNwZWN0b3JWaWV3ID0gMDsKIH0KIAordm9pZCBXZWJJbnNw ZWN0b3JQcm94eTo6cGxhdGZvcm1EaWRDbG9zZUZvckNyYXNoKCkKK3sKK30KKwogdm9pZCBXZWJJ bnNwZWN0b3JQcm94eTo6cGxhdGZvcm1JbnZhbGlkYXRlKCkKIHsKIH0KZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvV2ViSW5zcGVjdG9yUHJveHlNYWMubW0gYi9Tb3Vy Y2UvV2ViS2l0Mi9VSVByb2Nlc3MvbWFjL1dlYkluc3BlY3RvclByb3h5TWFjLm1tCmluZGV4IDc3 NGViOGYuLmNiOGI3ZTMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMv V2ViSW5zcGVjdG9yUHJveHlNYWMubW0KKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21h Yy9XZWJJbnNwZWN0b3JQcm94eU1hYy5tbQpAQCAtMTYyLDE4ICsxNjIsMTggQEAgV0tSZWN0IFdl Ykluc3BlY3RvclByb3h5OjppbnNwZWN0b3JXaW5kb3dGcmFtZSgpCiB2b2lkIFdlYkluc3BlY3Rv clByb3h5OjpjbG9zZVRpbWVyRmlyZWQoKQogewogICAgIEFTU0VSVCghbV9pc0F0dGFjaGVkIHx8 ICFtX2luc3BlY3RvcldpbmRvdyk7Ci0gICAgaWYgKG1faXNBdHRhY2hlZCB8fCBtX2luc3BlY3Rv cldpbmRvdykKLSAgICAgICAgcmV0dXJuOwogCiAgICAgaWYgKG1faW5zcGVjdG9yVmlldykgewog ICAgICAgICBtX2luc3BlY3RvclZpZXctPl9wYWdlLT5jbG9zZSgpOwogICAgICAgICBtX2luc3Bl Y3RvclZpZXcgPSBuaWw7CiAgICAgfQogCi0gICAgW1tOU05vdGlmaWNhdGlvbkNlbnRlciBkZWZh dWx0Q2VudGVyXSByZW1vdmVPYnNlcnZlcjptX2luc3BlY3RvclByb3h5T2JqQ0FkYXB0ZXIuZ2V0 KCldOworICAgIGlmIChtX2luc3BlY3RvclByb3h5T2JqQ0FkYXB0ZXIpIHsKKyAgICAgICAgW1tO U05vdGlmaWNhdGlvbkNlbnRlciBkZWZhdWx0Q2VudGVyXSByZW1vdmVPYnNlcnZlcjptX2luc3Bl Y3RvclByb3h5T2JqQ0FkYXB0ZXIuZ2V0KCldOwogCi0gICAgW21faW5zcGVjdG9yUHJveHlPYmpD QWRhcHRlciBjbG9zZV07Ci0gICAgbV9pbnNwZWN0b3JQcm94eU9iakNBZGFwdGVyID0gbmlsOwor ICAgICAgICBbbV9pbnNwZWN0b3JQcm94eU9iakNBZGFwdGVyIGNsb3NlXTsKKyAgICAgICAgbV9p bnNwZWN0b3JQcm94eU9iakNBZGFwdGVyID0gbmlsOworICAgIH0KIH0KIAogdm9pZCBXZWJJbnNw ZWN0b3JQcm94eTo6Y3JlYXRlSW5zcGVjdG9yV2luZG93KCkKQEAgLTM1MSw2ICszNTEsMTMgQEAg dm9pZCBXZWJJbnNwZWN0b3JQcm94eTo6cGxhdGZvcm1EaWRDbG9zZSgpCiAgICAgbV9jbG9zZVRp bWVyLnN0YXJ0T25lU2hvdCh3ZWJWaWV3Q2xvc2VUaW1lb3V0KTsKIH0KIAordm9pZCBXZWJJbnNw ZWN0b3JQcm94eTo6cGxhdGZvcm1EaWRDbG9zZUZvckNyYXNoKCkKK3sKKyAgICBtX2Nsb3NlVGlt ZXIuc3RvcCgpOworCisgICAgY2xvc2VUaW1lckZpcmVkKCk7Cit9CisKIHZvaWQgV2ViSW5zcGVj dG9yUHJveHk6OnBsYXRmb3JtSW52YWxpZGF0ZSgpCiB7CiAgICAgbV9jbG9zZVRpbWVyLnN0b3Ao KTsK