The default bug view has changed. See this FAQ.

Content process xpcshell doesn't print newlines




6 years ago
6 years ago


(Reporter: jdm, Assigned: jdm)


Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox5 fixed, firefox6 fixed)



(1 attachment)



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


6 years ago
Assignee: nobody → josh

Comment 1

6 years ago
Created attachment 522383 [details] [diff] [review]
Make content process xpcshell print newlines.
Attachment #522383 - Flags: review?(jorendorff)
Comment on attachment 522383 [details] [diff] [review]
Make content process xpcshell print newlines.


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.
Attachment #522383 - Flags: review?(jorendorff) → review-
I think we should merge xpcshell and IPCShell now that we require libXUL+IPC.
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.
Attachment #522383 - Flags: review- → review?(jorendorff)
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 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?
Attachment #522383 - Flags: review?(jorendorff) → review+
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

6 years ago
I've kicked off a try build:

Comment 9

6 years ago
Try reports all green. What are you seeing, Jason?
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.
Keywords: checkin-needed
Flags: in-testsuite?
Keywords: checkin-needed
OS: Linux → All
Hardware: x86 → All
Whiteboard: [fixed in cedar]
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Target Milestone: --- → mozilla7
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.
Attachment #522383 - Flags: approval-mozilla-beta?
Attachment #522383 - Flags: approval-mozilla-aurora?
Duplicate of this bug: 658758
Attachment #522383 - Flags: approval-mozilla-beta?
Attachment #522383 - Flags: approval-mozilla-beta+
Attachment #522383 - Flags: approval-mozilla-aurora?
Attachment #522383 - Flags: approval-mozilla-aurora+
Pushed to Beta for Firefox 5 and Aurora for Firefox 6:
status-firefox5: --- → fixed
status-firefox6: --- → fixed
Target Milestone: mozilla7 → mozilla5
This accidentally landed on a beta relbranch.  Backed out of the relbranch and re-landed on the default branch:
You need to log in before you can comment on or make changes to this bug.