139740 2014-12-17 12:12:25 -0800 Web Inspector: [Mac] Occasional Crashes Closing Inspector 2014-12-17 13:32:10 -0800 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck webkit-unassigned andersca ap commit-queue enrica graouts joepeck jonowells mattbaker nvasilyev timothy webkit-bug-importer oldest_to_newest 1056494 0 joepeck 2014-12-17 12:12:25 -0800 * SUMMARY I've been able to hit some crashes when closing Inspector Windows and using Guard Malloc. * STEPS TO REPRODUCE 1. shell> DYLD_INSERT_LIBRARIES=/usr/lib/libgmalloc.dylib run-safari 2. Open http://bogojoker.com 3. Rapidly open/close inspector windows (e.g. hold down Cmd+Shift+I and Cmd+W occasionally when inspector window is focused) => Crash * NOTES Two frequent crash points: (1) In WebInspectorProxy::inspectorWindowFrame using destroyed WebInspectorProxy as the client of the WKView's Page. This was because we were clearing our reference to the WKWebInspectorWKView, but it was actually kept alive by a dispatch in WKView, the WebInspectorProxy was destroyed, but still had a weak reference in the UIClient. - (void)_updateWindowAndViewFrames { ... dispatch_async(dispatch_get_main_queue(), ^{ ... _data->_page->windowAndViewFramesChanged(viewFrameInWindowCoordinates, accessibilityPosition); }); } (2) In WebPageProxy::didReceiveEvent. While handling a keypress, the WebPageProxy is destroyed, and we continue and use bad objects. 1056495 1 webkit-bug-importer 2014-12-17 12:12:50 -0800 <rdar://problem/19282330> 1056502 2 243451 joepeck 2014-12-17 12:28:29 -0800 Created attachment 243451 [PATCH] Proposed Fix 1056508 3 243451 timothy 2014-12-17 12:44:39 -0800 Comment on attachment 243451 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=243451&action=review > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:296 > + WebPageProxy* inspectorPage = toImpl(m_inspectorView.get().pageRef); > + inspectorPage->close(); WKView dealloc calls close(), but that assumes we hold the last ref here. 1056523 4 243451 commit-queue 2014-12-17 13:18:35 -0800 Comment on attachment 243451 [PATCH] Proposed Fix Clearing flags on attachment: 243451 Committed r177459: <http://trac.webkit.org/changeset/177459> 1056524 5 commit-queue 2014-12-17 13:18:39 -0800 All reviewed patches have been landed. Closing bug. 1056532 6 joepeck 2014-12-17 13:32:10 -0800 (In reply to comment #3) > Comment on attachment 243451 [details] > [PATCH] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=243451&action=review > > > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:296 > > + WebPageProxy* inspectorPage = toImpl(m_inspectorView.get().pageRef); > > + inspectorPage->close(); > > WKView dealloc calls close(), but that assumes we hold the last ref here. Correct, we were not holding the lat ref, because the WKWebInspectorWKView was being retained by a dispatch_async block. =( 243451 2014-12-17 12:28:29 -0800 2014-12-17 13:18:35 -0800 [PATCH] Proposed Fix fix-crashes-1.patch text/plain 5277 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQyL0No YW5nZUxvZwppbmRleCAwODliYTQ1Li42NjE2ODFmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0 Mi9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdDIvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMzQg QEAKKzIwMTQtMTItMTcgIEpvc2VwaCBQZWNvcmFybyAgPHBlY29yYXJvQGFwcGxlLmNvbT4KKwor ICAgICAgICBXZWIgSW5zcGVjdG9yOiBbTWFjXSBPY2Nvc2lvbmFsIENyYXNoZXMgQ2xvc2luZyBJ bnNwZWN0b3IKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTEzOTc0MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogVUlQcm9jZXNzL1dlYlBhZ2VQcm94eS5jcHA6CisgICAgICAgIChXZWJLaXQ6OldlYlBhZ2VQ cm94eTo6ZGlkUmVjZWl2ZUV2ZW50KToKKyAgICAgICAgVGhlIFdlYlBhZ2VQcm94eSBjb3VsZCBo YXZlIGdvdHRlbiBkZXN0cm95ZWQgaW4gaGFuZGxpbmcgKGUuZy4ga2V5Ym9hcmQgc2hvcnRjdXQK KyAgICAgICAgdG8gY2xvc2UgdGhlIEluc3BlY3RvciBXaW5kb3cpLCBzbyBwcm90ZWN0IHRoZSBv YmplY3QgdG8gYXZvaWQgZGVzdHJ1Y3Rpb24KKyAgICAgICAgZHVyaW5nIGhhbmRsaW5nLgorCisg ICAgICAgICogVUlQcm9jZXNzL1dlYkluc3BlY3RvclByb3h5Lmg6CisgICAgICAgICogVUlQcm9j ZXNzL1dlYlBhZ2VQcm94eS5jcHA6CisgICAgICAgIEFkZCBwbGF0Zm9ybUludmFsaWRhdGUgb3Bw b3J0dW5pdHkgZm9yIHRoZSBwbGF0Zm9ybSB0byBkbyBpbnZhbGlkYXRpb24gd29yay4KKworICAg ICAgICAqIFVJUHJvY2Vzcy9lZmwvV2ViSW5zcGVjdG9yUHJveHlFZmwuY3BwOgorICAgICAgICAo V2ViS2l0OjpXZWJJbnNwZWN0b3JQcm94eTo6cGxhdGZvcm1JbnZhbGlkYXRlKToKKyAgICAgICAg KiBVSVByb2Nlc3MvZ3RrL1dlYkluc3BlY3RvclByb3h5R3RrLmNwcDoKKyAgICAgICAgKFdlYktp dDo6V2ViSW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtSW52YWxpZGF0ZSk6CisgICAgICAgIEVtcHR5 IGltcGxlbWVudGF0aW9ucywgdGhlc2UgcG9ydHMgZG8gbm90IG5lZWQgdG8gZG8gYW55dGhpbmcu CisKKyAgICAgICAgKiBVSVByb2Nlc3MvV2ViSW5zcGVjdG9yUHJveHkuaDoKKyAgICAgICAgKiBV SVByb2Nlc3MvbWFjL1dlYkluc3BlY3RvclByb3h5TWFjLm1tOgorICAgICAgICAoV2ViS2l0OjpX ZWJJbnNwZWN0b3JQcm94eTo6Y2xvc2VUaW1lckZpcmVkKToKKyAgICAgICAgKFdlYktpdDo6V2Vi SW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtRGlkQ2xvc2UpOgorICAgICAgICAoV2ViS2l0OjpXZWJJ bnNwZWN0b3JQcm94eTo6cGxhdGZvcm1JbnZhbGlkYXRlKToKKyAgICAgICAgV2hlbiBjbG9zaW5n IC8gaW52YWxpZGF0aW5nIG1ha2Ugc3VyZSB3ZSBjbG9zZSB0aGUgV0tWaWV3IHRvIGNsZWFyIGNs aWVudAorICAgICAgICBmb3IgYW55IGRlbGF5ZWQgY2FsbGJhY2tzLgorCiAyMDE0LTEyLTE2ICBU aW0gSG9ydG9uICA8dGltb3RoeV9ob3J0b25AYXBwbGUuY29tPgogCiAgICAgICAgIERpc2FibGUg VGV4dEluZGljYXRpb24gZm9yIExvb2t1cCBpbW1lZGlhdGUgYWN0aW9ucwpkaWZmIC0tZ2l0IGEv U291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYkluc3BlY3RvclByb3h5LmNwcCBiL1NvdXJjZS9X ZWJLaXQyL1VJUHJvY2Vzcy9XZWJJbnNwZWN0b3JQcm94eS5jcHAKaW5kZXggYzNmNGI2OTcuLjJi MjBlNmYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJJbnNwZWN0b3JQ cm94eS5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYkluc3BlY3RvclByb3h5 LmNwcApAQCAtMTcwLDYgKzE3MCw3IEBAIHZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OmludmFsaWRh dGUoKQogICAgIG1fcGFnZS0+cHJvY2VzcygpLnJlbW92ZU1lc3NhZ2VSZWNlaXZlcihNZXNzYWdl czo6V2ViSW5zcGVjdG9yUHJveHk6Om1lc3NhZ2VSZWNlaXZlck5hbWUoKSwgbV9wYWdlLT5wYWdl SUQoKSk7CiAKICAgICBkaWRDbG9zZSgpOworICAgIHBsYXRmb3JtSW52YWxpZGF0ZSgpOwogCiAg ICAgbV9wYWdlID0gbnVsbHB0cjsKIH0KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJv Y2Vzcy9XZWJJbnNwZWN0b3JQcm94eS5oIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL1dlYklu c3BlY3RvclByb3h5LmgKaW5kZXggYWUyYjFhNS4uNmUzMWJkNSAxMDA2NDQKLS0tIGEvU291cmNl L1dlYktpdDIvVUlQcm9jZXNzL1dlYkluc3BlY3RvclByb3h5LmgKKysrIGIvU291cmNlL1dlYktp dDIvVUlQcm9jZXNzL1dlYkluc3BlY3RvclByb3h5LmgKQEAgLTE1Nyw2ICsxNTcsNyBAQCBwcml2 YXRlOgogICAgIFdlYlBhZ2VQcm94eSogcGxhdGZvcm1DcmVhdGVJbnNwZWN0b3JQYWdlKCk7CiAg ICAgdm9pZCBwbGF0Zm9ybU9wZW4oKTsKICAgICB2b2lkIHBsYXRmb3JtRGlkQ2xvc2UoKTsKKyAg ICB2b2lkIHBsYXRmb3JtSW52YWxpZGF0ZSgpOwogICAgIHZvaWQgcGxhdGZvcm1CcmluZ1RvRnJv bnQoKTsKICAgICB2b2lkIHBsYXRmb3JtSGlkZSgpOwogICAgIGJvb2wgcGxhdGZvcm1Jc0Zyb250 KCk7CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvV2ViUGFnZVByb3h5LmNw cCBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJQYWdlUHJveHkuY3BwCmluZGV4IDJmNjIy Y2QuLmY1MGIzYWIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJQYWdl UHJveHkuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9XZWJQYWdlUHJveHkuY3Bw CkBAIC00MjEzLDYgKzQyMTMsMTAgQEAgdm9pZCBXZWJQYWdlUHJveHk6OmRpZFJlY2VpdmVFdmVu dCh1aW50MzJfdCBvcGFxdWVUeXBlLCBib29sIGhhbmRsZWQpCiAgICAgICAgIGlmICghbV9rZXlF dmVudFF1ZXVlLmlzRW1wdHkoKSkKICAgICAgICAgICAgIG1fcHJvY2Vzcy0+c2VuZChNZXNzYWdl czo6V2ViUGFnZTo6S2V5RXZlbnQobV9rZXlFdmVudFF1ZXVlLmZpcnN0KCkpLCBtX3BhZ2VJRCk7 CiAKKyAgICAgICAgLy8gVGhlIGNhbGwgdG8gZG9uZVdpdGhLZXlFdmVudCBtYXkgY2xvc2UgdGhp cyBXZWJQYWdlLgorICAgICAgICAvLyBQcm90ZWN0IGFnYWluc3QgdGhpcyBiZWluZyBkZXN0cm95 ZWQuCisgICAgICAgIFJlZjxXZWJQYWdlUHJveHk+IHByb3RlY3QoKnRoaXMpOworCiAgICAgICAg IG1fcGFnZUNsaWVudC5kb25lV2l0aEtleUV2ZW50KGV2ZW50LCBoYW5kbGVkKTsKICAgICAgICAg aWYgKGhhbmRsZWQpCiAgICAgICAgICAgICBicmVhazsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJL aXQyL1VJUHJvY2Vzcy9lZmwvV2ViSW5zcGVjdG9yUHJveHlFZmwuY3BwIGIvU291cmNlL1dlYktp dDIvVUlQcm9jZXNzL2VmbC9XZWJJbnNwZWN0b3JQcm94eUVmbC5jcHAKaW5kZXggM2UyZGExMS4u YzZiZjlkMiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2VmbC9XZWJJbnNw ZWN0b3JQcm94eUVmbC5jcHAKKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL2VmbC9XZWJJ bnNwZWN0b3JQcm94eUVmbC5jcHAKQEAgLTE0Miw2ICsxNDIsMTAgQEAgdm9pZCBXZWJJbnNwZWN0 b3JQcm94eTo6cGxhdGZvcm1EaWRDbG9zZSgpCiAgICAgfQogfQogCit2b2lkIFdlYkluc3BlY3Rv clByb3h5OjpwbGF0Zm9ybUludmFsaWRhdGUoKQoreworfQorCiB2b2lkIFdlYkluc3BlY3RvclBy b3h5OjpwbGF0Zm9ybUhpZGUoKQogewogICAgIG5vdEltcGxlbWVudGVkKCk7CmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3MvZ3RrL1dlYkluc3BlY3RvclByb3h5R3RrLmNwcCBi L1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9ndGsvV2ViSW5zcGVjdG9yUHJveHlHdGsuY3BwCmlu ZGV4IGJmNmE4YzYuLjNlMDZhZTIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9ndGsvV2ViSW5zcGVjdG9yUHJveHlHdGsuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQyL1VJUHJv Y2Vzcy9ndGsvV2ViSW5zcGVjdG9yUHJveHlHdGsuY3BwCkBAIC0xNjgsNiArMTY4LDEwIEBAIHZv aWQgV2ViSW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtRGlkQ2xvc2UoKQogICAgIG1faW5zcGVjdG9y VmlldyA9IDA7CiB9CiAKK3ZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtSW52YWxpZGF0 ZSgpCit7Cit9CisKIHZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OnBsYXRmb3JtSGlkZSgpCiB7CiAg ICAgbm90SW1wbGVtZW50ZWQoKTsKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vz cy9tYWMvV2ViSW5zcGVjdG9yUHJveHlNYWMubW0gYi9Tb3VyY2UvV2ViS2l0Mi9VSVByb2Nlc3Mv bWFjL1dlYkluc3BlY3RvclByb3h5TWFjLm1tCmluZGV4IGJjMDRjMWIuLjhhYjhjYTMgMTAwNjQ0 Ci0tLSBhL1NvdXJjZS9XZWJLaXQyL1VJUHJvY2Vzcy9tYWMvV2ViSW5zcGVjdG9yUHJveHlNYWMu bW0KKysrIGIvU291cmNlL1dlYktpdDIvVUlQcm9jZXNzL21hYy9XZWJJbnNwZWN0b3JQcm94eU1h Yy5tbQpAQCAtMjkxLDcgKzI5MSwxMSBAQCB2b2lkIFdlYkluc3BlY3RvclByb3h5OjpjbG9zZVRp bWVyRmlyZWQoKQogICAgIGlmIChtX2lzQXR0YWNoZWQgfHwgbV9pbnNwZWN0b3JXaW5kb3cpCiAg ICAgICAgIHJldHVybjsKIAotICAgIG1faW5zcGVjdG9yVmlldyA9IG5pbDsKKyAgICBpZiAobV9p bnNwZWN0b3JWaWV3KSB7CisgICAgICAgIFdlYlBhZ2VQcm94eSogaW5zcGVjdG9yUGFnZSA9IHRv SW1wbChtX2luc3BlY3RvclZpZXcuZ2V0KCkucGFnZVJlZik7CisgICAgICAgIGluc3BlY3RvclBh Z2UtPmNsb3NlKCk7CisgICAgICAgIG1faW5zcGVjdG9yVmlldyA9IG5pbDsKKyAgICB9CiAKICAg ICBbbV9pbnNwZWN0b3JQcm94eU9iakNBZGFwdGVyIGNsb3NlXTsKICAgICBtX2luc3BlY3RvclBy b3h5T2JqQ0FkYXB0ZXIgPSBuaWw7CkBAIC01NDQsNiArNTQ4LDEzIEBAIHZvaWQgV2ViSW5zcGVj dG9yUHJveHk6OnBsYXRmb3JtRGlkQ2xvc2UoKQogICAgIG1fY2xvc2VUaW1lci5zdGFydE9uZVNo b3Qod2ViVmlld0Nsb3NlVGltZW91dCk7CiB9CiAKK3ZvaWQgV2ViSW5zcGVjdG9yUHJveHk6OnBs YXRmb3JtSW52YWxpZGF0ZSgpCit7CisgICAgbV9jbG9zZVRpbWVyLnN0b3AoKTsKKworICAgIGNs b3NlVGltZXJGaXJlZCgpOworfQorCiB2b2lkIFdlYkluc3BlY3RvclByb3h5OjpwbGF0Zm9ybUhp ZGUoKQogewogICAgIGlmIChtX2lzQXR0YWNoZWQpIHsK