Accessibility: AXObjectCache is cleared unnecessarily when non-top Document is detached

• Component: Accessibility
• Status: RESOLVED FIXED

[Dominic Mazzoni]

Only the top document has its own AXObjectCache, so Document::detach doesn't need to call clearAXObjectCache when the document being detached is not the top. This is needlessly causing the whole accessibility tree to be regenerated every time an iframe on a page reloads.

[Dominic Mazzoni]

Created attachment 108801 [details]
Patch

[chris fleizach]

Comment on attachment 108801 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=108801&action=review

> Source/WebCore/dom/Document.cpp:1830
> +

this has the ability to leak objects that came from an iframe until the whole document is cleared, does it not?

> LayoutTests/accessibility/deleting-iframe-destroys-axcache.html:53
> +            document.getElementById("console").innerText += "\nBefore:\n";

wrong indentation.

> LayoutTests/accessibility/deleting-iframe-destroys-axcache.html:56
> +            // Remove the iframe.

wrong indentation.

> LayoutTests/accessibility/deleting-iframe-destroys-axcache.html:64
> +            document.getElementById("console").innerText += "\nAfter:\n";

wrong indentation.

> LayoutTests/accessibility/deleting-iframe-destroys-axcache.html:66
> +            document.getElementById("console").innerText += "\n";

wrong indentation.

> LayoutTests/accessibility/deleting-iframe-destroys-axcache.html:69
> +            // are now invalid - they've been deleted.

wrong indentation.

> LayoutTests/accessibility/deleting-iframe-destroys-axcache.html:74
> +            // Make sure that the other nodes are unchanged.

wrong indentation.

[Dominic Mazzoni]

(In reply to comment #2)
> (From update of attachment 108801 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=108801&action=review
> 
> > Source/WebCore/dom/Document.cpp:1830
> > +
> 
> this has the ability to leak objects that came from an iframe until the whole document is cleared, does it not?

No, in the case where the iframe is actually deleted like this, I'm positive that AXObjectCache::remove is actually called on all of the accessibility objects within the iframe's tree. The objects are deleted when their reference count hits zero.

The test already confirms this by checking that the objects have lost their correct role, but I added my own debug logging to confirm it.

If there is any scenario where an iframe is detached but not either deleted or reattached somewhere else in the same page, then we could have a problem - but I can't think of any such circumstance.

[Dominic Mazzoni]

Created attachment 108856 [details]
Patch

[chris fleizach]

Comment on attachment 108856 [details]
Patch

looks good. r=me

[WebKit Review Bot]

Comment on attachment 108856 [details]
Patch

Clearing flags on attachment: 108856

Committed r96127: <http://trac.webkit.org/changeset/96127>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.