You can repro with a trivially simple HTML page that embeds a PDF, e.g.: <object data="hello.pdf" type="application/pdf" width="300" height="300"></object> Open the web page and enable VoiceOver, then try to navigate to the embedded PDF object - I get this assertion in debug mode: ASSERTION FAILED: canSafelyBeUsed() include/wtf/WeakPtr.h(138) : T *WTF::WeakPtr<WebKit::WebFrame>::operator->() const [T = WebKit::WebFrame, WeakPtrImpl = WTF::DefaultWeakPtrImpl] 1 WTFCrash 2 WTFCrashWithInfo(int, char const*, char const*, int) 3 WTF::WeakPtr<WebKit::WebFrame, WTF::DefaultWeakPtrImpl>::operator->() const 4 WebKit::PDFPlugin::axObjectCache() const 5 -[WKPDFPluginAccessibilityObject parent] 6 -[WKPDFPluginAccessibilityObject accessibilityAttributeValue:] 7 NSAccessibilityGetObjectForAttributeUsingLegacyAPI
<rdar://problem/118892293>
Created attachment 468788 [details] Patch
Comment on attachment 468788 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=468788&action=review > Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:194 > + callOnMainRunLoopAndWait([&] { if we don't do this is it a crash? also returning and storing an ivar (_parent) on a different thread might be bad
Created attachment 468789 [details] Patch
(In reply to chris fleizach from comment #3) > if we don't do this is it a crash? It's a debug assertion in WeakPtr: canSafelyBeUsed > also returning and storing an ivar (_parent) on a different thread might be > bad Got it, changed to use a local RetainPtr to move between threads
Created attachment 468790 [details] Patch
(In reply to Dominic Mazzoni from comment #6) > Created attachment 468790 [details] > Patch Use the main thread when accessing the document's axObjectCache from WKPDFPluginAccessibilityObject https://bugs.webkit.org/show_bug.cgi?id=265470 rdar://118894435 Reviewed by NOBODY (OOPS!). AG: Include here a brief explanation of the problem and solution. * Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm: (-[WKPDFPluginAccessibilityObject parent]): (WebKit::PDFPlugin::axObjectCache const): --- Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm b/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm index 4fc440c1fe89..fb77b90b95f6 100644 --- a/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm +++ b/Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm @@ -191,10 +191,14 @@ - (id)initWithPDFPlugin:(WebKit::PDFPlugin *)plugin andElement:(WebCore::HTMLPlu - (NSObject *)parent { if (!_parent) { - if (auto* axObjectCache = _pdfPlugin->axObjectCache()) { - if (RefPtr pluginAxObject = axObjectCache->getOrCreate(_pluginElement.get())) - _parent = pluginAxObject->wrapper(); - } + RetainPtr<NSObject> parent; + callOnMainRunLoopAndWait([&] { AG: I think you need to capture a strong reference to self because there is no guaranty that self is alive when the lambda is invoked on the main thread, similar to what we do in WebAccessibilityObjectWrapper. I.e., something like: callOnMainRunLoopAndWait([&parent, protectedSelf = retainPtr(self)] { and then use protectedSelf to access properties and methods in the lambda. + if (auto* axObjectCache = _pdfPlugin->axObjectCache()) { + if (RefPtr pluginAxObject = axObjectCache->getOrCreate(_pluginElement.get())) + parent = pluginAxObject->wrapper(); + } + }); + _parent = parent.get(); } return _parent.get().get(); }
Created attachment 468810 [details] Patch
(In reply to Andres Gonzalez from comment #7) > AG: I think you need to capture a strong reference to self because there is > no guaranty that self is alive when the lambda is invoked on the main > thread, similar to what we do in WebAccessibilityObjectWrapper. I.e., Thanks. Notice that the function accesses an ivar at the bottom - so I think it's actually necessary to retain self throughout the whole function scope, not just the lambda. Let me know if you disagree.
Committed 271622@main (0cab9f4dd652): <https://commits.webkit.org/271622@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 468810 [details].