750026 - rm some unneeded nsCOMPtrs

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Trevor Saunders (:tbsaunde)]

I'm pretty sure we don't need owning refs in any of these places.

Comment 1

[Trevor Saunders (:tbsaunde)]

Attachment: don't use nsCOMPtrs with the presShell unless we need to

Comment 2

[Trevor Saunders (:tbsaunde)]

Attachment: don't keep a ref to an atom when we don't need to

Comment 3

[alexander :surkov (:asurkov)]

Comment on attachment 619367 [details] [diff] [review]
don't keep a ref to an atom when we don't need to

Review of attachment 619367 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/html/nsHTMLSelectAccessible.cpp
@@ +190,2 @@
>      if (tag == nsGkAtoms::option ||
>          tag == nsGkAtoms::optgroup) {

I'd say use Tag() directly since it seems no benefits to have a local for that.

Comment 4

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #3)
> Comment on attachment 619367 [details] [diff] [review]
> don't keep a ref to an atom when we don't need to
> 
> Review of attachment 619367 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/src/html/nsHTMLSelectAccessible.cpp
> @@ +190,2 @@
> >      if (tag == nsGkAtoms::option ||
> >          tag == nsGkAtoms::optgroup) {
> 
> I'd say use Tag() directly since it seems no benefits to have a local for
> that.

well, maybe a little nicer to ahve local since its shorter I can merge the two lines of the if onto 1 this way.

Comment 5

[alexander :surkov (:asurkov)]

ok, up to you, just iirc they said to avoid locals for inlines.

Comment 6

[Trevor Saunders (:tbsaunde)]

landed https://hg.mozilla.org/integration/mozilla-inbound/rev/90e9ed44fc45

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/90e9ed44fc45