[webkit-reviews] review granted: [Bug 42092] [Chromium] Fix adoptRef usage violation in WebAccessibilityCacheImpl.cpp : [Attachment 61267] Proposed patch.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted chris.guillory at google.com's request
for review:
Bug 42092: [Chromium] Fix adoptRef usage violation in
WebAccessibilityCacheImpl.cpp
https://bugs.webkit.org/show_bug.cgi?id=42092

Attachment 61267: Proposed patch.
https://bugs.webkit.org/attachment.cgi?id=61267&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
>      // FIXME: Remove resetting ref-count from AccessibilityObjectWrapper
> -    // and convert to use adoptRef.

Should add the period on the line you are keeping.

> -    return new WebAccessibilityCacheImpl::WeakHandle(object);
> +    RefPtr<WebAccessibilityCacheImpl::WeakHandle> weakHandle = adoptRef(new
WebAccessibilityCacheImpl::WeakHandle(object));
> +    weakHandle->m_object->setWrapper(weakHandle.get());
> +    
> +    return weakHandle;

This should be "return weakHandle.release()" for best performance. Avoids one
round of reference counting churn.

It looks like this change actually fixes a storage leak that was happening
here. Or maybe not because of whatever "resetting ref-count from
AccessibilityObjectWrapper" is.

I’ll say r=me but I think this is not completely right.