66735 - Need to implement nsIProcess (blocking feature) on the Mac

Status: VERIFIED FIXED

(Core :: XPCOM, defect, P2)

Description

[dbragg]

The nsIProcess implementation on the Mac(xpcom/threads/nsProcessMac.cpp)
 currently ignores the blocking boolean.  Current behavior: executes another
process and immediately returns.

Assigning to dougt as module owner and per discussion with him.

Comment 1

[Daniel Veditz [:dveditz]]

Sorry, can't Future. This feature is required to be able to successfully 
install plugins via XPInstall without requiring a browser restart, which is in 
turn required by at least two major Mozilla clients.  If you can't/won't do 
this please bounce it up through your manager and let them fight it out who 
should own this.

Comment 2

[Doug Turner (:dougt)]

dan, if you need it, you implement it.  I don't remember why this was not
implmented in the first place.  

Comment 3

[dbragg]

I'm afraid I don't understand you're last comment Doug.  We had a discussion
about this before I created the bug and assigned it to you.  At the time you
were willing to do the implementation.  In fact, you only need to do 1/2 the
implementation because the non-blocking case is already in nsProcessMac.cpp.
All we need is the blocking case.

Comment 4

[Daniel Veditz [:dveditz]]

We desperately need a mac person to help us out. sdagley pointed us toward
watching based on the launched process's serial number.

Comment 5

[Samir Gehani]

I'll implement this and consult sdagley if needed.  We may need this sooner than
anticipated for a third party installer that may get added.

Comment 6

[Samir Gehani]

Attachment: Loop with a sleep until the process is no longer the process manager's list if blocking == true.

Comment 7

[Samir Gehani]

sdagley, please review.
sfraser, please super review.

Thanks.

Comment 8

[Simon Fraser [no longer active]]

Looks good, but does it work?
Minor changes:
1. I think 50ms is too short a sleep time here, since (I guess) the app that
   runs is going to take 1 second or more. Try 200ms, and tune down if it
   feels too sluggish.

2. I'd like to see an assertion hit if this is called on the main thread.

Comment 9

[Samir Gehani]

Yes, Simon, this works.  I tested it through XPInstall.

I'll change the sleep value to 200ms.  Although the app didn't feel sluggish
with 50ms too on my G3 400MHz machine.  I'll go with 200ms to accomodate lower
end machines and maybe QA can advise on those lower end configs if they feel the
app is visibly sluggish.

Regarding the assertion: I looked around and consulted dveditz but there is no
low cost way to find whether we are in fact the UI thread that we could find.
If you know of a way to do this please advise.  Besides, dveditz's point was
that this would be pretty obvious when someone tested this if it was called on
the UI thread since the app wouldn't launch.

Comment 10

[Samir Gehani]

Attachment: Revised patch with 200ms sleep.

Comment 11

[Daniel Veditz [:dveditz]]

The app would launch, but Mozilla would hang until it's done. Sorry if I was 
unclear. I still think it'd be pretty obvious to anyone who tried to do that, 
but if anyone knows a cheap way to figure out if you're on the UI thread an 
assertion would be nice.

Comment 12

[Samir Gehani]

Dan,
Actually on the Mac the app would not launch because WaitNextEvent() would not
be called since the main event loop would never be reached after we sit in the
sleep loop on the UI thread itself.  It is required that either of two Event
Manager routines be called: WaitNextEvent() or EventAvail() for an app to launch
after calling the Process Manager's LaunchApplication routine.  I guess I was
the one who was unclear.

Comment 13

[Samir Gehani]

Fix checked in.

Comment 14

[Jimmy Lee]

Build: 2001-04-03-09-trunk(MAC)

This is now working for me.  A couple of blocking test cases for Mac are 
http://jimbob/jars/a_exe_2_mac_block_true.xpi and 
http://jimbob/jars/a_filop_exe_2_mac_block_t.xpi

When blocking is set to false, it behaves as expected.

Changing QA Contact to me.  Marking Verified!