fixing bug 204162 - -201 error when doing performInstall. r=dveditz sr=jag, a=sspitzer

xpinstall/src/ScheduledTasks.cpp

@@ -220,18 +220,22 @@ PRInt32 ScheduleFileForDeletion(nsIFile *filename)
 
 
 
-PRInt32 ReplaceFileNow(nsIFile* replacementFile, nsIFile* doomedFile )
+PRInt32 ReplaceFileNow(nsIFile* aReplacementFile, nsIFile* aDoomedFile )
 {
     PRBool flagExists, flagIsEqual;
+    nsCOMPtr<nsIFile> replacementFile;
     nsresult rv;
 
+    // make a clone of aReplacement file so we touch affect callers
+    aReplacementFile->Clone(getter_AddRefs(replacementFile));
+
     // replacement file must exist, doomed file doesn't have to
     replacementFile->Exists(&flagExists);
     if ( !flagExists )
         return nsInstall::DOES_NOT_EXIST;
 
     // don't have to do anything if the files are the same
-    replacementFile->Equals(doomedFile, &flagIsEqual);
+    replacementFile->Equals(aDoomedFile, &flagIsEqual);
     if ( flagIsEqual )
         return nsInstall::SUCCESS;
 
@@ -241,9 +245,8 @@ PRInt32 ReplaceFileNow(nsIFile* replacementFile, nsIFile* doomedFile )
     // first try to rename the doomed file out of the way (if it exists)
     nsCOMPtr<nsIFile>      renamedDoomedFile;
     nsCOMPtr<nsILocalFile> tmpLocalFile;
-    nsCOMPtr<nsIFile> parent;
     
-    doomedFile->Clone(getter_AddRefs(renamedDoomedFile));
+    aDoomedFile->Clone(getter_AddRefs(renamedDoomedFile));
     renamedDoomedFile->Exists(&flagExists);
     if ( flagExists )
     {
@@ -253,49 +256,66 @@ PRInt32 ReplaceFileNow(nsIFile* replacementFile, nsIFile* doomedFile )
         // is despite the fact that the two FSRefs are independent objects. Until
         // the OS X file impl is changed to not use FSRefs, need to do this (see
         // bug 200024).
-        nsCOMPtr<nsILocalFile> doomedFileLocal(do_QueryInterface(doomedFile));
+        nsCOMPtr<nsILocalFile> doomedFileLocal = do_QueryInterface(aDoomedFile);
         nsCAutoString doomedFilePath;
         rv = doomedFileLocal->GetNativePath(doomedFilePath);
         if (NS_FAILED(rv))
             return nsInstall::UNEXPECTED_ERROR;
 #endif
+
         tmpLocalFile = do_QueryInterface(renamedDoomedFile, &rv); // Convert to an nsILocalFile
 
-        //get the leafname so we can convert its extension to .old
+        //get the doomedLeafname so we can convert its extension to .old
-        nsAutoString leafname;
+        nsAutoString doomedLeafname;
         nsCAutoString uniqueLeafName;
-        tmpLocalFile->GetLeafName(leafname);
+        tmpLocalFile->GetLeafName(doomedLeafname);
 
         // do not RFind on the native charset! UTF8 or Unicode are OK
-        PRInt32 extpos = leafname.RFindChar('.');
+        PRInt32 extpos = doomedLeafname.RFindChar('.');
         if (extpos != -1)
         {
             // We found the extension; 
-            leafname.Truncate(extpos + 1); //strip off the old extension
+            doomedLeafname.Truncate(extpos + 1); //strip off the old extension
         }
-        leafname.Append(NS_LITERAL_STRING("old"));
+        doomedLeafname.Append(NS_LITERAL_STRING("old"));
         
-        //Now reset the leafname
+        //Now reset the doomedLeafname
-        tmpLocalFile->SetLeafName(leafname);
+        tmpLocalFile->SetLeafName(doomedLeafname);
         tmpLocalFile->CreateUnique(nsIFile::NORMAL_FILE_TYPE, 0644);
-        tmpLocalFile->GetParent(getter_AddRefs(parent)); //get the parent for later use in MoveTo
+        tmpLocalFile->GetNativeLeafName(uniqueLeafName);//this is the new "unique" doomedLeafname
-        tmpLocalFile->GetNativeLeafName(uniqueLeafName);//this is the new "unique" leafname
 
-        rv = doomedFile->Clone(getter_AddRefs(renamedDoomedFile));// Reset renamedDoomed file so doomedfile isn't 
+        rv = aDoomedFile->Clone(getter_AddRefs(renamedDoomedFile));// Reset renamedDoomed file so aDoomedfile
-                                                                  //   changed during the MoveTo call
+                                                                   // isn't changed during the MoveTo call
-        if (NS_FAILED(rv)) result = nsInstall::UNEXPECTED_ERROR;
-        rv = renamedDoomedFile->MoveToNative(parent, uniqueLeafName);        
         if (NS_FAILED(rv))
-            return nsInstall::UNEXPECTED_ERROR;
+            result = nsInstall::UNEXPECTED_ERROR;
-
+        else
-        // The implementation of MoveToNative() on some platforms (osx and win32) resets
+        {
-        // the object to the 'moved to' filename.  This is incorrect behavior.  This
+            rv = renamedDoomedFile->MoveToNative(nsnull, uniqueLeafName);        
-        // implementation will be fixed in the future.  We need to take into account that
+            if (NS_FAILED(rv))
-        // fix by setting renamedDoomedFile to the filename that it was moved to above.
+            {
-        // See bug 200024.
+                // MoveToNative() failing is OK.  It simply means that the file
-        rv = renamedDoomedFile->SetNativeLeafName(uniqueLeafName);        
+                // was locked in memory and needs to be replaced on browser
-        if (NS_FAILED(rv))
+                // shutdown or system reboot.
-            return nsInstall::UNEXPECTED_ERROR;
+                //
+                // Since renamedDoomedFile->MoveToNative() failed, it created a
+                // 0 byte '-old' file that needs to be cleaned up.
+                tmpLocalFile->Remove(PR_FALSE);
+            }
+            else
+            {
+                // The implementation of MoveToNative() on some platforms (osx and win32) resets
+                // the object to the 'moved to' filename.  This is incorrect behavior.  This
+                // implementation will be fixed in the future.  We need to take into account that
+                // fix by setting renamedDoomedFile to the filename that it was moved to above.
+                // See bug 200024.
+                //
+                // renamedDoomedFile needs to be reset because it's used later on in this
+                // function.
+                rv = renamedDoomedFile->SetNativeLeafName(uniqueLeafName);        
+                if (NS_FAILED(rv))
+                    result = nsInstall::UNEXPECTED_ERROR;
+            }
+        }
 
 #ifdef XP_MACOSX
         rv = doomedFileLocal->InitWithNativePath(doomedFilePath);
@@ -308,56 +328,43 @@ PRInt32 ReplaceFileNow(nsIFile* replacementFile, nsIFile* doomedFile )
     }
 
 
-    // if doomedFile is gone move new file into place
+    // if aDoomedFile is still in the way, give up and return result.
-    doomedFile->Exists(&flagExists);
+    aDoomedFile->Exists(&flagExists);
-    if ( !flagExists )
+    if ( flagExists )
+        return result;
+
+    nsCOMPtr<nsIFile> parentofDoomedFile;
+    nsCAutoString doomedLeafname;
+
+    rv = aDoomedFile->GetParent(getter_AddRefs(parentofDoomedFile));
+    if ( NS_SUCCEEDED(rv) )
+        rv = aDoomedFile->GetNativeLeafName(doomedLeafname);
+    if ( NS_SUCCEEDED(rv) )
     {
-        nsCOMPtr<nsIFile> parentofFinalFile;
+        rv = replacementFile->MoveToNative(parentofDoomedFile, doomedLeafname);
-        nsCOMPtr<nsIFile> parentofReplacementFile;
+        // The implementation of MoveToNative() on some platforms (osx and win32) resets
-        nsCAutoString leafname;
+        // the object to the 'moved to' filename.  This is incorrect behavior.  This
+        // implementation will be fixed in the future.  We need to take into account that
+        // fix by setting replacementFile to the filename that it was moved to above.
+        // See bug 200024.
+        //
+        // However, since replacementFile is a clone of aReplacementFile and is also
+        // not used beyond here, there's no need to set the path+leafname to what
+        // it was MoveToNative()'ed to.
+    }
 
-        doomedFile->GetParent(getter_AddRefs(parentofFinalFile));
+    if (NS_SUCCEEDED(rv))
-        replacementFile->GetParent(getter_AddRefs(parentofReplacementFile));
+    {
-
+        // we replaced the old file OK, now we have to
-        // XXX looks dangerous, the replacement file name may NOT be unique in the
+        // get rid of it if it was renamed out of the way
-        // target directory if we have to move it! Either we should never move the
+        result = DeleteFileNowOrSchedule( renamedDoomedFile );
-        // files like this (i.e. error if not in the same dir) or we need to take
+    }
-        // a little more care in the move.
+    else
-        parentofReplacementFile->Equals(parentofFinalFile, &flagIsEqual);
+    {
-        if(!flagIsEqual)
+        // couldn't rename file, try to put old file back
-        {
+        renamedDoomedFile->MoveToNative(nsnull, doomedLeafname);
-            NS_WARNING("File unpacked into a non-dest dir" );
+        // No need to reset remanedDoomedFile after a MoveToNative() call
-            replacementFile->GetNativeLeafName(leafname);
+        // because renamedDoomedFile is not used beyond here.
-            rv = replacementFile->MoveToNative(parentofFinalFile, leafname);
-            if (NS_FAILED(rv))
-                return nsInstall::UNEXPECTED_ERROR;
-
-            // The implementation of MoveToNative() on some platforms (osx and win32) resets
-            // the object to the 'moved to' filename.  This is incorrect behavior.  This
-            // implementation will be fixed in the future.  We need to take into account that
-            // fix by setting renamedDoomedFile to the filename that it was moved to above.
-            // See bug 200024.
-            rv = renamedDoomedFile->SetNativeLeafName(leafname);        
-            if (NS_FAILED(rv))
-                return nsInstall::UNEXPECTED_ERROR;
-        }
-        	
-        rv = doomedFile->GetNativeLeafName(leafname);
-        if ( NS_SUCCEEDED(rv))
-            rv = replacementFile->MoveToNative(parentofReplacementFile, leafname );
-
-        if ( NS_SUCCEEDED(rv) ) 
-        {
-            // we replaced the old file OK, now we have to
-            // get rid of it if it was renamed out of the way
-            result = DeleteFileNowOrSchedule( renamedDoomedFile );
-        }
-        else
-        {
-            // couldn't rename file, try to put old file back
-            renamedDoomedFile->GetParent(getter_AddRefs(parent));
-            renamedDoomedFile->MoveToNative(parent, leafname);
-        }
     }
 
     return result;
@@ -367,16 +374,16 @@ PRInt32 ReplaceFileNow(nsIFile* replacementFile, nsIFile* doomedFile )
 
 
 
-PRInt32 ReplaceFileNowOrSchedule(nsIFile* replacementFile, nsIFile* doomedFile, PRInt32 aMode)
+PRInt32 ReplaceFileNowOrSchedule(nsIFile* aReplacementFile, nsIFile* aDoomedFile, PRInt32 aMode)
 {
-    PRInt32 result = ReplaceFileNow( replacementFile, doomedFile );
+    PRInt32 result = ReplaceFileNow( aReplacementFile, aDoomedFile );
 
     if ( result == nsInstall::ACCESS_DENIED )
     {
         // if we couldn't replace the file schedule it for later
 #ifdef _WINDOWS
         if ( (aMode & WIN_SYSTEM_FILE) && 
-             (ReplaceWindowsSystemFile(replacementFile, doomedFile) == 0) )
+             (ReplaceWindowsSystemFile(aReplacementFile, aDoomedFile) == 0) )
                 return nsInstall::REBOOT_NEEDED;
 #endif
 
@@ -403,8 +410,8 @@ PRInt32 ReplaceFileNowOrSchedule(nsIFile* replacementFile, nsIFile* doomedFile,
                     {
                         nsCAutoString srcowner;
                         nsCAutoString destowner;
-                        nsresult rv = GetPersistentStringFromSpec(replacementFile, srcowner);
+                        nsresult rv = GetPersistentStringFromSpec(aReplacementFile, srcowner);
-                        nsresult rv2 = GetPersistentStringFromSpec(doomedFile, destowner);
+                        nsresult rv2 = GetPersistentStringFromSpec(aDoomedFile, destowner);
                         if ( NS_SUCCEEDED(rv) && NS_SUCCEEDED(rv2) )
                         {
                             const char *fsrc  = srcowner.get();