rename nsISafeFileOutputStream to nsISafeOutputStream, and make nsBufferedOutputStream aware of it, to make life easier on consumers.

b=251648, r=biesi, sr=darin, with thanks to darin and bz for suggestions.

extensions/cookie/nsPermissionManager.cpp

@@ -932,23 +932,19 @@ nsPermissionManager::Write()
 
   delete[] hostList;
 
-  // XXX hack for bug 251648, to avoid dataloss:
-  // flush the buffered stream's data first
-  rv = bufferedOutputStream->Flush();
-
   // All went ok. Maybe except for problems in Write(), but the stream detects
   // that for us
-  nsCOMPtr<nsISafeFileOutputStream> safeStream = do_QueryInterface(fileOutputStream);
+  nsCOMPtr<nsISafeOutputStream> safeStream = do_QueryInterface(bufferedOutputStream);
-  if (NS_SUCCEEDED(rv) && safeStream)
+  if (safeStream) {
     rv = safeStream->Finish();
-
+    if (NS_FAILED(rv)) {
-  if (NS_FAILED(rv)) {
+      NS_WARNING("failed to save permissions file! possible dataloss");
-    NS_WARNING("failed to save permissions file! possible dataloss");
+      return rv;
-    return rv;
+    }
   }
 
   mChangedList = PR_FALSE;
-  return rv;
+  return NS_OK;
 }
 
 nsresult

netwerk/base/public/Makefile.in

@@ -62,6 +62,7 @@ SDK_XPIDLSRCS   = \
 XPIDLSRCS	= \
 		nsIAuthPrompt.idl \
 		nsIAsyncStreamCopier.idl \
+		nsISafeOutputStream.idl \
 		nsIBufferedStreams.idl \
 		nsIDownloader.idl \
 		nsIEncodedChannel.idl \

netwerk/base/public/nsIFileStreams.idl

@@ -96,34 +96,3 @@ interface nsIFileOutputStream : nsIOutputStream
     void init(in nsIFile file, in long ioFlags, in long perm,
               in long behaviorFlags);
 };
-
-/**
- * This interface provides a mechanism to control a file output stream
- * that takes care not to overwrite an existing file until it is known
- * that all writes to the file succeeded.
- * 
- * An object that supports this interface is intended to also support
- * nsIFileOutputStream.
- *
- * A file output stream that supports this interface writes to a
- * 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
-{
-    /**
-     * 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.
-     */
-    void finish();
-};

netwerk/base/public/nsISafeOutputStream.idl

@@ -0,0 +1,72 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is
+ * Michiel van Leeuwen (mvl@exedo.nl).
+ * Portions created by the Initial Developer are Copyright (C) 2004
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Daniel Witte (dwitte@stanford.edu)
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+#include "nsISupports.idl"
+
+/**
+ * This interface provides a mechanism to control an output stream
+ * that takes care not to overwrite an existing target until it is known
+ * that all writes to the destination succeeded.
+ * 
+ * An object that supports this interface is intended to also support
+ * nsIOutputStream.
+ *
+ * For example, a file output stream that supports this interface writes to
+ * a 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.
+ *
+ * Currently, this interface is implemented only for file output streams.
+ */
+[scriptable, uuid(5f914307-5c34-4e1f-8e32-ec749d25b27a)]
+interface nsISafeOutputStream : nsISupports
+{
+    /**
+     * Call this method to close the stream and cause the original target
+     * 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 target will be overwritten only
+     * if all calls to |write| succeeded and the stream was successfully closed.
+     */
+    void finish();
+};

netwerk/base/public/nsNetUtil.h

@@ -50,6 +50,7 @@
 #include "nsIURI.h"
 #include "nsIInputStream.h"
 #include "nsIOutputStream.h"
+#include "nsISafeOutputStream.h"
 #include "nsIStreamListener.h"
 #include "nsIRequestObserverProxy.h"
 #include "nsIStreamListenerProxy.h" // XXX for nsIAsyncStreamListener
@@ -730,8 +731,7 @@ NS_NewLocalFileOutputStream(nsIOutputStream **aResult,
     return rv;
 }
 
