Outparamdel nsEditor::GetPresShell

VERIFIED FIXED in mozilla6

Status

()

Core
Editor
--
minor
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

(Blocks: 1 bug)

Trunk
mozilla6
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Created attachment 532511 [details] [diff] [review]
Patch v1

Barely any consumer checks the nsresult return value. I hope you like.
Attachment #532511 - Flags: review?(ehsan)
Flags: in-testsuite-
Do we really need to addref on this method?
Comment on attachment 532511 [details] [diff] [review]
Patch v1

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

::: editor/libeditor/base/nsEditor.cpp
@@ +537,5 @@
>    NS_PRECONDITION(mDocWeak, "bad state, null mDocWeak");
>    nsCOMPtr<nsIDocument> doc = do_QueryReferent(mDocWeak);
> +  NS_ENSURE_TRUE(doc, NULL);
> +  nsRefPtr<nsIPresShell> ps = doc->GetShell();
> +  return ps.forget();

Can't you just say:

return doc->GetShell()
Attachment #532511 - Flags: review?(ehsan) → review+
(In reply to comment #2)
> ::: editor/libeditor/base/nsEditor.cpp
> @@ +537,5 @@
> >    NS_PRECONDITION(mDocWeak, "bad state, null mDocWeak");
> >    nsCOMPtr<nsIDocument> doc = do_QueryReferent(mDocWeak);
> > +  NS_ENSURE_TRUE(doc, NULL);
> > +  nsRefPtr<nsIPresShell> ps = doc->GetShell();
> > +  return ps.forget();
> 
> Can't you just say:
> 
> return doc->GetShell()

Well, of course you can't! :D

Can you just use nsCOMPtr?  r=me with that.
(In reply to comment #1)
> Do we really need to addref on this method?

It might be needed in some of the cases.  We can investigate the possibility of avoiding that in another bug.
(Assignee)

Comment 5

6 years ago
http://hg.mozilla.org/mozilla-central/rev/6a76f02f19e1
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
(Assignee)

Comment 6

6 years ago
(In reply to comment #3)
> Can you just use nsCOMPtr?  r=me with that.

And because I forgot you said this:

http://hg.mozilla.org/mozilla-central/rev/5e204a657cac

Comment 7

6 years ago
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0b3
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.