|Summary:||REGRESSION - Canon On-screen Manual.app crashes after a search|
|Product:||WebKit||Reporter:||Brady Eidson <beidson>|
|Component:||WebKit Misc.||Assignee:||Brady Eidson <beidson>|
|Severity:||Normal||CC:||abarth, eric, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description Brady Eidson 2011-05-10 12:57:20 PDT
REGRESSION - Canon On-screen Manual.app crashes after a search. Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 libobjc.A.dylib 0x9a4bda53 _class_getName + 10 1 libobjc.A.dylib 0x9a4c20e8 object_getClassName + 33 2 libobjc.A.dylib 0x9a4ca8d7 _objc_error + 57 3 libobjc.A.dylib 0x9a4caadd __objc_error + 45 4 libobjc.A.dylib 0x9a4c827c _freedHandler + 53 5 com.apple.Foundation 0x9aee937f -[NSConcreteNotification dealloc] + 59 6 libobjc.A.dylib 0x9a4d7c07 _objc_rootRelease + 47 7 libobjc.A.dylib 0x9a4d80c6 (anonymous namespace)::AutoreleasePoolPage::pop(void*) + 404 8 com.apple.CoreFoundation 0x98c9f495 _CFAutoreleasePoolPop + 53 9 com.apple.Foundation 0x9af210fd postQueueNotifications + 1149 10 com.apple.CoreFoundation 0x98d5fe6e __CFRUNLOOP_IS_CALLING_OUT_TO_AN_OBSERVER_CALLBACK_FUNCTION__ + 30 11 com.apple.CoreFoundation 0x98ce141d __CFRunLoopDoObservers + 413 12 com.apple.CoreFoundation 0x98ca239d __CFRunLoopRun + 1261 13 com.apple.CoreFoundation 0x98ca1a9c CFRunLoopRunSpecific + 332 14 com.apple.CoreFoundation 0x98ca1948 CFRunLoopRunInMode + 120 15 com.apple.HIToolbox 0x900fa313 RunCurrentEventLoopInMode + 318 16 com.apple.HIToolbox 0x900fa0db ReceiveNextEventCommon + 381 17 com.apple.HIToolbox 0x900f9f4a BlockUntilNextEventMatchingListInMode + 88 18 com.apple.AppKit 0x99a4e5dc _DPSNextEvent + 678 19 com.apple.AppKit 0x99a4de49 -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 113 20 com.apple.AppKit 0x99a0feb4 -[NSApplication run] + 897 21 com.apple.AppKit 0x99a08004 NSApplicationMain + 1047 22 jp.co.canon.ij.easy-guide-viewer 0x000028ca _start + 216 23 jp.co.canon.ij.easy-guide-viewer 0x000027f1 start + 41 The Canon app subclasses WebView, and in their own [CustomWebView dealloc] method, they manipulates WebPreferences. One thing they do before starting a search is disabling automatic image loading. One thing they do in their subclasses dealloc method is reenable automatic image loading. This crash started in r66577 when we started telling all Frames in the Page that they can start image loading, which they immediately do. This dispatches out to the delegates and re-retains the WebView, adding it to various collections and notifications. When one of these Notifications is later dealloc'ed after the runloop has spun, the WebView *has* actually been destroyed and free'd, and the attempt to lower its ref count crashes. In radar as <rdar://problem/9343191>
Comment 1 Brady Eidson 2011-05-10 12:59:13 PDT
Since the trouble originates in their subclassed dealloc method, we have no idea we're about to be dealloced. We just assume this is a normal [WebView close] and have no way of preventing the retain badness happening at this stage. When the preference is twiddled to allow image loading, there's absolutely no reason why we'd have to start the loads synchronously. Putting that on a 0-delay timer makes this go away which no real side effects.
Comment 4 Brady Eidson 2011-05-10 14:12:05 PDT
I committed without the ChangeLog and in-code comment I'd meant to include.
Comment 5 Brady Eidson 2011-05-10 14:19:58 PDT
Updated with comments in 86180
Comment 6 WebKit Review Bot 2011-05-10 16:10:51 PDT
http://trac.webkit.org/changeset/86180 might have broken Windows XP Debug (Tests) The following tests are not passing: media/controls-without-preload.html
Comment 7 Brady Eidson 2011-05-10 16:18:57 PDT
(In reply to comment #6) > http://trac.webkit.org/changeset/86180 might have broken Windows XP Debug (Tests) > The following tests are not passing: > media/controls-without-preload.html Appreciate the suggestion, but it's extremely unlikely that adding comments to code broke a layout test. (And almost as unlikely that the original preferences-related patch broke a media test that doesn't invoke that preference)
Comment 8 Adam Barth 2011-05-10 22:34:29 PDT
> Appreciate the suggestion, but it's extremely unlikely that adding comments to code broke a layout test. (And almost as unlikely that the original preferences-related patch broke a media test that doesn't invoke that preference) Yeah, we really need to work on the spaminess of these notifications. Sometimes I'm tempted to turn them off..
Comment 9 Eric Seidel (no email) 2011-05-10 22:38:07 PDT
@abarth, @beidson: I'll spend some time tomorrow working on making the SB smarter about which bugs it comments on. There is a lot of low hanging fruit here.
Comment 10 Eric Seidel (no email) 2011-05-11 10:39:11 PDT
Adam has turned off notifications when SB would notify more than 3 bugs. I'm not sure if that would have helped here, but it should reduce the times when he's just "spamming".