29337 2009-09-17 10:26:26 -0700 REGRESSION (r48060): unrepro but frequent crash in WebViewWndProc 2009-09-17 10:34:16 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Other Windows XP RESOLVED FIXED InRadar, PlatformOnly, Regression P2 Normal --- 1 aroben webkit-unassigned bweinstein sfalken oldest_to_newest 147831 0 39703 aroben 2009-09-17 10:26:26 -0700 Created attachment 39703 Patch v1 Many people have been seeing crashes in WebViewWndProc when browsing around with ToT builds. 147834 1 aroben 2009-09-17 10:27:38 -0700 <rdar://problem/7209897> 147836 2 aroben 2009-09-17 10:28:13 -0700 Some comments from our other bug system: 9/10/09 1:27 PM Lena Dochez: I just got this crash using Safari for 30 min when logging into yahoo mail. 9/14/09 5:35 PM Brian Weinstein: I was able to reproduce it relatively quickly (not 100% consistently though), by running Sunspider over and over, it took anywhere from 2-5 runs of Sunspider to hit the crash. 9/14/09 6:02 PM Alice Liu: the hwnd value in the crashing function can't be found among the window handles when observing safari windows with spy++. Would this mean that maybe the crash happens when a message is being sent to a window that has been destroyed? 9/14/09 8:01 PM Brian Weinstein: Both times I have caught this crash (from Sunspider), it has been a WM_TIMER call, where wParam was 1 (UpdateActiveStateTimer) and lParam was 0. The HWND that was passed in was nowhere to be found in Spy++, like Alice mentioned. 9/16/09 10:10 AM Adam Roben: I've been trying to reproduce this today. I've tried running Sunspider and the stress test, and haven't seen it yet. 9/16/09 10:32 AM Adam Roben: I also haven't reproduced it yet by browsing around with a ToT build. 9/16/09 12:20 PM Adam Roben: Hm, I just got the crash in a ToT build, but unfortunately it was a release build. 9/16/09 4:47 PM Brian Weinstein: It looks like this was caused by http://trac.webkit.org/changeset/48060. A way I have found to reproduce it is to: run-webkit-tests LayoutTests/http/tests/xmlhttprequest, and attach DumpRenderTree to the debugger. You have run into the crash if it crashes and the top or 2nd highest level in the call stack is WebViewWndProc. 9/16/09 4:48 PM Brian Weinstein: I narrowed it down to this revision by using spade builds, and then confirmed on a ToT build without this revision that this is what caused the crash. 9/17/09 11:57 AM Adam Roben: OK, I'm able to reproduce sometimes by running the xmlhttprequest tests. I tried changing UpdateActiveStateTimer to a different number (I chose 6), and the crash still happened, with wParam=6. So there's nothing special about wParam=1. 9/17/09 12:06 PM Adam Roben: We only create the UpdateActiveStateTimer in WebView::updateActiveStateSoon(). That function is only called in two places: when a WebView receives a WM_WINDOWPOSCHANGED message, and when a WebView's top-level parent window receives a WM_NCACTIVATE message (which WebView observes via WebView::windowReceivedMessage and WindowMessageBroadcaster). 9/17/09 12:22 PM Adam Roben: It looks like WM_WINDOWPOSCHANGED is never called during the xmlhttprequest tests (which isn't surprising; I don't think we allow the offscreen WebView to be moved). We do receive WM_NCACTIVATE messages, though. This must be how the timer is getting set. 9/17/09 12:34 PM Adam Roben: It looks like we're getting a windowMessageReceived call after our destructor runs. Maybe that's the cause of the bug. 9/17/09 12:36 PM Adam Roben: My last comment was mistaken; the windowMessageReceived call is being made on a different WebView from the one whose destructor ran. 9/17/09 1:08 PM Adam Roben: Steve speculated that perhaps the problem stems from us calling SetParent while handling WM_DESTROY. I'm trying a fix that makes setHostWindow(0) not call SetParent after WM_DESTROY is received; testing so far shows promise. 147840 3 aroben 2009-09-17 10:34:16 -0700 Committed r48473: <http://trac.webkit.org/changeset/48473> 39703 2009-09-17 10:26:26 -0700 2009-09-17 10:30:37 -0700 Patch v1 20090917132626.patch text/plain 2541 aroben ZGlmZiAtLWdpdCBhL1dlYktpdC93aW4vQ2hhbmdlTG9nIGIvV2ViS2l0L3dpbi9DaGFuZ2VMb2cK aW5kZXggMWNmZTg4Yy4uMDZkODI2NyAxMDA2NDQKLS0tIGEvV2ViS2l0L3dpbi9DaGFuZ2VMb2cK KysrIGIvV2ViS2l0L3dpbi9DaGFuZ2VMb2cKQEAgLTEsMyArMSwyOSBAQAorMjAwOS0wOS0xNyAg QWRhbSBSb2JlbiAgPGFyb2JlbkBhcHBsZS5jb20+CisKKyAgICAgICAgRml4IGEgY3Jhc2ggd2hl biBhIFdlYlZpZXcncyBIV05EIGlzIGRlc3Ryb3llZCBiZWZvcmUgSVdlYlZpZXc6OmNsb3NlCisg ICAgICAgIGlzIGNhbGxlZAorCisgICAgICAgIFdlYlZpZXcncyBIV05EIHdhcyBiZWluZyBzZW50 IGEgV01fVElNRVIgbWVzc2FnZSBhZnRlciB0aGUgV2ViVmlldyB3YXMKKyAgICAgICAgZGVzdHJv eWVkLiBUaGlzIG5vcm1hbGx5IGlzbid0IHBvc3NpYmxlICh3ZSBjYWxsIERlc3Ryb3lXaW5kb3cg aW5zaWRlCisgICAgICAgIFdlYlZpZXcncyBkZXN0cnVjdG9yIHRvIGVuc3VyZSB0aGUgSFdORCBk b2Vzbid0IG91dGxpdmUgdGhlIFdlYlZpZXcpLAorICAgICAgICBidXQgcjQ4MDYwIGNoYW5nZWQg V2ViVmlldyB0byBjYWxsIFNldFBhcmVudChtX3ZpZXdXaW5kb3csCisgICAgICAgIEhXTkRfTUVT U0FHRSkgd2hpbGUgaGFuZGxpbmcgV01fREVTVFJPWSwgYW5kIGFwcGFyZW50bHkgdGhpcyBwdXRz IHRoZQorICAgICAgICBIV05EIGludG8gc29tZSB3ZWlyZCBoYWxmLWRlc3Ryb3llZCBzdGF0ZSB3 aGVyZSBpdCBjYW4gc3RpbGwgcmVjZWl2ZQorICAgICAgICBXTV9USU1FUiBtZXNzYWdlcy4KKwor ICAgICAgICBOZWVkIGEgc2hvcnQgZGVzY3JpcHRpb24gYW5kIGJ1ZyBVUkwgKE9PUFMhKQorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogV2ViVmlldy5j cHA6CisgICAgICAgIChXZWJWaWV3V25kUHJvYyk6IFdoZW4gaGFuZGxpbmcgV01fREVTVFJPWSwg Y2FsbCBzZXRJc0JlaW5nRGVzdHJveWVkKCkKKyAgICAgICAgYmVmb3JlIGNhbGxpbmcgY2xvc2Uo KS4gVGhpcyBhbGxvd3Mgc2V0SG9zdFdpbmRvdyB0byBkZXRlY3QgdGhhdCB3ZSdyZQorICAgICAg ICBoYW5kbGluZyBXTV9ERVNUUk9ZLiBObyBvdGhlciBjb2RlIGluIGNsb3NlKCkgY2FyZWQgd2hl dGhlcgorICAgICAgICBpc0JlaW5nRGVzdHJveWVkKCkgd2FzIHRydWUgb3Igbm90LgorICAgICAg ICAoV2ViVmlldzo6c2V0SG9zdFdpbmRvdyk6IElmIG91ciBob3N0IHdpbmRvdyBpcyBiZWluZyBz ZXQgdG8gMCBhbmQKKyAgICAgICAgd2UncmUgYmVpbmcgZGVzdHJveWVkLCBkb24ndCBiZWNvbWUg YSBtZXNzYWdlLW9ubHkgd2luZG93LiBEb2luZyB0aGlzCisgICAgICAgIHdoaWxlIGhhbmRsaW5n IFdNX0RFU1RST1kgc2VlbXMgdG8gbGVhZCB0byBjcmFzaGVzLgorCiAyMDA5LTA5LTE2ICBBZGFt IFJvYmVuICA8YXJvYmVuQGFwcGxlLmNvbT4KIAogICAgICAgICBBZGQgYSBVSSBkZWxlZ2F0ZSBj YWxsYmFjayB0byBsZXQgdGhlIGRlbGVnYXRlIGRyYXcgYSBXZWJWaWV3J3MKZGlmZiAtLWdpdCBh L1dlYktpdC93aW4vV2ViVmlldy5jcHAgYi9XZWJLaXQvd2luL1dlYlZpZXcuY3BwCmluZGV4IGI3 MmRhYjcuLjc3NGI3MzUgMTAwNjQ0Ci0tLSBhL1dlYktpdC93aW4vV2ViVmlldy5jcHAKKysrIGIv V2ViS2l0L3dpbi9XZWJWaWV3LmNwcApAQCAtMTg3OCw4ICsxODc4LDggQEAgc3RhdGljIExSRVNV TFQgQ0FMTEJBQ0sgV2ViVmlld1duZFByb2MoSFdORCBoV25kLCBVSU5UIG1lc3NhZ2UsIFdQQVJB TSB3UGFyYW0sIEwKICAgICAgICAgICAgIHdlYlZpZXctPnBhaW50KChIREMpd1BhcmFtLCBsUGFy YW0pOwogICAgICAgICAgICAgYnJlYWs7CiAgICAgICAgIGNhc2UgV01fREVTVFJPWToKLSAgICAg ICAgICAgIHdlYlZpZXctPmNsb3NlKCk7CiAgICAgICAgICAgICB3ZWJWaWV3LT5zZXRJc0JlaW5n RGVzdHJveWVkKCk7CisgICAgICAgICAgICB3ZWJWaWV3LT5jbG9zZSgpOwogICAgICAgICAgICAg d2ViVmlldy0+cmV2b2tlRHJhZ0Ryb3AoKTsKICAgICAgICAgICAgIGJyZWFrOwogICAgICAgICBj YXNlIFdNX0dFU1RVUkVOT1RJRlk6CkBAIC0yOTk1LDggKzI5OTUsMTIgQEAgSFJFU1VMVCBTVERN RVRIT0RDQUxMVFlQRSBXZWJWaWV3OjpzZXRIb3N0V2luZG93KAogICAgIC8qIFtpbl0gKi8gT0xF X0hBTkRMRSBvbGVXaW5kb3cpCiB7CiAgICAgSFdORCB3aW5kb3cgPSAoSFdORCkoVUxPTkc2NClv bGVXaW5kb3c7Ci0gICAgaWYgKG1fdmlld1dpbmRvdykKLSAgICAgICAgU2V0UGFyZW50KG1fdmll d1dpbmRvdywgd2luZG93ID8gd2luZG93IDogSFdORF9NRVNTQUdFKTsKKyAgICBpZiAobV92aWV3 V2luZG93KSB7CisgICAgICAgIGlmICh3aW5kb3cpCisgICAgICAgICAgICBTZXRQYXJlbnQobV92 aWV3V2luZG93LCB3aW5kb3cpOworICAgICAgICBlbHNlIGlmICghaXNCZWluZ0Rlc3Ryb3llZCgp KQorICAgICAgICAgICAgU2V0UGFyZW50KG1fdmlld1dpbmRvdywgSFdORF9NRVNTQUdFKTsKKyAg ICB9CiAKICAgICBtX2hvc3RXaW5kb3cgPSB3aW5kb3c7