1047403 - Printing std:stringstream requires a bunch of duplicated goop for android

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Bug 1027496 buffered various debug output into std::stringstream before dumping it to stderr/logcat. Bug 1030245 fixed the issue where the output was getting truncated on android. However this required a bunch of duplicated code which is only really needed on android. We can refactor this to do better.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

What I had in mind. Still building/testing this.

Comment 2

[Benoit Girard (:BenWa)]

Comment on attachment 8466201 [details] [diff] [review]
Patch

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

::: xpcom/glue/nsDebug.h
@@ +13,5 @@
>  #include "nsXPCOM.h"
>  #include "mozilla/Assertions.h"
>  #include "mozilla/Likely.h"
>  #include <stdarg.h>
> +#include <sstream>

stringstream should be forward declared if possible, nsDebug.h is a common include. I think we can use '#include <iosfwd>' for this?

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Good point. I moved the sstream include to nsCRTGlue.cpp and used iosfwd in nsDebug.h.

Build-only try push:

https://tbpl.mozilla.org/?tree=Try&rev=09479402c191
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=09479402c191

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Doesn't seem to like iosfwd that much.

New try with the original patch: https://tbpl.mozilla.org/?tree=Try&rev=41153b66c218

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8466201 [details] [diff] [review]
Patch

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

I would really like to see <iosfwd> work, and your current try push seems to be blowing up.  Can you investigate?

Comment 6

[Kartikaya Gupta (email:[e-mail])]

My local linux build also will not find <iosfwd> for some reason. Although there are definitely files including it, because if I copy the bits of iosfwd that I need into nsDebug.h, I get a duplicate definition error and the compiler claims iosfwd was already included.

Also the try push shows OS X errors which will likely need a fix such as pointed to at https://bugzilla.mozilla.org/show_bug.cgi?id=965340#c9

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Latest failed attempt: https://tbpl.mozilla.org/?tree=Try&rev=ec691928ba1b

Seems like STL requires infallible new, and xpcom/glue uses fallible new? At least in the context of the "sample program". I might just move this to LayersLogging.cpp or somewhere not so stringent with obscure requirements.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

I moved it to LayersLogging since I got too frustrated with trying to put it in nsDebug.h. This works just fine:

https://tbpl.mozilla.org/?tree=Try&rev=45670c6023b3

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ac94341843e2

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ac94341843e2