267269 - run() method of nsIProcess stopped accepting arguments for the new process

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[neomjp]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20041026 Firefox/1.0RC1
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.7.3) Gecko/20041026 Firefox/1.0RC1

this used to work for Firefox 0.10 but does not work for Firefox 1.0RC1.

The cause is from the following checkin

mozilla/xpcom/threads/nsProcessCommon.cpp
1.11 jst%mozilla.jstenback.com Fixing bug 263429. Making nsProcessCommon::Run()
work on Mac OS X. 

http://lxr.mozilla.org/aviarybranch/source/xpcom/threads/nsProcessCommon.cpp#201
201 #if defined(XP_MACOSX)
202     // You can't pass arguments to mac apps, tell the caller that it
203     // just aint going to work.
204     if (count) {
205         return NS_ERROR_INVALID_ARG;
206     }
207 #else

This is a regression, but a minor loss of a function that is not used (I
suppose) in the official releases. setting severity to minor.

Reproducible: Always
Steps to Reproduce:
1. install Venkman, and Nvu.app. (do not start Nvu.app yet)
2. from the Venkman interactive session run,
 var appf =
Components.classes['@mozilla.org/file/local;1'].createInstance(Components.interfaces.nsILocalFile);
appf.initWithPath("/Applications/Nvu0.50.app/Contents/MacOS/nvu-bin");
var proce =
Components.classes['@mozilla.org/process/util;1'].createInstance(Components.interfaces.nsIProcess);
proce.init(appf);
proce.run(false, [], 0);
3. You see that Nvu.app starts up.
4. quit Nvu.app.
5. Again from the Venkman interactive session run,
proce.run(false, ["-edit", "file:///Users/myusername/an_existing_file.html"], 2);
Actual Results:  
In Firefox 1.0RC1 you get
[Exception... "Component returned failure code: 0x80070057
(NS_ERROR_ILLEGAL_VALUE) [nsIProcess.run]" nsresult: "0x80070057
(NS_ERROR_ILLEGAL_VALUE)" location: "JS frame ::
chrome://venkman/content/venkman-eval.js :: con_eval :: line 46" data: no]

Expected Results:  
Nvu.app should be started up with the an_existing_file.html page, as it used to
do for Firefox 0.10

Comment 1

[Boris Zbarsky [:bzbarsky]]

Reporter, the code removed in bug 263429 didn't pass the args to the new process
as far as I can tell... it just silently ignored them (where the new code throws).

Comment 2

[Johnny Stenback (:jst)]

Actually, the code used to use NSPR's process creation code, and that took
arguments etc, but it didn't work for starting normal mac apps (things like the
shockwave plugin installer), it just wouldn't launch the apps. I don't know what
the difference is here. The old mac code that we're now using to launch
processes, which works fine with the shockwave installer erc, can't deal with
arguments, that's why I made it throw an error in stead of simply just ignoring
them...

Comment 3

[neomjp]

bzbarsky and jst, thank you for you attention.

Yes, the previous code used PR_CreateProcess[Detached]() from
nsprpub/pr/src/misc/prinit.c  to handle args. It worked only for executables
that could be run from bash. /Volumes/Macromedia Shockwave Player/Shockwave
Installer (I guess this is the file jst meant.) cannot be run from bash, and
that is probably why it could not be run directly using the previous code. But
if you use the Mac OS X-specific command /usr/bin/open to indirectly open the
installer, it works.

appf.initWithPath("/usr/bin/open");proce.init(appf);
proce.run(false, ["/Volumes/Macromedia Shockwave Player/Shockwave Installer"], 1);

or

proce.run(false, ["-a","/Volumes/Macromedia Shockwave Player/Shockwave
Installer"], 2);

Is it possible that you use this workaround instead to write the xpi wrapper,
(and restore PR_CreateProcess[Detached]() for Mac OS X)?

Comment 4

[Johnny Stenback (:jst)]

Maybe we could change the code such that if arguments are passed to Run(), we'll
use the NSPR code we used to use, and if no arguments are passed, we'll use what
we use today?

Comment 5

[Boris Zbarsky [:bzbarsky]]

That sounds reasonable to me.. ccing some Mac people for comment.

Comment 6

[neomjp]

Attachment: use NSPR's process creation code if arguments are passed.

Sounds reasonable to me, too.

Comment 7

[Mike Pinkerton (not reading bugmail)]

sounds ok to me

Comment 8

[Johnny Stenback (:jst)]

Attachment: Same as above w/o duplicating code.

Comment 9

[Johnny Stenback (:jst)]

Attachment: diff -w of the above for review.

Comment 10

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 165473 [details] [diff] [review]
diff -w of the above for review.

r=pink

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 165473 [details] [diff] [review]
diff -w of the above for review.

sr=bzbarsky

Comment 12

[Johnny Stenback (:jst)]

Fixed.