Last Comment Bug 657210 - Outparamdel nsEditor::GetPresShell
: Outparamdel nsEditor::GetPresShell
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: Editor (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla6
Assigned To: :Ms2ger
:
Mentors:
Depends on:
Blocks: deCOM
  Show dependency treegraph
 
Reported: 2011-05-15 05:12 PDT by :Ms2ger
Modified: 2011-07-27 04:35 PDT (History)
4 users (show)
Ms2ger: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (24.16 KB, patch)
2011-05-15 05:12 PDT, :Ms2ger
ehsan: review+
Details | Diff | Review

Description :Ms2ger 2011-05-15 05:12:22 PDT
Created attachment 532511 [details] [diff] [review]
Patch v1

Barely any consumer checks the nsresult return value. I hope you like.
Comment 1 Kyle Huey [:khuey] (khuey@mozilla.com) 2011-05-15 10:30:58 PDT
Do we really need to addref on this method?
Comment 2 :Ehsan Akhgari (out sick) 2011-05-15 16:30:38 PDT
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 :Ehsan Akhgari (out sick) 2011-05-15 16:32:32 PDT
(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 :Ehsan Akhgari (out sick) 2011-05-15 16:33:33 PDT
(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 6 :Ms2ger 2011-05-23 12:34:38 PDT
(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] 2011-07-27 04:35:08 PDT
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0b3

Note You need to log in before you can comment on or make changes to this bug.