Bug 483626: nsIProcess.isRunning doesn't fail correctly and should be boolean. r=bsmedberg
authorDave Townsend <dtownsend@oxymoronical.com>
Wed, 18 Mar 2009 10:59:32 +0000
changeset 26296 5092d98e31ad16bb45a8920539840c262cb50566
parent 26295 1a9504b2373bcad3e1ecae802395ca43f5503398
child 26297 1f3d5e332a4429052cc629b6cb704bdd57f00c15
push idunknown
push userunknown
push dateunknown
reviewersbsmedberg
bugs483626
milestone1.9.2a1pre
Bug 483626: nsIProcess.isRunning doesn't fail correctly and should be boolean. r=bsmedberg
xpcom/tests/unit/test_nsIProcess.js
xpcom/threads/nsIProcess.idl
xpcom/threads/nsProcess.h
xpcom/threads/nsProcessCommon.cpp
--- a/xpcom/tests/unit/test_nsIProcess.js
+++ b/xpcom/tests/unit/test_nsIProcess.js
@@ -64,79 +64,78 @@ function test_kill()
   var file = Components.classes["@mozilla.org/file/local;1"]
                        .createInstance(Components.interfaces.nsILocalFile);
   file.initWithPath(testapp);
  
   var process = Components.classes["@mozilla.org/process/util;1"]
                           .createInstance(Components.interfaces.nsIProcess);
   process.init(file);
   
+  do_check_false(process.isRunning);
+
+  try {
+    process.kill();
+    do_throw("Attempting to kill a not-running process should throw");
+  }
+  catch (e) { }
+
   process.run(false, [], 0);
 
-  var rv = process.isRunning;
-
-  if (!rv) {    
-    return false;
-  }
+  do_check_true(process.isRunning);
 
   process.kill();
 
-  rv = process.isRunning;
+  do_check_false(process.isRunning);
 
-  if (rv){
-    return false;
+  try {
+    process.kill();
+    do_throw("Attempting to kill a not-running process should throw");
   }
-  return true;
-
+  catch (e) { }
 }
 
 // test if we can get an exit value from an application that is
 // guaranteed to return an exit value of 42
 function test_quick()
 {
   var testapp = filePrefix + "TestQuickReturn" + fileSuffix;
   
   var file = Components.classes["@mozilla.org/file/local;1"]
-  .createInstance(Components.interfaces.nsILocalFile);
+                       .createInstance(Components.interfaces.nsILocalFile);
   file.initWithPath(testapp);
   
   var process = Components.classes["@mozilla.org/process/util;1"]
-  .createInstance(Components.interfaces.nsIProcess);
+                          .createInstance(Components.interfaces.nsIProcess);
   process.init(file);
   
   // to get an exit value it must be a blocking process
   process.run(true, [], 0);
-  
-  if (process.exitValue != 42) {
-    return false;
-  }
-  return true;
+
+  do_check_eq(process.exitValue, 42);
 }
 
 // test if an argument can be successfully passed to an application
 // that will return -1 if "mozilla" is not the first argument
 function test_arguments()
 {
   var testapp = filePrefix + "TestArguments" + fileSuffix;
   
   var file = Components.classes["@mozilla.org/file/local;1"]
-  .createInstance(Components.interfaces.nsILocalFile);
+                       .createInstance(Components.interfaces.nsILocalFile);
   file.initWithPath(testapp);
   
   var process = Components.classes["@mozilla.org/process/util;1"]
-  .createInstance(Components.interfaces.nsIProcess);
+                          .createInstance(Components.interfaces.nsIProcess);
   process.init(file);
   
   var args= ["mozilla"];
   
   process.run(true, args, args.length);
   
-  if (process.exitValue) {
-    return false;
-  }
-  return true;
+  // exit codes actually seem to be unsigned bytes...
+  do_check_neq(process.exitValue, 255);
 }
 
 function run_test() {
-  do_check_true(test_kill());
-  do_check_true(test_quick());
-  do_check_true(test_arguments());
+  test_kill();
+  test_quick();
+  test_arguments();
 }
--- a/xpcom/threads/nsIProcess.idl
+++ b/xpcom/threads/nsIProcess.idl
@@ -17,16 +17,16 @@ interface nsIProcess : nsISupports
          * @param count The length of the args array */
         void run(in boolean blocking, [array, size_is(count)] in string args, in unsigned long count);
 
 	readonly attribute nsIFile location;
 	readonly attribute unsigned long pid;
 	readonly attribute string processName;
 	readonly attribute unsigned long processSignature;
 	readonly attribute long exitValue;
-	readonly attribute unsigned long isRunning;
+	readonly attribute boolean isRunning;
 };
 
 %{C++
 
 #define NS_PROCESS_CONTRACTID "@mozilla.org/process/util;1"
 #define NS_PROCESS_CLASSNAME "Process Specification"
 %}
--- a/xpcom/threads/nsProcess.h
+++ b/xpcom/threads/nsProcess.h
@@ -66,16 +66,17 @@ public:
   nsProcess();
 
 private:
   ~nsProcess();
 
   nsCOMPtr<nsIFile> mExecutable;
   PRInt32 mExitValue;
   nsCString mTargetPath;
-  PRProcess *mProcess;
 
 #if defined(PROCESSMODEL_WINAPI) 
   PROCESS_INFORMATION procInfo;
+#else
+  PRProcess *mProcess;
 #endif
 };
 
 #endif
--- a/xpcom/threads/nsProcessCommon.cpp
+++ b/xpcom/threads/nsProcessCommon.cpp
@@ -68,21 +68,22 @@
 
 //-------------------------------------------------------------------//
 // nsIProcess implementation
 //-------------------------------------------------------------------//
 NS_IMPL_ISUPPORTS1(nsProcess, nsIProcess)
 
 //Constructor
 nsProcess::nsProcess()
-    : mExitValue(-1),
-      mProcess(nsnull)
+    : mExitValue(-1)
 {
 #if defined(PROCESSMODEL_WINAPI)
     procInfo.dwProcessId = nsnull;
+#else
+    mProcess = nsnull;
 #endif
 }
 
 //Destructor
 nsProcess::~nsProcess()
 {
 #if defined(PROCESSMODEL_WINAPI)
     if (procInfo.dwProcessId) {
@@ -352,34 +353,41 @@ nsProcess::Run(PRBool blocking, const ch
     nsMemory::Free(my_argv);
 
     if (status != PR_SUCCESS)
         return NS_ERROR_FILE_EXECUTION_FAILED;
 
     return NS_OK;
 }
 
-NS_IMETHODIMP nsProcess::GetIsRunning(PRUint32 *aIsRunning)
+NS_IMETHODIMP nsProcess::GetIsRunning(PRBool *aIsRunning)
 {
 #if defined(PROCESSMODEL_WINAPI)
+    if (!procInfo.dwProcessId) {
+        *aIsRunning = PR_FALSE;
+        return NS_OK;
+    }
     DWORD ec = 0;
     BOOL br = GetExitCodeProcess(procInfo.hProcess, &ec);
-    if (!br) {
-        aIsRunning = 0;
-        return NS_OK;
-    }
-    *aIsRunning = (ec == STILL_ACTIVE ? 1 : 0); 
+    if (!br)
+        return NS_ERROR_FAILURE;
+    *aIsRunning = (ec == STILL_ACTIVE ? PR_TRUE : PR_FALSE); 
     return NS_OK;
 #elif defined WINCE
     return NS_ERROR_NOT_IMPLEMENTED;   
 #else
+    if (!mProcess) {
+        *aIsRunning = PR_FALSE;
+        return NS_OK;
+    }
     PRUint32 pid;
-    GetPid(&pid);
+    nsresult rv = GetPid(&pid);
+    NS_ENSURE_SUCCESS(rv, rv);
     if (pid)
-        *aIsRunning = (kill(pid, 0) != -1) ? 1 : 0;
+        *aIsRunning = (kill(pid, 0) != -1) ? PR_TRUE : PR_FALSE;
     return NS_OK;        
 #endif
 }
 
 NS_IMETHODIMP nsProcess::InitWithPid(PRUint32 pid)
 {
     return NS_ERROR_NOT_IMPLEMENTED;
 }
@@ -396,20 +404,19 @@ nsProcess::GetPid(PRUint32 *aPid)
 #if defined(PROCESSMODEL_WINAPI)
     if (!procInfo.dwProcessId)
         return NS_ERROR_FAILURE;
     *aPid = procInfo.dwProcessId;
     return NS_OK;
 #elif defined WINCE
     return NS_ERROR_NOT_IMPLEMENTED;
 #else
-    if (!mProcess) {
-        *aPid = 0;
+    if (!mProcess)
         return NS_ERROR_FAILURE;
-    }
+
     struct MYProcess {
         PRUint32 pid;
     };
     MYProcess* ptrProc = (MYProcess *) mProcess;
     *aPid = ptrProc->pid;
     return NS_OK;
 #endif
 }
@@ -424,32 +431,36 @@ NS_IMETHODIMP
 nsProcess::GetProcessSignature(PRUint32 *aProcessSignature)
 {
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
 NS_IMETHODIMP
 nsProcess::Kill()
 {
-    nsresult rv = NS_OK;
 #if defined(PROCESSMODEL_WINAPI)
-    if ( TerminateProcess(procInfo.hProcess, NULL) == 0 ) {
-        rv = NS_ERROR_FAILURE;
-    }
-    else {
-        CloseHandle( procInfo.hProcess );
-        procInfo.dwProcessId = nsnull;
-    }
+    if (!procInfo.dwProcessId)
+        return NS_ERROR_NOT_INITIALIZED;
+
+    if ( TerminateProcess(procInfo.hProcess, NULL) == 0 )
+        return NS_ERROR_FAILURE;
+
+    CloseHandle( procInfo.hProcess );
+    CloseHandle( procInfo.hThread );
+    procInfo.dwProcessId = nsnull;
 #else
-    if (mProcess)
-        rv = PR_KillProcess(mProcess) == PR_SUCCESS ? NS_OK : NS_ERROR_FAILURE;
-    if (rv == NS_OK)
-        mProcess = nsnull;
+    if (!mProcess)
+        return NS_ERROR_NOT_INITIALIZED;
+
+    if (PR_KillProcess(mProcess) != PR_SUCCESS)
+        return NS_ERROR_FAILURE;
+
+    mProcess = nsnull;
 #endif  
-    return rv;
+    return NS_OK;
 }
 
 NS_IMETHODIMP
 nsProcess::GetExitValue(PRInt32 *aExitValue)
 {
     *aExitValue = mExitValue;
     
     return NS_OK;