Bug 1409329 - NS_NewBufferedOutputStream should take the ownership of the outputStream, r=smaug
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 24 Oct 2017 14:38:23 +0200
changeset 387853 17c1e024efc2ac9dd1638c568d70c91548e986dc
parent 387852 34b8dc4374dc3d707402d11ef3e2f9e82791f486
child 387854 3d2dad9361218c0b1b1cbe9a9644aafcaa0d6184
push id32738
push userarchaeopteryx@coole-files.de
push dateTue, 24 Oct 2017 21:58:00 +0000
treeherdermozilla-central@a124f4901430 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1409329
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1409329 - NS_NewBufferedOutputStream should take the ownership of the outputStream, r=smaug
CLOBBER
dom/webbrowserpersist/nsWebBrowserPersist.cpp
extensions/spellcheck/src/mozPersonalDictionary.cpp
modules/libjar/zipwriter/nsZipWriter.cpp
modules/libpref/Preferences.cpp
netwerk/base/BackgroundFileSaver.cpp
netwerk/base/nsNetUtil.cpp
netwerk/base/nsNetUtil.h
netwerk/cache/nsDiskCacheDeviceSQL.cpp
rdf/base/nsRDFXMLDataSource.cpp
security/manager/ssl/nsCertOverrideService.cpp
toolkit/components/url-classifier/VariableLengthPrefixSet.cpp
toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
--- a/CLOBBER
+++ b/CLOBBER
@@ -17,9 +17,9 @@
 #
 # Modifying this file will now automatically clobber the buildbot machines \o/
 #
 
 # Are you updating CLOBBER because you think it's needed for your WebIDL
 # changes to stick? As of bug 928195, this shouldn't be necessary! Please
 # don't change CLOBBER for WebIDL changes any more.
 
