1162293 - Remove usage of |#ifdef PR_LOGGING| from widget

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Eric Rahm [:erahm]]

In bug 1161238 we plan on removing |--disable-logging| which makes |#ifdef PR_LOGGING| redundant.

Comment 1

[Eric Rahm [:erahm]]

Attachment: Part 1: Remove instances of #ifdef PR_LOGGING

PR_LOGGING is now always defined, we can remove #ifdefs checking for it.

Comment 2

[Eric Rahm [:erahm]]

Attachment: Part 2: Fix string leak in logging code

If logging is enabled the result from |ToNewCString| will be leaked. Switched
to |NS_LossyConvertUTF16toASCII| instead.

Comment 3

[Eric Rahm [:erahm]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f2f2fc3f5279

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 8602381 [details] [diff] [review]
Part 2: Fix string leak in logging code

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

Does this choke if we use NSConvertUTF16toUTF8().get()?  I should think that attempting not to lose data here is preferable.  r=me assuming that works.

Comment 5

[Eric Rahm [:erahm]]

(In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #4)
> Comment on attachment 8602381 [details] [diff] [review]
> Part 2: Fix string leak in logging code
> 
> Review of attachment 8602381 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Does this choke if we use NSConvertUTF16toUTF8().get()?  I should think that
> attempting not to lose data here is preferable.  r=me assuming that works.

I'm maintaining parity w/ the previous implementation, |ToNewCString| does a lossy convert, whereas |ToNewUTF8String| would have mapped to |NS_ConvertUTF16toUTF8|.

Comment 6

[Nathan Froyd [:froydnj]]

(In reply to Eric Rahm [:erahm] from comment #5)
> (In reply to Nathan Froyd [:froydnj] [:nfroyd] from comment #4)
> > Does this choke if we use NSConvertUTF16toUTF8().get()?  I should think that
> > attempting not to lose data here is preferable.  r=me assuming that works.
> 
> I'm maintaining parity w/ the previous implementation, |ToNewCString| does a
> lossy convert, whereas |ToNewUTF8String| would have mapped to
> |NS_ConvertUTF16toUTF8|.

Fair point!  r=me unconditionally, then. :)

Comment 7

[Eric Rahm [:erahm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/830ce4e36b47
https://hg.mozilla.org/integration/mozilla-inbound/rev/93eded78def9

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/830ce4e36b47
https://hg.mozilla.org/mozilla-central/rev/93eded78def9