1407458 - Use nsString for Observation filenames

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Nicholas Nethercote [inactive]]

This avoids some char16_t*/nsString mismatches.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Use nsString for Observation filenames

Comment 2

[Eric Rahm [:erahm]]

Comment on attachment 8917202 [details] [diff] [review]
Use nsString for Observation filenames

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

Overall looks good. Can you change the commit message? Something like "avoid unnecessary string copies" instead of "mismatches" would be clearer. AFAICT you need to do `PoisonIOInterposerWin` [1] as well. Just return an `nsString&` seems clear to me, but I don't feel too strongly about that.

[1] http://searchfox.org/mozilla-central/rev/ed1d5223adcdc07e9a2589ee20f4e5ee91b4df10/xpcom/build/PoisonIOInterposerWin.cpp#198-216

::: xpcom/build/IOInterposer.h
@@ +95,5 @@
>       */
>      const char* Reference() const { return mReference; }
>  
> +    /** Request filename associated with the I/O operation, empty if unknown */
> +    virtual void Filename(nsAString& aString) { aString.Truncate(); }

It might be cleaner to just return a ref. In this case you could use EmptyString.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Use nsString for Observation filenames

I added the Windows support. I left the outparam because that's the standard
way of returning nsString values.

Comment 4

[Eric Rahm [:erahm]]

(In reply to Nicholas Nethercote [:njn] from comment #3)
> Created attachment 8919684 [details] [diff] [review]
> Use nsString for Observation filenames
> 
> I added the Windows support. I left the outparam because that's the standard
> way of returning nsString values.

I'm not convinced that's true, but meh.

Comment 5

[Nicholas Nethercote [inactive]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/380afb457c9a45c977c1cace6263c9284d6bf649
Bug 1407458 - Use nsString for Observation filenames. r=erahm.

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/380afb457c9a