Bug 60572 – REGRESSION - Canon On-screen Manual.app crashes after a search

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Brady Eidson]

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>

[Brady Eidson]

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.

[Brady Eidson]

Created attachment 92997 [details]
Patch v1

[Brady Eidson]

Fixed in r86179

[Brady Eidson]

I committed without the ChangeLog and in-code comment I'd meant to include.

[Brady Eidson]

Updated with comments in 86180

[WebKit Review Bot]

http://trac.webkit.org/changeset/86180 might have broken Windows XP Debug (Tests)
The following tests are not passing:
media/controls-without-preload.html

[Brady Eidson]

(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)

[Adam Barth]

> 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..

[Eric Seidel (no email)]

@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.

[Eric Seidel (no email)]

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".