Bug 772528 - Remove nsFileInputStream::Seek() from nsPartialFileInputStream::Init(). r=baku
authorEden Chuang <echuang@mozilla.com>
Mon, 21 Mar 2016 10:48:59 +0800
changeset 289929 422d5142ae422a67797f9d04488b538ebf90cbb5
parent 289928 81ba35b38dc1626863841170cf5293e6d238b6bf
child 289930 5d3a8a8f017842b11d0c2b969586b59280e20f4c
push id74041
push userryanvm@gmail.com
push dateWed, 23 Mar 2016 02:47:03 +0000
treeherdermozilla-inbound@22615a6fd646 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs772528
milestone48.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 772528 - Remove nsFileInputStream::Seek() from nsPartialFileInputStream::Init(). r=baku
netwerk/base/nsFileStreams.cpp
netwerk/base/nsFileStreams.h
--- a/netwerk/base/nsFileStreams.cpp
+++ b/netwerk/base/nsFileStreams.cpp
@@ -318,16 +318,18 @@ nsFileStreamBase::MaybeOpen(nsIFile* aFi
         NS_ENSURE_TRUE(mOpenParams.localFile, NS_ERROR_UNEXPECTED);
 
         mDeferredOpen = true;
         return NS_OK;
     }
 
     mOpenParams.localFile = aFile;
 
