Last Comment Bug 645532 - Content process xpcshell doesn't print newlines
: Content process xpcshell doesn't print newlines
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: IPC (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla5
Assigned To: Josh Matthews [:jdm] (on vacation until Dec 5)
:
: Bill McCloskey (:billm)
Mentors:
: 658758 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-27 12:19 PDT by Josh Matthews [:jdm] (on vacation until Dec 5)
Modified: 2011-06-09 11:17 PDT (History)
10 users (show)
mounir: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed


Attachments
Make content process xpcshell print newlines. (920 bytes, patch)
2011-03-28 09:51 PDT, Josh Matthews [:jdm] (on vacation until Dec 5)
mrbkap: review+
gavin.sharp: approval‑mozilla‑aurora+
gavin.sharp: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Josh Matthews [:jdm] (on vacation until Dec 5) 2011-03-27 12:19:44 PDT
See bug 613516.  The same fix is needed in XPCShellEnvironment.cpp.
Comment 1 Josh Matthews [:jdm] (on vacation until Dec 5) 2011-03-28 09:51:00 PDT
Created attachment 522383 [details] [diff] [review]
Make content process xpcshell print newlines.
Comment 2 Jason Orendorff [:jorendorff] 2011-03-28 12:48:10 PDT
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!) 2011-03-28 13:26:49 PDT
I think we should merge xpcshell and IPCShell now that we require libXUL+IPC.
Comment 4 Jason Duell [:jduell] (needinfo me) 2011-05-22 00:08:20 PDT
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 [:jduell] (needinfo me) 2011-05-30 08:29:43 PDT
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) 2011-05-31 03:05:47 PDT
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 [:jduell] (needinfo me) 2011-06-01 04:19:03 PDT
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] (on vacation until Dec 5) 2011-06-01 06:04:30 PDT
I've kicked off a try build: http://tbpl.mozilla.org/?tree=Try&pusher=josh@joshmatthews.net&rev=594f29f1345a
Comment 9 Josh Matthews [:jdm] (on vacation until Dec 5) 2011-06-01 10:35:41 PDT
Try reports all green. What are you seeing, Jason?
Comment 10 Jason Duell [:jduell] (needinfo me) 2011-06-01 14:34:26 PDT
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) 2011-06-03 03:10:02 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/964bfab70f87
Comment 12 Jason Duell [:jduell] (needinfo me) 2011-06-07 10:42:28 PDT
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 13 Jason Duell [:jduell] (needinfo me) 2011-06-07 10:43:47 PDT
*** Bug 658758 has been marked as a duplicate of this bug. ***
Comment 14 Matt Brubeck (:mbrubeck) 2011-06-09 10:49:27 PDT
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) 2011-06-09 11:17:11 PDT
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

Note You need to log in before you can comment on or make changes to this bug.