RESOLVED FIXED 184919
AX: soft link libAccessibility.dylb
https://bugs.webkit.org/show_bug.cgi?id=184919
Summary AX: soft link libAccessibility.dylb
Nan Wang
Reported 2018-04-24 09:32:52 PDT
Crash on some machine Dyld Error Message: Library not loaded: /usr/lib/libAccessibility.dylib Referenced from: /System/Library/Frameworks/WebKit.framework/Versions/A/WebKit Reason: image not found <rdar://problem/39669349>
Attachments
patch (7.49 KB, patch)
2018-04-24 09:42 PDT, Nan Wang
mitz: review-
patch (2.94 KB, patch)
2018-04-24 18:40 PDT, Nan Wang
no flags
patch (3.17 KB, patch)
2018-04-24 21:43 PDT, Nan Wang
mitz: review+
Nan Wang
Comment 1 2018-04-24 09:42:33 PDT
WebKit Commit Bot
Comment 2 2018-04-24 14:27:06 PDT
Comment on attachment 338655 [details] patch Clearing flags on attachment: 338655 Committed r230971: <https://trac.webkit.org/changeset/230971>
WebKit Commit Bot
Comment 3 2018-04-24 14:27:08 PDT
All reviewed patches have been landed. Closing bug.
mitz
Comment 4 2018-04-24 14:51:19 PDT
Comment on attachment 338655 [details] patch This is not an appropriate way to resolve this sort of issue. The correct fix would have been to use $(WK_MACOS_WEAK_FRAMEWORK) instead of -framework, and to perform the appropriate nil checks.
WebKit Commit Bot
Comment 5 2018-04-24 15:05:03 PDT
Re-opened since this is blocked by bug 184939
Nan Wang
Comment 6 2018-04-24 15:05:31 PDT
(In reply to mitz from comment #4) > Comment on attachment 338655 [details] > patch > > This is not an appropriate way to resolve this sort of issue. The correct > fix would have been to use $(WK_MACOS_WEAK_FRAMEWORK) instead of -framework, > and to perform the appropriate nil checks. Ok I rolled it out and will fix that again
Nan Wang
Comment 7 2018-04-24 18:40:42 PDT
Created attachment 338695 [details] patch weak link instead
mitz
Comment 8 2018-04-24 20:10:26 PDT
Comment on attachment 338695 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=338695&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:3079 > + if (functionPtr) You generally can’t null-check function pointers like this. Consider using isNullFunctionPointer() from <https://trac.webkit.org/browser/webkit/trunk/Source/WTF/wtf/darwin/WeakLinking.h>.
Nan Wang
Comment 9 2018-04-24 21:43:41 PDT
Created attachment 338706 [details] patch update from review
mitz
Comment 10 2018-04-24 21:49:51 PDT
Comment on attachment 338706 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=338706&action=review > Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:706 > + CFNotificationCenterAddObserver(CFNotificationCenterGetDarwinNotifyCenter(), (__bridge const void *)(self), accessibilityEventsEnabledChangedCallback, kAXSWebAccessibilityEventsEnabledNotification, 0, CFNotificationSuspensionBehaviorDeliverImmediately); Not new to this patch, but why is it OK to add this observer and never remove it?
Nan Wang
Comment 11 2018-04-24 21:55:43 PDT
Comment on attachment 338706 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=338706&action=review >> Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:706 >> + CFNotificationCenterAddObserver(CFNotificationCenterGetDarwinNotifyCenter(), (__bridge const void *)(self), accessibilityEventsEnabledChangedCallback, kAXSWebAccessibilityEventsEnabledNotification, 0, CFNotificationSuspensionBehaviorDeliverImmediately); > > Not new to this patch, but why is it OK to add this observer and never remove it? Good catch, there is CFNotificationCenterRemoveObserver in dealloc but it's guarded in PLATFORM(IOS). I will move that into #if ENABLE(ACCESSIBILITY_EVENTS) since that's the only place with CFNotification
Nan Wang
Comment 12 2018-04-24 22:52:59 PDT
Note You need to log in before you can comment on or make changes to this bug.