rm some unneeded nsCOMPtrs

RESOLVED FIXED in mozilla15

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: tbsaunde, Assigned: tbsaunde)

Tracking

(Blocks: 1 bug)

unspecified
mozilla15
x86_64
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
I'm pretty sure we don't need owning refs in any of these places.
(Assignee)

Comment 1

5 years ago
Created attachment 619366 [details] [diff] [review]
don't use nsCOMPtrs with the presShell unless we need to
Attachment #619366 - Flags: review?(surkov.alexander)
(Assignee)

Comment 2

5 years ago
Created attachment 619367 [details] [diff] [review]
don't keep a ref to an atom when we don't need to
Attachment #619367 - Flags: review?(surkov.alexander)
Summary: rm some unneeded nsnsCOMPtrs → rm some unneeded nsCOMPtrs

Updated

5 years ago
Attachment #619366 - Flags: review?(surkov.alexander) → review+

Comment 3

5 years ago
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.
Attachment #619367 - Flags: review?(surkov.alexander) → review+
(Assignee)

Comment 4

5 years ago
(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

5 years ago
ok, up to you, just iirc they said to avoid locals for inlines.
(Assignee)

Comment 6

5 years ago
landed https://hg.mozilla.org/integration/mozilla-inbound/rev/90e9ed44fc45
(Assignee)

Updated

5 years ago
Target Milestone: --- → mozilla15

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/90e9ed44fc45
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.