Bug 993591: Eagerly free nsStreamLoader data. r=mcmanus
authorBen Kelly <ben@wanderview.com>
Thu, 17 Apr 2014 10:59:54 -0400
changeset 198706 feded4c79308a7f972aacbec7e0c61d157482b7c
parent 198705 800df0af425160dd45f7d9cbf63796b7b6a4c633
child 198708 3f6c842e8180f279d145d80d14270125c7fd785e
push id486
push userasasaki@mozilla.com
push dateMon, 14 Jul 2014 18:39:42 +0000
treeherdermozilla-release@d33428174ff1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs993591
milestone31.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 993591: Eagerly free nsStreamLoader data. r=mcmanus
netwerk/base/public/nsIStreamLoader.idl
netwerk/base/src/nsStreamLoader.cpp
netwerk/base/src/nsStreamLoader.h
--- a/netwerk/base/public/nsIStreamLoader.idl
+++ b/netwerk/base/public/nsIStreamLoader.idl
@@ -22,19 +22,19 @@ interface nsIStreamLoaderObserver : nsIS
      *
      * This method will always be called asynchronously by the
      * nsIStreamLoader involved, on the thread that called the
      * loader's init() method.
      *
      * If the observer wants to take over responsibility for the
      * data buffer (result), it returns NS_SUCCESS_ADOPTED_DATA
      * in place of NS_OK as its success code. The loader will then
-     * "forget" about the data, and not NS_Free() it in its own
-     * destructor; observer must call NS_Free() when the data is
-     * no longer required.
+     * "forget" about the data and not NS_Free() it after
+     * onStreamComplete() returns; observer must call NS_Free()
+     * when the data is no longer required.
      */
     void onStreamComplete(in nsIStreamLoader loader,
                           in nsISupports ctxt,
                           in nsresult status,
                           in unsigned long resultLength,
                           [const,array,size_is(resultLength)] in octet result);
 };
 
--- a/netwerk/base/src/nsStreamLoader.cpp
+++ b/netwerk/base/src/nsStreamLoader.cpp
@@ -13,19 +13,17 @@ nsStreamLoader::nsStreamLoader()
   : mData(nullptr),
     mAllocated(0),
     mLength(0)
 {
 }
 
 nsStreamLoader::~nsStreamLoader()
 {
-  if (mData) {
-    NS_Free(mData);
-  }
+  ReleaseData();
 }
 
 NS_IMETHODIMP
 nsStreamLoader::Init(nsIStreamLoaderObserver* observer)
 {
   NS_ENSURE_ARG_POINTER(observer);
   mObserver = observer;
   return NS_OK;
@@ -98,20 +96,19 @@ nsStreamLoader::OnStopRequest(nsIRequest
     // provide nsIStreamLoader::request during call to OnStreamComplete
     mRequest = request;
     nsresult rv = mObserver->OnStreamComplete(this, mContext, aStatus,
                                               mLength, mData);
     if (rv == NS_SUCCESS_ADOPTED_DATA) {
       // the observer now owns the data buffer, and the loader must
       // not deallocate it
       mData = nullptr;
-      mLength = 0;
-      mAllocated = 0;
     }
     // done.. cleanup
+    ReleaseData();
     mRequest = 0;
     mObserver = 0;
     mContext = 0;
   }
   return NS_OK;
 }
 
 NS_METHOD
@@ -127,18 +124,17 @@ nsStreamLoader::WriteSegmentFun(nsIInput
   if (count > UINT32_MAX - self->mLength) {
     return NS_ERROR_ILLEGAL_VALUE; // is there a better error to use here?
   }
 
   if (self->mLength + count > self->mAllocated) {
     self->mData = static_cast<uint8_t*>(NS_Realloc(self->mData,
                                                    self->mLength + count));
     if (!self->mData) {
-      self->mLength = 0;
-      self->mAllocated = 0;
+      self->ReleaseData();
       return NS_ERROR_OUT_OF_MEMORY;
     }
     self->mAllocated = self->mLength + count;
   }
 
   ::memcpy(self->mData + self->mLength, fromSegment, count);
   self->mLength += count;
 
@@ -150,8 +146,19 @@ nsStreamLoader::WriteSegmentFun(nsIInput
 NS_IMETHODIMP 
 nsStreamLoader::OnDataAvailable(nsIRequest* request, nsISupports *ctxt, 
                                 nsIInputStream *inStr, 
                                 uint64_t sourceOffset, uint32_t count)
 {
   uint32_t countRead;
   return inStr->ReadSegments(WriteSegmentFun, this, count, &countRead);
 }
+
+void
+nsStreamLoader::ReleaseData()
+{
+  if (mData) {
+    NS_Free(mData);
+    mData = nullptr;
+  }
+  mLength = 0;
+  mAllocated = 0;
+}
--- a/netwerk/base/src/nsStreamLoader.h
+++ b/netwerk/base/src/nsStreamLoader.h
@@ -25,16 +25,20 @@ public:
 
   static nsresult
   Create(nsISupports *aOuter, REFNSIID aIID, void **aResult);
 
 protected:
   static NS_METHOD WriteSegmentFun(nsIInputStream *, void *, const char *,
                                    uint32_t, uint32_t, uint32_t *);
 
+  // Utility method to free mData, if present, and update other state to
+  // reflect that no data has been allocated.
+  void ReleaseData();
+
   nsCOMPtr<nsIStreamLoaderObserver> mObserver;
   nsCOMPtr<nsISupports>             mContext;  // the observer's context
   nsCOMPtr<nsIRequest>              mRequest;
 
   uint8_t  *mData;      // buffer to accumulate incoming data
   uint32_t  mAllocated; // allocated size of data buffer (we preallocate if
                         //   contentSize is available)
   uint32_t  mLength;    // actual length of data in buffer