60572 2011-05-10 12:57:20 -0700 REGRESSION - Canon On-screen Manual.app crashes after a search 2011-05-11 10:39:11 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Mac All RESOLVED FIXED InRadar P2 Normal --- 1 beidson beidson abarth eric webkit.review.bot oldest_to_newest 401429 0 beidson 2011-05-10 12:57:20 -0700 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> 401430 1 beidson 2011-05-10 12:59:13 -0700 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. 401432 2 92997 beidson 2011-05-10 13:01:32 -0700 Created attachment 92997 Patch v1 401442 3 beidson 2011-05-10 13:26:29 -0700 Fixed in r86179 401485 4 beidson 2011-05-10 14:12:05 -0700 I committed without the ChangeLog and in-code comment I'd meant to include. 401497 5 beidson 2011-05-10 14:19:58 -0700 Updated with comments in 86180 401609 6 webkit.review.bot 2011-05-10 16:10:51 -0700 http://trac.webkit.org/changeset/86180 might have broken Windows XP Debug (Tests) The following tests are not passing: media/controls-without-preload.html 401615 7 beidson 2011-05-10 16:18:57 -0700 (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) 401777 8 abarth 2011-05-10 22:34:29 -0700 > 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.. 401779 9 eric 2011-05-10 22:38:07 -0700 @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. 402098 10 eric 2011-05-11 10:39:11 -0700 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". 92997 2011-05-10 13:01:32 -0700 2011-05-10 13:21:00 -0700 Patch v1 patch.txt text/plain 3016 beidson SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg2MTc4KQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTEtMDUtMTAgIEJyYWR5IEVp ZHNvbiAgPGJlaWRzb25AYXBwbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS85MzQzMTkxPiBhbmQgaHR0cHM6Ly9i dWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYwNTcyCisKKyAgICAgICAgTm8gbmV3IHRl c3RzIC0gVGhlcmUncyBubyB3YXkgdG8gcmVwcm9kdWNlIHRoZSB0cnVseSBiaXphcnJlIHRoaW5n cyBDYW5vbiBkb2VzIGluIHRoZWlyIGFwcAorICAgICAgICB0aGF0IHdvdWxkIHJlYXNvbmFibHkg Zml0IGluIHRvIG91ciB0ZXN0aW5nIGluZnJhc3RydWN0dXJlLgorCisgICAgICAgIEFkZCBhIHRp bWVyIHRvIFNldHRpbmdzIGFuZCBpbnN0ZWFkIG9mIGNhbGxpbmcgc2V0TG9hZHNJbWFnZXNBdXRv bWF0aWNhbGx5SW5BbGxGcmFtZXMgcmlnaHQKKyAgICAgICAgYXdheSB3aGVuIHRoaXMgcHJlZiBj aGFuZ2VzLCBjYWxsIGl0IG9uIGEgMC1kZWxheToKKyAgICAgICAgKiBwYWdlL1NldHRpbmdzLmNw cDoKKyAgICAgICAgKFdlYkNvcmU6OlNldHRpbmdzOjpTZXR0aW5ncyk6CisgICAgICAgIChXZWJD b3JlOjpTZXR0aW5nczo6c2V0TG9hZHNJbWFnZXNBdXRvbWF0aWNhbGx5KToKKyAgICAgICAgKFdl YkNvcmU6OlNldHRpbmdzOjpsb2Fkc0ltYWdlc0F1dG9tYXRpY2FsbHlUaW1lckZpcmVkKToKKyAg ICAgICAgKiBwYWdlL1NldHRpbmdzLmg6CisKIDIwMTEtMDUtMTAgIE1hcnRpbiBSb2JpbnNvbiAg PG1yb2JpbnNvbkBpZ2FsaWEuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IFhhbiBMb3Blei4K SW5kZXg6IFNvdXJjZS9XZWJDb3JlL3BhZ2UvU2V0dGluZ3MuY3BwCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJDb3JlL3BhZ2UvU2V0dGluZ3MuY3BwCShyZXZpc2lvbiA4NjE2OCkKKysrIFNvdXJj ZS9XZWJDb3JlL3BhZ2UvU2V0dGluZ3MuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC0xODIsNiArMTgy LDcgQEAgU2V0dGluZ3M6OlNldHRpbmdzKFBhZ2UqIHBhZ2UpCiAgICAgLCBtX3Nob3VsZEluamVj dFVzZXJTY3JpcHRzSW5Jbml0aWFsRW1wdHlEb2N1bWVudChmYWxzZSkKICAgICAsIG1fYWxsb3dE aXNwbGF5T2ZJbnNlY3VyZUNvbnRlbnQodHJ1ZSkKICAgICAsIG1fYWxsb3dSdW5uaW5nT2ZJbnNl Y3VyZUNvbnRlbnQodHJ1ZSkKKyAgICAsIG1fbG9hZHNJbWFnZXNBdXRvbWF0aWNhbGx5VGltZXIo dGhpcywgJlNldHRpbmdzOjpsb2Fkc0ltYWdlc0F1dG9tYXRpY2FsbHlUaW1lckZpcmVkKQogewog ICAgIC8vIEEgRnJhbWUgbWF5IG5vdCBoYXZlIGJlZW4gY3JlYXRlZCB5ZXQsIHNvIHdlIGluaXRp YWxpemUgdGhlIEF0b21pY1N0cmluZyAKICAgICAvLyBoYXNoIGJlZm9yZSB0cnlpbmcgdG8gdXNl IGl0LgpAQCAtMjgxLDcgKzI4Miw3IEBAIHZvaWQgU2V0dGluZ3M6OnNldERlZmF1bHRGaXhlZEZv bnRTaXplKGkKIHZvaWQgU2V0dGluZ3M6OnNldExvYWRzSW1hZ2VzQXV0b21hdGljYWxseShib29s IGxvYWRzSW1hZ2VzQXV0b21hdGljYWxseSkKIHsKICAgICBtX2xvYWRzSW1hZ2VzQXV0b21hdGlj YWxseSA9IGxvYWRzSW1hZ2VzQXV0b21hdGljYWxseTsKLSAgICBzZXRMb2Fkc0ltYWdlc0F1dG9t YXRpY2FsbHlJbkFsbEZyYW1lcyhtX3BhZ2UpOworICAgIG1fbG9hZHNJbWFnZXNBdXRvbWF0aWNh bGx5VGltZXIuc3RhcnRPbmVTaG90KDApOwogfQogCiB2b2lkIFNldHRpbmdzOjpzZXRMb2Fkc1Np dGVJY29uc0lnbm9yaW5nSW1hZ2VMb2FkaW5nU2V0dGluZyhib29sIGxvYWRzU2l0ZUljb25zKQpA QCAtNzQyLDQgKzc0Myw5IEBAIHZvaWQgU2V0dGluZ3M6OnNldFRpbGVkQmFja2luZ1N0b3JlRW5h YmwKICNlbmRpZgogfQogCit2b2lkIFNldHRpbmdzOjpsb2Fkc0ltYWdlc0F1dG9tYXRpY2FsbHlU aW1lckZpcmVkKFRpbWVyPFNldHRpbmdzPiopCit7CisgICAgc2V0TG9hZHNJbWFnZXNBdXRvbWF0 aWNhbGx5SW5BbGxGcmFtZXMobV9wYWdlKTsKK30KKwogfSAvLyBuYW1lc3BhY2UgV2ViQ29yZQpJ bmRleDogU291cmNlL1dlYkNvcmUvcGFnZS9TZXR0aW5ncy5oCj09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJj ZS9XZWJDb3JlL3BhZ2UvU2V0dGluZ3MuaAkocmV2aXNpb24gODYxNjgpCisrKyBTb3VyY2UvV2Vi Q29yZS9wYWdlL1NldHRpbmdzLmgJKHdvcmtpbmcgY29weSkKQEAgLTMwLDYgKzMwLDcgQEAKICNp bmNsdWRlICJFZGl0aW5nQmVoYXZpb3JUeXBlcy5oIgogI2luY2x1ZGUgIkZvbnRSZW5kZXJpbmdN b2RlLmgiCiAjaW5jbHVkZSAiS1VSTC5oIgorI2luY2x1ZGUgIlRpbWVyLmgiCiAjaW5jbHVkZSA8 d3RmL3RleHQvQXRvbWljU3RyaW5nLmg+CiAKIG5hbWVzcGFjZSBXZWJDb3JlIHsKQEAgLTUxNCw2 ICs1MTUsOSBAQCBuYW1lc3BhY2UgV2ViQ29yZSB7CiAgICAgICAgIGJvb2wgbV9hbGxvd0Rpc3Bs YXlPZkluc2VjdXJlQ29udGVudCA6IDE7CiAgICAgICAgIGJvb2wgbV9hbGxvd1J1bm5pbmdPZklu c2VjdXJlQ29udGVudCA6IDE7CiAKKyAgICAgICAgVGltZXI8U2V0dGluZ3M+IG1fbG9hZHNJbWFn ZXNBdXRvbWF0aWNhbGx5VGltZXI7CisgICAgICAgIHZvaWQgbG9hZHNJbWFnZXNBdXRvbWF0aWNh bGx5VGltZXJGaXJlZChUaW1lcjxTZXR0aW5ncz4qKTsKKwogI2lmIFVTRShBVkZPVU5EQVRJT04p CiAgICAgICAgIHN0YXRpYyBib29sIGdBVkZvdW5kYXRpb25FbmFibGVkOwogI2VuZGlmCg==