545738 - clean up nsAccessibilityService::GetAccessible() and related

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[alexander :surkov (:asurkov)]

https://build.mozilla.org/tryserver-builds/[e-mail]-try-dcd9e3c79010/

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 426657 [details] [diff] [review]
patch

I probably need coffee but:

>+already_AddRefed<nsAccessible>
>+nsAccessibilityService::CreateAccessibleForDeckChild(nsIFrame* aFrame,
>+                                                     nsIDOMNode *aNode,
>+                                                     nsIWeakReference *aWeakShell)
> {

>-  return NS_OK;
>+  return accessible.forget();
> }

This doesn't look safe since we haven't addrefed the accessible. You do this in several places...

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)
> (From update of attachment 426657 [details] [diff] [review])
> I probably need coffee but:

yep, please :)

> >+already_AddRefed<nsAccessible>
> >+nsAccessibilityService::CreateAccessibleForDeckChild(nsIFrame* aFrame,
> >+                                                     nsIDOMNode *aNode,
> >+                                                     nsIWeakReference *aWeakShell)
> > {
> 
> >-  return NS_OK;
> >+  return accessible.forget();
> > }
> 
> This doesn't look safe since we haven't addrefed the accessible. You do this in
> several places...

forget() forgets the pointer and returns already_AddRefed

Comment 5

[David Bolter [:davidb] (NeedInfo me for attention)]

Ah right the caller is using a smart pointer.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Comment on attachment 426657 [details] [diff] [review]
patch

r=me. Nice changes.

Comment 7

[alexander :surkov (:asurkov)]

landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/acab380e4de3