Bug 29337 – REGRESSION (r48060): unrepro but frequent crash in WebViewWndProc

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

Created attachment 39703 [details]
Patch v1

Many people have been seeing crashes in WebViewWndProc when browsing around with ToT builds.

[Adam Roben (:aroben)]

<rdar://problem/7209897>

[Adam Roben (:aroben)]

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.

[Adam Roben (:aroben)]

Committed r48473: <http://trac.webkit.org/changeset/48473>