[webkit-reviews] review denied: [Bug 175340] AX: crash at WebCore::AccessibilityObject::supportsARIALiveRegion() const + 24 : [Attachment 317793] patch

[bugzilla-daemon at webkit.org]

zalan <zalan at apple.com> has denied Nan Wang <n_wang at apple.com>'s request for
review:
Bug 175340: AX: crash at WebCore::AccessibilityObject::supportsARIALiveRegion()
const + 24
https://bugs.webkit.org/show_bug.cgi?id=175340

Attachment 317793: patch

https://bugs.webkit.org/attachment.cgi?id=317793&action=review




--- Comment #20 from zalan <zalan at apple.com> ---
Comment on attachment 317793
  --> https://bugs.webkit.org/attachment.cgi?id=317793
patch

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

> Source/WebCore/rendering/RenderImage.cpp:156
> +    // Remove this from accessibility first, since the below shutdown()
function
> +    // will clear m_cachedImage and lead to a stale ax render object.
> +    if (UNLIKELY(AXObjectCache::accessibilityEnabled())) {
> +	   if (AXObjectCache* cache = document().existingAXObjectCache())
> +	       cache->remove(this);
> +    }

Do you mind explaining what you mean by "clear m_cachedImage and lead to a
stale ax render object". What's the connection between the m_cachedImage and
the AX render object? What exactly prevents us from going through the normal
RenderObject::willBeDestroyed path? Also, though Chris might know more about
this, but this changeset makes a call to AXObjectCache::remove() before
notifying the parent (please see the related comments in
RenderObject::willBeDestoryed()).
If the problem here is that the AccessibilitySVGRoot outlives the AX
RenderImage (which is correct, since svg root is a resource, while the
RenderImage is a renderer), then it needs to be addressed in another way.

I tried to debug this myself but the attached test case (in second patch)
passes fine even with guard malloc. -is it ASan only?