+    // Following call open() at main thread.
+    // Main thread might be blocked, while open a remote file.
     return DoOpen();
 }
 
 void
 nsFileStreamBase::CleanUpOpen()
 {
     mOpenParams.localFile = nullptr;
     mDeferredOpen = false;
@@ -427,29 +429,33 @@ nsFileInputStream::Open(nsIFile* aFile, 
     // Open the file
     if (aIOFlags == -1)
         aIOFlags = PR_RDONLY;
     if (aPerm == -1)
         aPerm = 0;
 
     rv = MaybeOpen(aFile, aIOFlags, aPerm,
                    mBehaviorFlags & nsIFileInputStream::DEFER_OPEN);
+
     if (NS_FAILED(rv)) return rv;
 
-    if (mBehaviorFlags & DELETE_ON_CLOSE) {
-        // POSIX compatible filesystems allow a file to be unlinked while a
-        // file descriptor is still referencing the file.  since we've already
-        // opened the file descriptor, we'll try to remove the file.  if that
-        // fails, then we'll just remember the nsIFile and remove it after we
-        // close the file descriptor.
-        rv = aFile->Remove(false);
-        if (NS_SUCCEEDED(rv)) {
-          // No need to remove it later. Clear the flag.
-          mBehaviorFlags &= ~DELETE_ON_CLOSE;
-        }
+    // if defer open is set, do not remove the file here.
+    // remove the file while Close() is called.
+    if ((mBehaviorFlags & DELETE_ON_CLOSE) &&
+        !(mBehaviorFlags & nsIFileInputStream::DEFER_OPEN)) {
+      // POSIX compatible filesystems allow a file to be unlinked while a
+      // file descriptor is still referencing the file.  since we've already
+      // opened the file descriptor, we'll try to remove the file.  if that
+      // fails, then we'll just remember the nsIFile and remove it after we
+      // close the file descriptor.
+      rv = aFile->Remove(false);
+      if (NS_SUCCEEDED(rv)) {
+        // No need to remove it later. Clear the flag.
+        mBehaviorFlags &= ~DELETE_ON_CLOSE;
+      }
     }
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFileInputStream::Init(nsIFile* aFile, int32_t aIOFlags, int32_t aPerm,
                         int32_t aBehaviorFlags)
@@ -509,32 +515,37 @@ nsFileInputStream::Read(char* aBuf, uint
     }
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFileInputStream::ReadLine(nsACString& aLine, bool* aResult)
 {
-    nsresult rv = DoPendingOpen();
-    NS_ENSURE_SUCCESS(rv, rv);
-
     if (!mLineBuffer) {
       mLineBuffer = new nsLineBuffer<char>;
     }
     return NS_ReadLine(this, mLineBuffer.get(), aLine, aResult);
 }
 
 NS_IMETHODIMP
 nsFileInputStream::Seek(int32_t aWhence, int64_t aOffset)
 {
+  return SeekInternal(aWhence, aOffset);
+}
+
+nsresult
+nsFileInputStream::SeekInternal(int32_t aWhence, int64_t aOffset, bool aClearBuf)
+{
     nsresult rv = DoPendingOpen();
     NS_ENSURE_SUCCESS(rv, rv);
 
-    mLineBuffer = nullptr;
+    if (aClearBuf) {
+        mLineBuffer = nullptr;
+    }
     if (!mFD) {
         if (mBehaviorFlags & REOPEN_ON_REWIND) {
             rv = Open(mFile, mIOFlags, mPerm);
             NS_ENSURE_SUCCESS(rv, rv);
 
             // If the file was closed, and we do a relative seek, use the
             // position we cached when we closed the file to seek to the right
             // location.
@@ -694,63 +705,82 @@ nsPartialFileInputStream::Init(nsIFile* 
                                int32_t aPerm, int32_t aBehaviorFlags)
 {
     mStart = aStart;
     mLength = aLength;
     mPosition = 0;
 
     nsresult rv = nsFileInputStream::Init(aFile, aIOFlags, aPerm,
                                           aBehaviorFlags);
+
+    // aFile is a partial file, it must exist.
     NS_ENSURE_SUCCESS(rv, rv);
 
-    return nsFileInputStream::Seek(NS_SEEK_SET, mStart);
+    mDeferredSeek = true;
+
+    return rv;
 }
 
 NS_IMETHODIMP
 nsPartialFileInputStream::Tell(int64_t *aResult)
 {
     int64_t tell = 0;
-    nsresult rv = nsFileInputStream::Tell(&tell);
-    if (NS_SUCCEEDED(rv)) {
-        *aResult = tell - mStart;
-    }
+
+    nsresult rv = DoPendingSeek();
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = nsFileInputStream::Tell(&tell);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+
+    *aResult = tell - mStart;
     return rv;
 }
 
 NS_IMETHODIMP
 nsPartialFileInputStream::Available(uint64_t* aResult)
 {
     uint64_t available = 0;
-    nsresult rv = nsFileInputStream::Available(&available);
-    if (NS_SUCCEEDED(rv)) {
-        *aResult = TruncateSize(available);
-    }
+
+    nsresult rv = DoPendingSeek();
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    rv = nsFileInputStream::Available(&available);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    *aResult = TruncateSize(available);
     return rv;
 }
 
 NS_IMETHODIMP
 nsPartialFileInputStream::Read(char* aBuf, uint32_t aCount, uint32_t* aResult)
 {
+    nsresult rv = DoPendingSeek();
+    NS_ENSURE_SUCCESS(rv, rv);
+
     uint32_t readsize = (uint32_t) TruncateSize(aCount);
     if (readsize == 0 && mBehaviorFlags & CLOSE_ON_EOF) {
         Close();
         *aResult = 0;
         return NS_OK;
     }
 
-    nsresult rv = nsFileInputStream::Read(aBuf, readsize, aResult);
-    if (NS_SUCCEEDED(rv)) {
-        mPosition += readsize;
-    }
+    rv = nsFileInputStream::Read(aBuf, readsize, aResult);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    mPosition += readsize;
     return rv;
 }
 
 NS_IMETHODIMP
 nsPartialFileInputStream::Seek(int32_t aWhence, int64_t aOffset)
 {
+    nsresult rv = DoPendingSeek();
+    NS_ENSURE_SUCCESS(rv, rv);
+
     int64_t offset;
     switch (aWhence) {
         case NS_SEEK_SET:
             offset = mStart + aOffset;
             break;
         case NS_SEEK_CUR:
             offset = mStart + mPosition + aOffset;
             break;
@@ -760,20 +790,20 @@ nsPartialFileInputStream::Seek(int32_t a
         default:
             return NS_ERROR_ILLEGAL_VALUE;
     }
 
     if (offset < (int64_t)mStart || offset > (int64_t)(mStart + mLength)) {
         return NS_ERROR_INVALID_ARG;
     }
 
-    nsresult rv = nsFileInputStream::Seek(NS_SEEK_SET, offset);
-    if (NS_SUCCEEDED(rv)) {
-        mPosition = offset - mStart;
-    }
+    rv = nsFileInputStream::Seek(NS_SEEK_SET, offset);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    mPosition = offset - mStart;
     return rv;
 }
 
 void
 nsPartialFileInputStream::Serialize(InputStreamParams& aParams,
                                     FileDescriptorArray& aFileDescriptors)
 {
     // Serialize the base class first.
@@ -823,16 +853,29 @@ nsPartialFileInputStream::Deserialize(
     if (!mStart) {
       return true;
     }
 
     // XXX This is so broken. Main thread IO alert.
     return NS_SUCCEEDED(nsFileInputStream::Seek(NS_SEEK_SET, mStart));
 }
 
+nsresult
+nsPartialFileInputStream::DoPendingSeek()
+{
+    if (!mDeferredSeek) {
+       return NS_OK;
+    }
+
+    mDeferredSeek = false;
+
+    // This is the first time to open the file, don't clear mLinebuffer.
+    // mLineBuffer might be already initialized by ReadLine().
+    return nsFileInputStream::SeekInternal(NS_SEEK_SET, mStart, false);
+}
 ////////////////////////////////////////////////////////////////////////////////
 // nsFileOutputStream
 
 NS_IMPL_ISUPPORTS_INHERITED(nsFileOutputStream,
                             nsFileStreamBase,
                             nsIOutputStream,
                             nsIFileOutputStream)
 
--- a/netwerk/base/nsFileStreams.h
+++ b/netwerk/base/nsFileStreams.h
@@ -138,16 +138,18 @@ public:
     Create(nsISupports *aOuter, REFNSIID aIID, void **aResult);
 
 protected:
     virtual ~nsFileInputStream()
     {
         Close();
     }
 
+    nsresult SeekInternal(int32_t aWhence, int64_t aOffset, bool aClearBuf=true);
+
     nsAutoPtr<nsLineBuffer<char> > mLineBuffer;
 
     /**
      * The file being opened.
      */
     nsCOMPtr<nsIFile> mFile;
     /**
      * The IO flags passed to Init() for the file open.
@@ -179,39 +181,42 @@ class nsPartialFileInputStream : public 
 public:
     using nsFileInputStream::Init;
     using nsFileInputStream::Read;
     NS_DECL_ISUPPORTS_INHERITED
     NS_DECL_NSIPARTIALFILEINPUTSTREAM
     NS_DECL_NSIIPCSERIALIZABLEINPUTSTREAM
 
     nsPartialFileInputStream()
-      : mStart(0), mLength(0), mPosition(0)
+      : mStart(0), mLength(0), mPosition(0), mDeferredSeek(false)
     { }
 
     NS_IMETHOD Tell(int64_t *aResult) override;
     NS_IMETHOD Available(uint64_t *aResult) override;
     NS_IMETHOD Read(char* aBuf, uint32_t aCount, uint32_t* aResult) override;
     NS_IMETHOD Seek(int32_t aWhence, int64_t aOffset) override;
 
     static nsresult
     Create(nsISupports *aOuter, REFNSIID aIID, void **aResult);
 
 protected:
     ~nsPartialFileInputStream()
     { }
 
+    inline nsresult DoPendingSeek();
+
 private:
     uint64_t TruncateSize(uint64_t aSize) {
           return std::min<uint64_t>(mLength - mPosition, aSize);
     }
 
     uint64_t mStart;
     uint64_t mLength;
     uint64_t mPosition;
+    bool mDeferredSeek;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 
 class nsFileOutputStream : public nsFileStreamBase,
                            public nsIFileOutputStream
 {
 public: