1015430 - Fix more XPCOM constructors to clarify whether they should be explicit

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Comment 1

[(no longer active)]

Attachment: Fix more XPCOM constructors to clarify whether they should be explicit

Comment 2

[Nathan Froyd [:froydnj]]

Comment on attachment 8428003 [details] [diff] [review]
Fix more XPCOM constructors to clarify whether they should be explicit

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

::: ipc/glue/BackgroundImpl.cpp
@@ +926,5 @@
>  
>      MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(runnable)));
>    }
>  
> +  return already_AddRefed<ContentParent>(actor->mContent.get());

This seems really dodgy, but I guess it's what the code already does, so...

::: xpcom/string/public/nsTString.h
@@ +58,1 @@
>    nsTString_CharT( const substring_tuple_type& tuple )

Does MOZ_IMPLICIT nsTString_CharT(...) overflow a line or something?  Can you put the MOZ_IMPLICIT on the same line if not?

@@ +529,1 @@
>    nsTAutoString_CharT( const substring_tuple_type& tuple )

Here too.

Comment 3

[(no longer active)]

(In reply to Nathan Froyd (:froydnj) from comment #2)
> Comment on attachment 8428003 [details] [diff] [review]
> Fix more XPCOM constructors to clarify whether they should be explicit
> 
> Review of attachment 8428003 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: ipc/glue/BackgroundImpl.cpp
> @@ +926,5 @@
> >  
> >      MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(runnable)));
> >    }
> >  
> > +  return already_AddRefed<ContentParent>(actor->mContent.get());
> 
> This seems really dodgy, but I guess it's what the code already does, so...

See bug 1015182.  Surprisingly, this is intentional!

> ::: xpcom/string/public/nsTString.h
> @@ +58,1 @@
> >    nsTString_CharT( const substring_tuple_type& tuple )
> 
> Does MOZ_IMPLICIT nsTString_CharT(...) overflow a line or something?  Can
> you put the MOZ_IMPLICIT on the same line if not?

Sure.

Comment 4

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/28d085d43fa6

Comment 5

[[e-mail]]

(In reply to Nathan Froyd from comment #2)
> > +  return already_AddRefed<ContentParent>(actor->mContent.get());
> 
> This seems really dodgy

It is indeed dodgy, it should say dont_AddRef(actor->mContent.get());

Comment 6

[[e-mail]]

(partly for the free type deduction and partly for the self-documenting function name)

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/28d085d43fa6