A lot of crashes for this one. What happens is when someone holds onto a AXTextMarker and then tries to do something with it after the node that the text marker references is deallocated. we need a way to know whether a node is gone or not before trying to make a VisiblePosition out of it
Created attachment 67850 [details] patch The solution in this patch is: The AXObjectCache instance now keeps a HashSet of Node's being used. When a node becomes deallocated, it removes itself from the HashSet. When creating a VisiblePosition from an AXTextMarker, the cache can then check if the node is valid before proceeding.
Attachment 67850 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1 WebCore/dom/Node.cpp:30: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 67852 [details] patch
The mac EWS hung processing this patch. Suggesting it failed to build (we have a subprocess problem with our python, wherby if the subprocess -- in this case buildw-ebkit -- produces to much output, we deadlock. Workign on a fix.)
Comment on attachment 67852 [details] patch The EWS is hanging on this patch, suggesting it's a build failure. I'm still trying to diagnose the EWS hang and get you a build log, but for now i'm setting r- to not hang the queue overnight. If this builds fine on your mac, feel free to set r? again and if the queue hangs that's my fault and I'll deal. :)
FYI the Mac EWS is currently Mac Leopard.
(In reply to comment #5) > (From update of attachment 67852 [details]) > The EWS is hanging on this patch, suggesting it's a build failure. I'm still trying to diagnose the EWS hang and get you a build log, but for now i'm setting r- to not hang the queue overnight. > > If this builds fine on your mac, feel free to set r? again and if the queue hangs that's my fault and I'll deal. :) SnowLeopard was building for me. Possible it was leopard related.
Created attachment 68101 [details] patch had no problems compiling this morning after updating. let's see if this compiles.
looks like it built on mac alright now
Comment on attachment 68101 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=68101&action=review > WebCore/dom/Node.cpp:386 > + m_document->axObjectCache()->removeNodeForUse(this); What's the performance impact of this? For whom is AXObjectCache::accessibilityEnabled() true?
(In reply to comment #10) > (From update of attachment 68101 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=68101&action=review > > > WebCore/dom/Node.cpp:386 > > + m_document->axObjectCache()->removeNodeForUse(this); > > What's the performance impact of this? For whom is AXObjectCache::accessibilityEnabled() true? Non that was noticeable. We're already doing the same thing for all RenderObjects, which hasn't proved to be a perf. bottleneck. so at worse this will be 2x what the RenderObject code imposes, but most likely will be less, since we don't need to actually store very many Nodes, so that HashMap will generally be small accessibilityEnabled() only turns on when someone is using VoiceOver (or a screen reader) or using something accessing the accessibility info. so for the vast majority of customers, this is a no-op.
http://trac.webkit.org/changeset/68541