add Finish() method to nsISafeFileOutputStream, and tweak things slightly.

b=251091, r=mvl, sr=darin

extensions/cookie/nsPermissionManager.cpp

@@ -936,7 +936,7 @@ nsPermissionManager::Write()
   // that for us
   nsCOMPtr<nsISafeFileOutputStream> safeStream = do_QueryInterface(fileOutputStream);
   if (safeStream)
-    safeStream->SetWriteSucceeded(PR_TRUE);
+    safeStream->Finish();
 
   return NS_OK;
 }

netwerk/base/public/nsIFileStreams.idl

@@ -106,26 +106,24 @@ interface nsIFileOutputStream : nsIOutputStream
  * nsIFileOutputStream.
  *
  * A file output stream that supports this interface writes to a
- * temporary file, and moves it over the original file when the stream
- * is closed, and all writes succeeded.  If the stream is closed, and
- * something went wrong during writing, it will delete the temporary
- * file and not touch the original.
+ * temporary file, and moves it over the original file when |finish| is
+ * called only if the stream can be successfully closed and all writes
+ * succeeded.  If |finish| is called but something went wrong during
+ * writing, it will delete the temporary file and not touch the original.
+ * If the stream is closed by calling |close| directly, or the stream
+ * goes away, the original file will not be overwritten, and the temporary
+ * file will be deleted.
  */
-
 [scriptable, uuid(5f914307-5c34-4e1f-8e32-ec749d25b27a)]
 interface nsISafeFileOutputStream : nsISupports
 {
     /**
-     * Set this attribute to true to cause the original file to be
-     * overwritten. Note: if any call to |write| failed to write out
-     * all of the data given to it, then setting this attribute to
-     * true will be ignored. The file will only be saved if all calls
-     * to |write| succeeded and if this attribute is set to true.
-     * If this attribute isn't set before |close| is called,
-     * or the stream goes away, the original file will not be
-     * overwritten, and the temporary file will be deleted.
-     *
-     * The default value for this attribute is false.
+     * Call this method to close the stream and cause the original file
+     * to be overwritten. Note: if any call to |write| failed to write out
+     * all of the data given to it, then calling this method will |close| the
+     * stream and return failure. Further, if closing the stream fails, this
+     * method will return failure. The original file will be overwritten only
+     * if all calls to |write| succeeded and the stream was successfully closed.
      */
-    attribute boolean writeSucceeded;
+    void finish();
 };

netwerk/base/src/nsFileStreams.cpp

@@ -525,32 +525,33 @@ nsSafeFileOutputStream::Init(nsIFile* file, PRInt32 ioFlags, PRInt32 perm,
     return rv;
 }
 
-NS_IMETHODIMP
-nsSafeFileOutputStream::SetWriteSucceeded(PRBool aWriteSucceeded)
-{
-    mWriteSucceeded = aWriteSucceeded;
-    return NS_OK;
-}
-
-NS_IMETHODIMP
-nsSafeFileOutputStream::GetWriteSucceeded(PRBool *aWriteSucceeded)
-{
-    *aWriteSucceeded = mWriteSucceeded && mInternalWriteSucceeded;
-    return NS_OK;
-}
-
 NS_IMETHODIMP
 nsSafeFileOutputStream::Close()
 {
     nsresult rv = nsFileOutputStream::Close();
 
+    // the consumer doesn't want the original file overwritten -
+    // so clean up by removing the temp file.
+    if (mTempFile) {
+        mTempFile->Remove(PR_FALSE);
+        mTempFile = nsnull;
+    }
+
+    return rv;
+}
+
+NS_IMETHODIMP
+nsSafeFileOutputStream::Finish()
+{
+    nsresult rv = nsFileOutputStream::Close();
+
     // if there is no temp file, don't try to move it over the original target.
     // It would destroy the targetfile if close() is called twice.
     if (!mTempFile)
-        return NS_OK;
+        return rv;
 
     // Only overwrite if everything was ok, and the temp file could be closed.
-    if (mWriteSucceeded && mInternalWriteSucceeded && NS_SUCCEEDED(rv)) {
+    if (NS_SUCCEEDED(mWriteResult) && NS_SUCCEEDED(rv)) {
         NS_ENSURE_STATE(mTargetFile);
 
         if (!mTargetFileExists) {
@@ -573,7 +574,11 @@ nsSafeFileOutputStream::Close()
             rv = mTempFile->MoveToNative(nsnull, targetFilename); // This will replace target
     }
     else {
-        rv = mTempFile->Remove(PR_FALSE);
+        mTempFile->Remove(PR_FALSE);
+
+        // if writing failed, propagate the failure code to the caller.
+        if (NS_FAILED(mWriteResult))
+            rv = mWriteResult;
     }
     mTempFile = nsnull;
     return rv;
@@ -583,8 +588,15 @@ NS_IMETHODIMP
 nsSafeFileOutputStream::Write(const char *buf, PRUint32 count, PRUint32 *result)
 {
     nsresult rv = nsFileOutputStream::Write(buf, count, result);
-    if (NS_FAILED(rv) || count != *result)
-        mInternalWriteSucceeded = PR_FALSE;
+    if (NS_SUCCEEDED(mWriteResult)) {
+        if (NS_FAILED(rv))
+            mWriteResult = rv;
+        else if (count != *result)
+            mWriteResult = NS_ERROR_LOSS_OF_SIGNIFICANT_DATA;
+
+        if (NS_FAILED(mWriteResult))
+            NS_WARNING("writing to output stream failed! data may be lost");
+    } 
     return rv;
 }
 

netwerk/base/src/nsFileStreams.h

@@ -161,8 +161,7 @@ public:
     NS_DECL_NSISAFEFILEOUTPUTSTREAM
 
     nsSafeFileOutputStream() :
-        mWriteSucceeded(PR_FALSE), 
-        mInternalWriteSucceeded(PR_TRUE) {}
+        mWriteResult(NS_OK) {}
 
     virtual ~nsSafeFileOutputStream() { nsSafeFileOutputStream::Close(); }
 
@@ -174,9 +173,8 @@ protected:
     nsCOMPtr<nsIFile>         mTargetFile;
     nsCOMPtr<nsIFile>         mTempFile;
 
-    PRBool mTargetFileExists;
-    PRBool mWriteSucceeded;         // Consumer reported
-    PRBool mInternalWriteSucceeded; // Internally detected in Write()
+    PRBool   mTargetFileExists;
+    nsresult mWriteResult; // Internally set in Write()
 };
 
 ////////////////////////////////////////////////////////////////////////////////

netwerk/cookie/src/nsCookieService.cpp

@@ -1119,7 +1119,7 @@ nsCookieService::Write()
   // that for us
   nsCOMPtr<nsISafeFileOutputStream> safeStream = do_QueryInterface(fileOutputStream);
   if (safeStream)
-    safeStream->SetWriteSucceeded(PR_TRUE);
+    safeStream->Finish();
 
   mCookieChanged = PR_FALSE;
   return NS_OK;