657210 - Outparamdel nsEditor::GetPresShell

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Barely any consumer checks the nsresult return value. I hope you like.

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Do we really need to addref on this method?

Comment 2

[(no longer active)]

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()

Comment 3

[(no longer active)]

(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.

Comment 4

[(no longer active)]

(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.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/mozilla-central/rev/6a76f02f19e1

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(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

[Vlad [QA]]

Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0b3