-Merge day clobber
+Bug 1409329 works better after a clobber.
--- a/dom/webbrowserpersist/nsWebBrowserPersist.cpp
+++ b/dom/webbrowserpersist/nsWebBrowserPersist.cpp
@@ -2317,18 +2317,19 @@ nsWebBrowserPersist::MakeOutputStreamFro
 
     // XXX brade:  get the right flags here!
     int32_t ioFlags = -1;
     if (mPersistFlags & nsIWebBrowserPersist::PERSIST_FLAGS_APPEND_TO_FILE)
       ioFlags = PR_APPEND | PR_CREATE_FILE | PR_WRONLY;
     rv = fileOutputStream->Init(aFile, ioFlags, -1, 0);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    *aOutputStream = NS_BufferOutputStream(fileOutputStream,
-                                           BUFFERED_OUTPUT_SIZE).take();
+    rv = NS_NewBufferedOutputStream(aOutputStream, fileOutputStream.forget(),
+                                    BUFFERED_OUTPUT_SIZE);
+    NS_ENSURE_SUCCESS(rv, rv);
 
     if (mPersistFlags & PERSIST_FLAGS_CLEANUP_ON_FAILURE)
     {
         // Add to cleanup list in event of failure
         auto *cleanupData = new CleanupData;
         if (!cleanupData) {
           NS_RELEASE(*aOutputStream);
           return NS_ERROR_OUT_OF_MEMORY;
--- a/extensions/spellcheck/src/mozPersonalDictionary.cpp
+++ b/extensions/spellcheck/src/mozPersonalDictionary.cpp
@@ -100,17 +100,17 @@ public:
       nsCOMPtr<nsIOutputStream> outStream;
       NS_NewSafeLocalFileOutputStream(getter_AddRefs(outStream), mFile,
                                       PR_CREATE_FILE | PR_WRONLY | PR_TRUNCATE,
                                       0664);
 
       // Get a buffered output stream 4096 bytes big, to optimize writes.
       nsCOMPtr<nsIOutputStream> bufferedOutputStream;
       res = NS_NewBufferedOutputStream(getter_AddRefs(bufferedOutputStream),
-                                       outStream, 4096);
+                                       outStream.forget(), 4096);
       if (NS_FAILED(res)) {
         return res;
       }
 
       uint32_t bytesWritten;
       nsAutoCString utf8Key;
       for (uint32_t i = 0; i < mDictWords.Length(); ++i) {
         CopyUTF16toUTF8(mDictWords[i], utf8Key);
--- a/modules/libjar/zipwriter/nsZipWriter.cpp
+++ b/modules/libjar/zipwriter/nsZipWriter.cpp
@@ -265,19 +265,19 @@ NS_IMETHODIMP nsZipWriter::Open(nsIFile 
     nsCOMPtr<nsIOutputStream> stream;
     rv = NS_NewLocalFileOutputStream(getter_AddRefs(stream), mFile, aIoFlags);
     if (NS_FAILED(rv)) {
         mHeaders.Clear();
         mEntryHash.Clear();
         return rv;
     }
 
-    rv = NS_NewBufferedOutputStream(getter_AddRefs(mStream), stream, 64 * 1024);
+    rv = NS_NewBufferedOutputStream(getter_AddRefs(mStream), stream.forget(),
+                                    64 * 1024);
     if (NS_FAILED(rv)) {
-        stream->Close();
         mHeaders.Clear();
         mEntryHash.Clear();
         return rv;
     }
 
     if (mCDSOffset > 0) {
         rv = SeekCDS();
         NS_ENSURE_SUCCESS(rv, rv);
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -3468,17 +3468,17 @@ public:
     // Execute a "safe" save by saving through a tempfile.
     rv = NS_NewSafeLocalFileOutputStream(
       getter_AddRefs(outStreamSink), aFile, -1, 0600);
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     rv = NS_NewBufferedOutputStream(
-      getter_AddRefs(outStream), outStreamSink, 4096);
+      getter_AddRefs(outStream), outStreamSink.forget(), 4096);
     if (NS_FAILED(rv)) {
       return rv;
     }
 
     struct CharComparator
     {
       bool LessThan(const mozilla::UniqueFreePtr<char>& a,
                     const mozilla::UniqueFreePtr<char>& b) const
--- a/netwerk/base/BackgroundFileSaver.cpp
+++ b/netwerk/base/BackgroundFileSaver.cpp
@@ -615,20 +615,21 @@ BackgroundFileSaver::ProcessStateChange(
   // extension. Those part files should never be group or world-writable even
   // if the umask allows it.
   nsCOMPtr<nsIOutputStream> outputStream;
   rv = NS_NewLocalFileOutputStream(getter_AddRefs(outputStream),
                                    mActualTarget,
                                    PR_WRONLY | creationIoFlags, 0600);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  outputStream = NS_BufferOutputStream(outputStream, BUFFERED_IO_SIZE);
-  if (!outputStream) {
-    return NS_ERROR_FAILURE;
-  }
+  nsCOMPtr<nsIOutputStream> bufferedStream;
+  rv = NS_NewBufferedOutputStream(getter_AddRefs(bufferedStream),
+                                  outputStream.forget(), BUFFERED_IO_SIZE);
+  NS_ENSURE_SUCCESS(rv, rv);
+  outputStream = bufferedStream;
 
   // Wrap the output stream so that it feeds the digest context if needed.
   if (mDigestContext) {
     // No need to acquire the NSS lock here, DigestOutputStream must acquire it
     // in any case before each asynchronous write. Constructing the
     // DigestOutputStream cannot fail. Passing mDigestContext to
     // DigestOutputStream is safe, because BackgroundFileSaver always outlives
     // the outputStream. BackgroundFileSaver is reference-counted before the
--- a/netwerk/base/nsNetUtil.cpp
+++ b/netwerk/base/nsNetUtil.cpp
@@ -1332,48 +1332,34 @@ NS_NewLocalFileStream(nsIFileStream **re
         rv = stream->Init(file, ioFlags, perm, behaviorFlags);
         if (NS_SUCCEEDED(rv))
             stream.forget(result);
     }
     return rv;
 }
 
 nsresult
-NS_NewBufferedOutputStream(nsIOutputStream **result,
-                           nsIOutputStream  *str,
-                           uint32_t          bufferSize)
+NS_NewBufferedOutputStream(nsIOutputStream** aResult,
+                           already_AddRefed<nsIOutputStream> aOutputStream,
+                           uint32_t aBufferSize)
 {
+    nsCOMPtr<nsIOutputStream> outputStream = Move(aOutputStream);
+
     nsresult rv;
     nsCOMPtr<nsIBufferedOutputStream> out =
         do_CreateInstance(NS_BUFFEREDOUTPUTSTREAM_CONTRACTID, &rv);
     if (NS_SUCCEEDED(rv)) {
-        rv = out->Init(str, bufferSize);
+        rv = out->Init(outputStream, aBufferSize);
         if (NS_SUCCEEDED(rv)) {
-            out.forget(result);
+            out.forget(aResult);
         }
     }
     return rv;
 }
 
-already_AddRefed<nsIOutputStream>
-NS_BufferOutputStream(nsIOutputStream *aOutputStream,
-                      uint32_t aBufferSize)
-{
-    NS_ASSERTION(aOutputStream, "No output stream given!");
-
-    nsCOMPtr<nsIOutputStream> bos;
-    nsresult rv = NS_NewBufferedOutputStream(getter_AddRefs(bos), aOutputStream,
-                                             aBufferSize);
-    if (NS_SUCCEEDED(rv))
-        return bos.forget();
-
-    bos = aOutputStream;
-    return bos.forget();
-}
-
 MOZ_MUST_USE nsresult
 NS_NewBufferedInputStream(nsIInputStream** aResult,
                           already_AddRefed<nsIInputStream> aInputStream,
                           uint32_t aBufferSize)
 {
     nsCOMPtr<nsIInputStream> inputStream = Move(aInputStream);
 
     nsresult rv;
--- a/netwerk/base/nsNetUtil.h
+++ b/netwerk/base/nsNetUtil.h
@@ -512,34 +512,19 @@ nsresult NS_NewLocalFileStream(nsIFileSt
 
 MOZ_MUST_USE nsresult
 NS_NewBufferedInputStream(nsIInputStream** aResult,
                           already_AddRefed<nsIInputStream> aInputStream,
                           uint32_t aBufferSize);
 
 // note: the resulting stream can be QI'ed to nsISafeOutputStream iff the
 // provided stream supports it.
-nsresult NS_NewBufferedOutputStream(nsIOutputStream **result,
-                                    nsIOutputStream  *str,
-                                    uint32_t          bufferSize);
-
-/**
- * Attempts to buffer a given stream.  If this fails, it returns the
- * passed-in stream.
- *
- * @param aOutputStream
- *        The output stream we want to buffer.  This cannot be null.
- * @param aBufferSize
- *        The size of the buffer for the buffered output stream.
- * @returns an nsIOutputStream that is buffered with the specified buffer size,
- *          or is aOutputStream if creating the new buffered stream failed.
- */
-already_AddRefed<nsIOutputStream>
-NS_BufferOutputStream(nsIOutputStream *aOutputStream,
-                      uint32_t aBufferSize);
+nsresult NS_NewBufferedOutputStream(nsIOutputStream** aResult,
+                                    already_AddRefed<nsIOutputStream> aOutputStream,
+                                    uint32_t aBufferSize);
 
 // returns an input stream compatible with nsIUploadChannel::SetUploadStream()
 nsresult NS_NewPostDataStream(nsIInputStream  **result,
                               bool              isFile,
                               const nsACString &data);
 
 nsresult NS_ReadInputStreamToBuffer(nsIInputStream *aInputStream,
                                     void **aDest,
--- a/netwerk/cache/nsDiskCacheDeviceSQL.cpp
+++ b/netwerk/cache/nsDiskCacheDeviceSQL.cpp
@@ -1810,20 +1810,21 @@ nsOfflineCacheDevice::OpenOutputStreamFo
   if (offset != 0)
     seekable->Seek(nsISeekableStream::NS_SEEK_SET, offset);
 
   // truncate the file at the given offset
   seekable->SetEOF();
 
   nsCOMPtr<nsIOutputStream> bufferedOut;
   nsresult rv =
-    NS_NewBufferedOutputStream(getter_AddRefs(bufferedOut), out, 16 * 1024);
+    NS_NewBufferedOutputStream(getter_AddRefs(bufferedOut), out.forget(),
+                               16 * 1024);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  bufferedOut.swap(*result);
+  bufferedOut.forget(result);
   return NS_OK;
 }
 
 nsresult
 nsOfflineCacheDevice::GetFileForEntry(nsCacheEntry *entry, nsIFile **result)
 {
   LOG(("nsOfflineCacheDevice::GetFileForEntry [key=%s]\n",
        entry->Key()->get()));
--- a/rdf/base/nsRDFXMLDataSource.cpp
+++ b/rdf/base/nsRDFXMLDataSource.cpp
@@ -753,17 +753,18 @@ RDFXMLDataSourceImpl::rdfXMLFlush(nsIURI
             rv = NS_NewSafeLocalFileOutputStream(getter_AddRefs(out),
                                                  file,
                                                  PR_WRONLY | PR_CREATE_FILE,
                                                  /*octal*/ 0666,
                                                  0);
             if (NS_FAILED(rv)) return rv;
 
             nsCOMPtr<nsIOutputStream> bufferedOut;
-            rv = NS_NewBufferedOutputStream(getter_AddRefs(bufferedOut), out, 4096);
+            rv = NS_NewBufferedOutputStream(getter_AddRefs(bufferedOut),
+                                            out.forget(), 4096);
             if (NS_FAILED(rv)) return rv;
 
             rv = Serialize(bufferedOut);
             if (NS_FAILED(rv)) return rv;
 
             // All went ok. Maybe except for problems in Write(), but the stream detects
             // that for us
             nsCOMPtr<nsISafeOutputStream> safeStream = do_QueryInterface(bufferedOut, &rv);
--- a/security/manager/ssl/nsCertOverrideService.cpp
+++ b/security/manager/ssl/nsCertOverrideService.cpp
@@ -276,17 +276,18 @@ nsCertOverrideService::Write(const Mutex
                                        0600);
   if (NS_FAILED(rv)) {
     NS_ERROR("failed to open cert_warn_settings.txt for writing");
     return rv;
   }
 
   // get a buffered output stream 4096 bytes big, to optimize writes
   nsCOMPtr<nsIOutputStream> bufferedOutputStream;
-  rv = NS_NewBufferedOutputStream(getter_AddRefs(bufferedOutputStream), fileOutputStream, 4096);
+  rv = NS_NewBufferedOutputStream(getter_AddRefs(bufferedOutputStream),
+                                  fileOutputStream.forget(), 4096);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   static const char kHeader[] =
       "# PSM Certificate Override Settings file" NS_LINEBREAK
       "# This is a generated file!  Do not edit." NS_LINEBREAK;
 
--- a/toolkit/components/url-classifier/VariableLengthPrefixSet.cpp
+++ b/toolkit/components/url-classifier/VariableLengthPrefixSet.cpp
@@ -273,18 +273,20 @@ VariableLengthPrefixSet::StoreToFile(nsI
 
     fileSize += mFixedPrefixSet->CalculatePreallocateSize();
     fileSize += CalculatePreallocateSize();
 
     Unused << fos->Preallocate(fileSize);
   }
 
   // Convert to buffered stream
-  nsCOMPtr<nsIOutputStream> out =
-    NS_BufferOutputStream(localOutFile, std::min(fileSize, MAX_BUFFER_SIZE));
+  nsCOMPtr<nsIOutputStream> out;
+  rv = NS_NewBufferedOutputStream(getter_AddRefs(out), localOutFile.forget(),
+                                  std::min(fileSize, MAX_BUFFER_SIZE));
+  NS_ENSURE_SUCCESS(rv, rv);
 
   rv = mFixedPrefixSet->WritePrefixes(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
   rv = WritePrefixes(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
   return NS_OK;
--- a/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierPrefixSet.cpp
@@ -389,18 +389,20 @@ nsUrlClassifierPrefixSet::StoreToFile(ns
     fileSize = CalculatePreallocateSize();
 
     // Ignore failure, the preallocation is a hint and we write out the entire
     // file later on
     Unused << fos->Preallocate(fileSize);
   }
 
   // Convert to buffered stream
-  nsCOMPtr<nsIOutputStream> out =
-    NS_BufferOutputStream(localOutFile, std::min(fileSize, MAX_BUFFER_SIZE));
+  nsCOMPtr<nsIOutputStream> out;
+  rv = NS_NewBufferedOutputStream(getter_AddRefs(out), localOutFile.forget(),
+                                  std::min(fileSize, MAX_BUFFER_SIZE));
+  NS_ENSURE_SUCCESS(rv, rv);
 
   rv = WritePrefixes(out);
   NS_ENSURE_SUCCESS(rv, rv);
 
   LOG(("Saving PrefixSet successful\n"));
 
   return NS_OK;
 }