-// Returns a file output stream. The object can be QI-ed to
+// returns a file output stream which can be QI'ed to nsISafeOutputStream.
-// nsISafeFileOuputStream.
 inline nsresult
 NS_NewSafeLocalFileOutputStream(nsIOutputStream **aResult,
                                 nsIFile          *aFile,
@@ -816,6 +816,8 @@ NS_NewBufferedInputStream(nsIInputStream **aResult,
     return rv;
 }
 
+// note: the resulting stream can be QI'ed to nsISafeOutputStream iff the
+// provided stream supports it.
 inline nsresult
 NS_NewBufferedOutputStream(nsIOutputStream **aResult,
                            nsIOutputStream  *aStr,

netwerk/base/src/nsBufferedStreams.cpp

@@ -478,12 +478,17 @@ nsBufferedInputStream::GetUnbufferedStream(nsISupports* *aStream)
 ////////////////////////////////////////////////////////////////////////////////
 // nsBufferedOutputStream
 
-NS_IMPL_ISUPPORTS_INHERITED3(nsBufferedOutputStream, 
+NS_IMPL_ADDREF_INHERITED(nsBufferedOutputStream, nsBufferedStream)
-                             nsBufferedStream,
+NS_IMPL_RELEASE_INHERITED(nsBufferedOutputStream, nsBufferedStream)
-                             nsIOutputStream,
+// This QI uses NS_INTERFACE_MAP_ENTRY_CONDITIONAL to check for
-                             nsIBufferedOutputStream,
+// non-nullness of mSafeStream.
-                             nsIStreamBufferAccess)
+NS_INTERFACE_MAP_BEGIN(nsBufferedOutputStream)
- 
+    NS_INTERFACE_MAP_ENTRY(nsIOutputStream)
+    NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsISafeOutputStream, mSafeStream)
+    NS_INTERFACE_MAP_ENTRY(nsIBufferedOutputStream)
+    NS_INTERFACE_MAP_ENTRY(nsIStreamBufferAccess)
+NS_INTERFACE_MAP_END_INHERITING(nsBufferedStream)
+
 NS_METHOD
 nsBufferedOutputStream::Create(nsISupports *aOuter, REFNSIID aIID, void **aResult)
 {
@@ -501,6 +506,9 @@ nsBufferedOutputStream::Create(nsISupports *aOuter, REFNSIID aIID, void **aResul
 NS_IMETHODIMP
 nsBufferedOutputStream::Init(nsIOutputStream* stream, PRUint32 bufferSize)
 {
+    // QI stream to an nsISafeOutputStream, to see if we should support it
+    mSafeStream = do_QueryInterface(stream);
+
     return nsBufferedStream::Init(stream, bufferSize);
 }
 
@@ -573,6 +581,28 @@ nsBufferedOutputStream::Flush()
     return NS_ERROR_FAILURE;        // didn't flush all
 }
 
+// nsISafeOutputStream
+NS_IMETHODIMP
+nsBufferedOutputStream::Finish()
+{
+    // flush the stream, to write out any buffered data...
+    nsresult rv = nsBufferedOutputStream::Flush();
+    if (NS_FAILED(rv))
+        NS_WARNING("failed to flush buffered data! possible dataloss");
+
+    // ... and finish the underlying stream...
+    if (NS_SUCCEEDED(rv))
+        rv = mSafeStream->Finish();
+    else
+        Sink()->Close();
+
+    // ... and close the buffered stream, so any further attempts to flush/close
+    // the buffered stream won't cause errors.
+    nsBufferedStream::Close();
+
+    return rv;
+}
+
 static NS_METHOD
 nsReadFromInputStream(nsIOutputStream* outStr,
                       void* closure,

netwerk/base/src/nsBufferedStreams.h

@@ -41,6 +41,7 @@
 #include "nsIBufferedStreams.h"
 #include "nsIInputStream.h"
 #include "nsIOutputStream.h"
+#include "nsISafeOutputStream.h"
 #include "nsISeekableStream.h"
 #include "nsIStreamBufferAccess.h"
 #include "nsCOMPtr.h"
@@ -114,12 +115,14 @@ protected:
 ////////////////////////////////////////////////////////////////////////////////
 
 class nsBufferedOutputStream : public nsBufferedStream, 
+                               public nsISafeOutputStream,
                                public nsIBufferedOutputStream,
                                public nsIStreamBufferAccess
 {
 public:
     NS_DECL_ISUPPORTS_INHERITED
     NS_DECL_NSIOUTPUTSTREAM
+    NS_DECL_NSISAFEOUTPUTSTREAM
     NS_DECL_NSIBUFFEREDOUTPUTSTREAM
     NS_DECL_NSISTREAMBUFFERACCESS
 
@@ -135,6 +138,8 @@ public:
 
 protected:
     NS_IMETHOD Fill() { return NS_OK; } // no-op for input streams
+
+    nsCOMPtr<nsISafeOutputStream> mSafeStream; // QI'd from mStream
 };
 
 ////////////////////////////////////////////////////////////////////////////////

netwerk/base/src/nsFileStreams.cpp

@@ -491,7 +491,7 @@ nsFileOutputStream::IsNonBlocking(PRBool *aNonBlocking)
 
 NS_IMPL_ISUPPORTS_INHERITED3(nsSafeFileOutputStream, 
                              nsFileOutputStream,
-                             nsISafeFileOutputStream,
+                             nsISafeOutputStream,
                              nsIOutputStream,
                              nsIFileOutputStream)
 
@@ -505,6 +505,7 @@ nsSafeFileOutputStream::Init(nsIFile* file, PRInt32 ioFlags, PRInt32 perm,
         mTargetFileExists = PR_TRUE; // Safer to assume it exists - we just do more work.
     }
 
+    // XXXdwitte I think we want to be following symlinks here... see e.g. bug 206567.
     nsCOMPtr<nsIFile> tempResult;
     rv = file->Clone(getter_AddRefs(tempResult));
     if (NS_SUCCEEDED(rv) && mTargetFileExists) {
@@ -514,7 +515,7 @@ nsSafeFileOutputStream::Init(nsIFile* file, PRInt32 ioFlags, PRInt32 perm,
             origPerm = perm;
         }
         // XXX What if |perm| is more restrictive then |origPerm|?
-        // This leaves the user supplied permssions as they were.
+        // This leaves the user supplied permissions as they were.
         rv = tempResult->CreateUnique(nsIFile::NORMAL_FILE_TYPE, origPerm);
     }
     if (NS_SUCCEEDED(rv)) {

netwerk/base/src/nsFileStreams.h

@@ -42,6 +42,7 @@
 #include "nsIFile.h"
 #include "nsIInputStream.h"
 #include "nsIOutputStream.h"
+#include "nsISafeOutputStream.h"
 #include "nsISeekableStream.h"
 #include "nsILineInputStream.h"
 #include "nsCOMPtr.h"
@@ -154,11 +155,11 @@ public:
 ////////////////////////////////////////////////////////////////////////////////
 
 class nsSafeFileOutputStream : public nsFileOutputStream,
-                               public nsISafeFileOutputStream
+                               public nsISafeOutputStream
 {
 public:
     NS_DECL_ISUPPORTS_INHERITED
-    NS_DECL_NSISAFEFILEOUTPUTSTREAM
+    NS_DECL_NSISAFEOUTPUTSTREAM
 
     nsSafeFileOutputStream() :
         mWriteResult(NS_OK) {}

netwerk/build/nsNetCID.h

@@ -435,7 +435,7 @@
     {0x8c, 0xda, 0x00, 0x60, 0xb0, 0xfc, 0x14, 0xa3} \
 }
 
-// component implementing nsISafeFileOutputStream
+// component implementing nsISafeOutputStream
 #define NS_SAFELOCALFILEOUTPUTSTREAM_CLASSNAME \
     "nsSafeFileOutputStream"
 #define NS_SAFELOCALFILEOUTPUTSTREAM_CONTRACTID \

netwerk/cookie/src/nsCookieService.cpp

@@ -430,9 +430,6 @@ nsCookieService::~nsCookieService()
 
   if (mWriteTimer)
     mWriteTimer->Cancel();
-
-  // clean up memory
-  RemoveAllFromMemory();
 }
 
 NS_IMETHODIMP
@@ -1115,23 +1112,19 @@ nsCookieService::Write()
     bufferedOutputStream->Write(kNew, sizeof(kNew) - 1, &rv);
   }
 
-  // XXX hack for bug 251648, to avoid dataloss:
-  // flush the buffered stream's data first
-  rv = bufferedOutputStream->Flush();
-
   // All went ok. Maybe except for problems in Write(), but the stream detects
   // that for us
-  nsCOMPtr<nsISafeFileOutputStream> safeStream = do_QueryInterface(fileOutputStream);
+  nsCOMPtr<nsISafeOutputStream> safeStream = do_QueryInterface(bufferedOutputStream);
-  if (NS_SUCCEEDED(rv) && safeStream)
+  if (safeStream) {
     rv = safeStream->Finish();
-
+    if (NS_FAILED(rv)) {
-  if (NS_FAILED(rv)) {
+      NS_WARNING("failed to save cookie file! possible dataloss");
-    NS_WARNING("failed to save cookie file! possible dataloss");
+      return rv;
-    return rv;
+    }
   }
 
   mCookieChanged = PR_FALSE;
-  return rv;
+  return NS_OK;
 }
 
 /******************************************************************************