Created attachment 39703 [details] Patch v1 Many people have been seeing crashes in WebViewWndProc when browsing around with ToT builds.
<rdar://problem/7209897>
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.
Committed r48473: <http://trac.webkit.org/changeset/48473>