645532 - Content process xpcshell doesn't print newlines

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Josh Matthews [:jdm]]

See bug 613516.  The same fix is needed in XPCShellEnvironment.cpp.

Comment 1

[Josh Matthews [:jdm]]

Attachment: Make content process xpcshell print newlines.

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 522383 [details] [diff] [review]
Make content process xpcshell print newlines.

Yuck!

I'll reconsider if this is super-urgent, but really the only sane fix here is to common up the code, right? It can't be that hard. Worst case, we put this stuff in js/src/jsshelljunk.h and include it from both places.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

I think we should merge xpcshell and IPCShell now that we require libXUL+IPC.

Comment 4

[Jason Duell]

Comment on attachment 522383 [details] [diff] [review]
Make content process xpcshell print newlines.

> I'll reconsider if this is super-urgent

Please reconsider immediately.  This bug has caused any xpcshell failures for e10s to be silently ignored for the last two months at least (see bug 658758)

Note that the equivalent behavior (xpcshell failures being silently ignored) for non-e10s was considered urgent enough to close the tree and be considered a release blocker (see bug 613516).  We're about to branch for aurora, and while my (much hackier than this) patch for 658758 seems to show that no failures are actually being masked at present, that could change between now and Monday at midnight.

Please +r and land this ASAP.

Comment 5

[Jason Duell]

Clanging the bell a little louder.  I really don't think it's kosher for JS to be breaking all xpcshell tests for e10s for months at a time.  Redesign and refactor code all you like, but not while the tree burns (or rather, might be burning and we'd never know).

If any of the JS peers I've cc'd can approve this teensy patch, we can move along.

It'd be nice to get approval to land on Aurora too.  What flag needs to be set to get that?

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 522383 [details] [diff] [review]
Make content process xpcshell print newlines.

Ok. Let's get this in. There's no reason for tests to be perma-orange over this. Do we have a bug tracking the work to consolidate the three (!) shells?

Comment 7

[Jason Duell]

Wait!  Don't land this--for some reason it appears to break some *non*-e10s tests.   I'll poke around and see if I can figure out why.

Comment 8

[Josh Matthews [:jdm]]

I've kicked off a try build: http://tbpl.mozilla.org/?tree=Try&pusher=[e-mail]&rev=594f29f1345a

Comment 9

[Josh Matthews [:jdm]]

Try reports all green. What are you seeing, Jason?

Comment 10

[Jason Duell]

Hmm, weird.  Whatever I saw must have been something transient--everything works fine for me (all the netwerk tests are fine, except test_bug651100.js, which fails both with and w/o the patch.

Comment 11

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/964bfab70f87

Comment 12

[Jason Duell]

Comment on attachment 522383 [details] [diff] [review]
Make content process xpcshell print newlines.

Nominating this NPOTB patch for Aurora/beta:  without it e10s xpcshell tests all always report as successful, even when they fail.  The breakage goes back to at least 2010-11-19 (see bug 613516 comment 0), so it's in both aurora/beta at the moment.

Comment 14

[Matt Brubeck (:mbrubeck)]

Pushed to Beta for Firefox 5 and Aurora for Firefox 6:
http://hg.mozilla.org/releases/mozilla-beta/rev/08a50378d37c
http://hg.mozilla.org/releases/mozilla-aurora/rev/c86edc448a83

Comment 15

[Matt Brubeck (:mbrubeck)]

This accidentally landed on a beta relbranch.  Backed out of the relbranch and re-landed on the default branch:
http://hg.mozilla.org/releases/mozilla-beta/rev/b950bfa8ddc5
http://hg.mozilla.org/releases/mozilla-beta/rev/5f7925d4bb2b