Bug 588507: Skip caching if Content-Length is greater than eviction size. r=michal, a=betaN+
authorByron Milligan <bmilligan@mozilla.com>
Tue, 07 Sep 2010 15:39:28 -0700
changeset 52143 4905e9c69a529455247a63a233f115bf1dbc46ab
parent 52142 e9d58983c174ef8a1f451b5d8d1d583ff950a0ea
child 52144 1633b8d8a1844eb88b2cdac3ad99a490a4f7d558
push idunknown
push userunknown
push dateunknown
reviewersmichal, betaN
bugs588507
milestone2.0b6pre
Bug 588507: Skip caching if Content-Length is greater than eviction size. r=michal, a=betaN+
netwerk/cache/nsCacheEntry.h
netwerk/cache/nsCacheEntryDescriptor.cpp
netwerk/cache/nsCacheService.cpp
netwerk/cache/nsDiskCacheDevice.cpp
netwerk/cache/nsDiskCacheDevice.h
netwerk/cache/nsDiskCacheDeviceSQL.h
netwerk/cache/nsICacheEntryDescriptor.idl
netwerk/cache/nsMemoryCacheDevice.cpp
netwerk/cache/nsMemoryCacheDevice.h
netwerk/protocol/http/nsHttpChannel.cpp
xpcom/io/nsInputStreamTee.cpp
--- a/netwerk/cache/nsCacheEntry.h
+++ b/netwerk/cache/nsCacheEntry.h
@@ -99,16 +99,19 @@ public:
     const char *    GetDeviceID();
 
     /**
      * Data accessors
      */
     nsISupports *Data()                           { return mData; }
     void         SetData( nsISupports * data);
 
+    PRInt64  PredictedDataSize()                  { return mPredictedDataSize; }
+    void     SetPredictedDataSize(PRInt64 size)   { mPredictedDataSize = size; }
+
     PRUint32 DataSize()                           { return mDataSize; }
     void     SetDataSize( PRUint32  size)         { mDataSize = size; }
 
     void     TouchData();
     
     /**
      * Meta data accessors
      */
@@ -234,16 +237,17 @@ private:
 
     nsCString *             mKey;            // 4  // XXX ask scc about const'ness
     PRUint32                mFetchCount;     // 4
     PRUint32                mLastFetched;    // 4
     PRUint32                mLastModified;   // 4
     PRUint32                mLastValidated;  // 4
     PRUint32                mExpirationTime; // 4
     PRUint32                mFlags;          // 4
+    PRInt64                 mPredictedDataSize;  // Size given by ContentLength.
     PRUint32                mDataSize;       // 4
     nsCacheDevice *         mCacheDevice;    // 4
     nsCOMPtr<nsISupports>   mSecurityInfo;   // 
     nsISupports *           mData;           // strong ref
     nsCOMPtr<nsIThread>     mThread;
     nsCacheMetaData         mMetaData;       // 4
     PRCList                 mRequestQ;       // 8
     PRCList                 mDescriptorQ;    // 8
--- a/netwerk/cache/nsCacheEntryDescriptor.cpp
+++ b/netwerk/cache/nsCacheEntryDescriptor.cpp
@@ -181,16 +181,35 @@ NS_IMETHODIMP nsCacheEntryDescriptor::Is
     NS_ENSURE_ARG_POINTER(result);
     nsCacheServiceAutoLock lock;
     if (!mCacheEntry)  return NS_ERROR_NOT_AVAILABLE;
 
     *result = mCacheEntry->IsStreamData();
     return NS_OK;
 }
 
+NS_IMETHODIMP nsCacheEntryDescriptor::GetPredictedDataSize(PRInt64 *result)
+{
+    NS_ENSURE_ARG_POINTER(result);
+    nsCacheServiceAutoLock lock;
+    if (!mCacheEntry) return NS_ERROR_NOT_AVAILABLE;
+
+    *result = mCacheEntry->PredictedDataSize();
+    return NS_OK;
+}
+
+NS_IMETHODIMP nsCacheEntryDescriptor::SetPredictedDataSize(PRInt64
+                                                           predictedSize)
+{
+    nsCacheServiceAutoLock lock;
+    if (!mCacheEntry)  return NS_ERROR_NOT_AVAILABLE;
+
+    mCacheEntry->SetPredictedDataSize(predictedSize);
+    return NS_OK;
+}
 
 NS_IMETHODIMP nsCacheEntryDescriptor::GetDataSize(PRUint32 *result)
 {
     NS_ENSURE_ARG_POINTER(result);
     nsCacheServiceAutoLock lock;
     if (!mCacheEntry)  return NS_ERROR_NOT_AVAILABLE;
 
     *result = mCacheEntry->DataSize();
--- a/netwerk/cache/nsCacheService.cpp
+++ b/netwerk/cache/nsCacheService.cpp
@@ -1476,39 +1476,56 @@ nsCacheService::SearchCacheDevices(nsCSt
 
 
 nsCacheDevice *
 nsCacheService::EnsureEntryHasDevice(nsCacheEntry * entry)
 {
     nsCacheDevice * device = entry->CacheDevice();
     if (device)  return device;
 
+    PRInt64 predictedDataSize = entry->PredictedDataSize();
 #ifdef NECKO_DISK_CACHE
     if (entry->IsStreamData() && entry->IsAllowedOnDisk() && mEnableDiskDevice) {
         // this is the default
         if (!mDiskDevice) {
             (void)CreateDiskDevice();  // ignore the error (check for mDiskDevice instead)
         }
 
         if (mDiskDevice) {
+            // Bypass the cache if Content-Length says the entry will be too big
+            if (predictedDataSize != -1 &&
+                mDiskDevice->EntryIsTooBig(predictedDataSize)) {
+                nsresult rv = nsCacheService::DoomEntry(entry);
+                NS_ASSERTION(NS_SUCCEEDED(rv),"DoomEntry() failed.");
+                return nsnull;
+            }
+
             entry->MarkBinding();  // enter state of binding
             nsresult rv = mDiskDevice->BindEntry(entry);
             entry->ClearBinding(); // exit state of binding
             if (NS_SUCCEEDED(rv))
                 device = mDiskDevice;
         }
     }
 #endif // !NECKO_DISK_CACHE
-     
+
     // if we can't use mDiskDevice, try mMemoryDevice
     if (!device && mEnableMemoryDevice && entry->IsAllowedInMemory()) {        
         if (!mMemoryDevice) {
             (void)CreateMemoryDevice();  // ignore the error (check for mMemoryDevice instead)
         }
         if (mMemoryDevice) {
+            // Bypass the cache if Content-Length says entry will be too big
+            if (predictedDataSize != -1 &&
+                mMemoryDevice->EntryIsTooBig(predictedDataSize)) {
+                nsresult rv = nsCacheService::DoomEntry(entry);
+                NS_ASSERTION(NS_SUCCEEDED(rv),"DoomEntry() failed.");
+                return nsnull;
+            }
+
             entry->MarkBinding();  // enter state of binding
             nsresult rv = mMemoryDevice->BindEntry(entry);
             entry->ClearBinding(); // exit state of binding
             if (NS_SUCCEEDED(rv))
                 device = mMemoryDevice;
         }
     }
 
--- a/netwerk/cache/nsDiskCacheDevice.cpp
+++ b/netwerk/cache/nsDiskCacheDevice.cpp
@@ -771,17 +771,17 @@ nsDiskCacheDevice::OnDataSizeChange(nsCa
 
     NS_ASSERTION(binding->mRecord.ValidRecord(), "bad record");
 
     PRUint32  newSize = entry->DataSize() + deltaSize;
     PRUint32  newSizeK =  ((newSize + 0x3FF) >> 10);
 
     // If the new size is larger than max. file size or larger than
     // 1/8 the cache capacity (which is in KiB's), doom the entry and abort
-    if ((newSize > kMaxDataFileSize) || (newSizeK > mCacheCapacity/8)) {
+    if (EntryIsTooBig(newSize)) {
 #ifdef DEBUG
         nsresult rv =
 #endif
             nsCacheService::DoomEntry(entry);
         NS_ASSERTION(NS_SUCCEEDED(rv),"DoomEntry() failed.");
         return NS_ERROR_ABORT;
     }
 
@@ -854,16 +854,23 @@ nsDiskCacheDevice::Visit(nsICacheVisitor
     if (keepGoing) {
         EntryInfoVisitor  infoVisitor(&mCacheMap, visitor);
         return mCacheMap.VisitRecords(&infoVisitor);
     }
 
     return NS_OK;
 }
 
+// Max allowed size for an entry is currently MIN(5MB, 1/8 CacheCapacity)
+bool
+nsDiskCacheDevice::EntryIsTooBig(PRInt64 entrySize)
+{
+    return entrySize > kMaxDataFileSize
+           || entrySize > (static_cast<PRInt64>(mCacheCapacity) * 1024 / 8);
+}
 
 nsresult
 nsDiskCacheDevice::EvictEntries(const char * clientID)
 {
     CACHE_LOG_DEBUG(("CACHE: disk EvictEntries [%s]\n", clientID));
 
     if (!Initialized())  return NS_ERROR_NOT_INITIALIZED;
     nsresult  rv;
--- a/netwerk/cache/nsDiskCacheDevice.h
+++ b/netwerk/cache/nsDiskCacheDevice.h
@@ -82,16 +82,18 @@ public:
                                             nsIFile **        result);
 
     virtual nsresult        OnDataSizeChange(nsCacheEntry * entry, PRInt32 deltaSize);
     
     virtual nsresult        Visit(nsICacheVisitor * visitor);
 
     virtual nsresult        EvictEntries(const char * clientID);
 
+    bool                    EntryIsTooBig(PRInt64 entrySize);
+
     /**
      * Preference accessors
      */
     void                    SetCacheParentDirectory(nsILocalFile * parentDir);
     void                    SetCapacity(PRUint32  capacity);
 
 /* private: */
 
--- a/netwerk/cache/nsDiskCacheDeviceSQL.h
+++ b/netwerk/cache/nsDiskCacheDeviceSQL.h
@@ -129,17 +129,16 @@ public:
                                           nsIFile **        result);
 
   virtual nsresult        OnDataSizeChange(nsCacheEntry * entry, PRInt32 deltaSize);
   
   virtual nsresult        Visit(nsICacheVisitor * visitor);
 
   virtual nsresult        EvictEntries(const char * clientID);
 
-
   /* Entry ownership */
   nsresult                GetOwnerDomains(const char *        clientID,
                                           PRUint32 *          count,
                                           char ***            domains);
   nsresult                GetOwnerURIs(const char *           clientID,
                                        const nsACString &     ownerDomain,
                                        PRUint32 *             count,
                                        char ***               uris);
--- a/netwerk/cache/nsICacheEntryDescriptor.idl
+++ b/netwerk/cache/nsICacheEntryDescriptor.idl
@@ -98,17 +98,25 @@ interface nsICacheEntryDescriptor : nsIC
     nsIOutputStream openOutputStream(in unsigned long offset);
 
     /**
      * Get/set the cache data element.  This will fail if the cache entry
      * IS stream based.  The cache entry holds a strong reference to this
      * object.  The object will be released when the cache entry is destroyed.
      */
     attribute nsISupports cacheElement;
-     
+    
+    /**
+      * Stores the Content-Length specified in the HTTP header for this
+      * entry. Checked before we write to the cache entry, to prevent ever
+      * taking up space in the cache for an entry that we know up front 
+      * is going to have to be evicted anyway. See bug 588507.
+      */
+    attribute PRInt64    predictedDataSize;
+
     /**
      * Get the access granted to this descriptor.  See nsICache.idl for the
      * definitions of the access modes and a thorough description of their
      * corresponding meanings.
      */
     readonly attribute nsCacheAccessMode accessGranted;
     
     /**
--- a/netwerk/cache/nsMemoryCacheDevice.cpp
+++ b/netwerk/cache/nsMemoryCacheDevice.cpp
@@ -289,24 +289,30 @@ nsMemoryCacheDevice::OpenOutputStreamFor
 
 nsresult
 nsMemoryCacheDevice::GetFileForEntry( nsCacheEntry *    entry,
                                       nsIFile **        result )
 {
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+bool
+nsMemoryCacheDevice::EntryIsTooBig(PRInt64 entrySize)
+{
+    return entrySize > mSoftLimit;
+}
+
 
 nsresult
 nsMemoryCacheDevice::OnDataSizeChange( nsCacheEntry * entry, PRInt32 deltaSize)
 {
     if (entry->IsStreamData()) {
         // we have the right to refuse or pre-evict
         PRUint32  newSize = entry->DataSize() + deltaSize;
-        if ((PRInt32) newSize > mSoftLimit) {
+        if (EntryIsTooBig(newSize)) {
 #ifdef DEBUG
             nsresult rv =
 #endif
                 nsCacheService::DoomEntry(entry);
             NS_ASSERTION(NS_SUCCEEDED(rv),"DoomEntry() failed.");
             return NS_ERROR_ABORT;
         }
     }
--- a/netwerk/cache/nsMemoryCacheDevice.h
+++ b/netwerk/cache/nsMemoryCacheDevice.h
@@ -83,17 +83,18 @@ public:
 
     virtual nsresult OnDataSizeChange( nsCacheEntry * entry, PRInt32 deltaSize );
 
     virtual nsresult Visit( nsICacheVisitor * visitor );
 
     virtual nsresult EvictEntries(const char * clientID);
     
     void             SetCapacity(PRInt32  capacity);
- 
+
+    bool             EntryIsTooBig(PRInt64 entrySize);
 private:
     friend class nsMemoryCacheDeviceInfo;
     enum      { DELETE_ENTRY        = PR_TRUE,
                 DO_NOT_DELETE_ENTRY = PR_FALSE };
 
     void      AdjustMemoryLimits( PRInt32  softLimit, PRInt32  hardLimit);
     void      EvictEntry( nsCacheEntry * entry , PRBool deleteEntry);
     void      EvictEntriesIfNecessary();
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -2793,16 +2793,23 @@ nsHttpChannel::InitCacheEntry()
 
     // Set the expiration time for this cache entry
     rv = UpdateExpirationTime();
     if (NS_FAILED(rv)) return rv;
 
     rv = AddCacheEntryHeaders(mCacheEntry);
     if (NS_FAILED(rv)) return rv;
 
+    // set predicted size of entry so we do not store anything that will exceed
+    // the max entry size limit
+    PRInt64 predictedDataSize;
+    GetContentLength(&predictedDataSize);
+    rv = mCacheEntry->SetPredictedDataSize(predictedDataSize);
+    if (NS_FAILED(rv)) return rv;
+
     mInitedCacheEntry = PR_TRUE;
     return NS_OK;
 }
 
 
 nsresult
 nsHttpChannel::InitOfflineCacheEntry()
 {
--- a/xpcom/io/nsInputStreamTee.cpp
+++ b/xpcom/io/nsInputStreamTee.cpp
@@ -41,85 +41,101 @@
 
 #include "nsIInputStreamTee.h"
 #include "nsIInputStream.h"
 #include "nsIOutputStream.h"
 #include "nsCOMPtr.h"
 #include "nsAutoPtr.h"
 #include "nsIEventTarget.h"
 #include "nsThreadUtils.h"
+#include <prlock.h>
+#include "nsAutoLock.h"
 
 #ifdef PR_LOGGING
 static PRLogModuleInfo* gInputStreamTeeLog = PR_NewLogModule("nsInputStreamTee");
 #define LOG(args) PR_LOG(gInputStreamTeeLog, PR_LOG_DEBUG, args)
 #else
 #define LOG(args)
 #endif
 
 class nsInputStreamTee : public nsIInputStreamTee
 {
 public:
     NS_DECL_ISUPPORTS
     NS_DECL_NSIINPUTSTREAM
     NS_DECL_NSIINPUTSTREAMTEE
 
     nsInputStreamTee();
+    bool SinkIsValid();
+    void InvalidateSink();
 
 private:
-    ~nsInputStreamTee() {}
+    ~nsInputStreamTee() { if (mLock) PR_DestroyLock(mLock); }
 
     nsresult TeeSegment(const char *buf, PRUint32 count);
-
+    
     static NS_METHOD WriteSegmentFun(nsIInputStream *, void *, const char *,
                                      PRUint32, PRUint32, PRUint32 *);
 
 private:
     nsCOMPtr<nsIInputStream>  mSource;
     nsCOMPtr<nsIOutputStream> mSink;
     nsCOMPtr<nsIEventTarget>  mEventTarget;
     nsWriteSegmentFun         mWriter;  // for implementing ReadSegments
-    void                     *mClosure; // for implementing ReadSegments
+    void                      *mClosure; // for implementing ReadSegments
+    PRLock                    *mLock; // synchronize access to mSinkIsValid
+    bool                      mSinkIsValid; // False if TeeWriteEvent fails 
 };
 
 class nsInputStreamTeeWriteEvent : public nsRunnable {
 public:
+    // aTee's lock is held across construction of this object
     nsInputStreamTeeWriteEvent(const char *aBuf, PRUint32 aCount,
-                               nsIOutputStream *aSink)
+                               nsIOutputStream  *aSink, 
+                               nsInputStreamTee *aTee)
     {
         // copy the buffer - will be free'd by dtor
         mBuf = (char *)malloc(aCount);
         if (mBuf) memcpy(mBuf, (char *)aBuf, aCount);
         mCount = aCount;
         mSink = aSink;
         PRBool isNonBlocking;
         mSink->IsNonBlocking(&isNonBlocking);
         NS_ASSERTION(isNonBlocking == PR_FALSE, "mSink is nonblocking");
+        mTee = aTee;
     }
 
     NS_IMETHOD Run()
     {
         if (!mBuf) {
             NS_WARNING("nsInputStreamTeeWriteEvent::Run() "
-                    "memory not allocated\n");
+                       "memory not allocated\n");
             return NS_OK;
         }
         NS_ABORT_IF_FALSE(mSink, "mSink is null!");
 
+        //  The output stream could have been invalidated between when 
+        //  this event was dispatched and now, so check before writing.
+        if (!mTee->SinkIsValid()) {
+            return NS_OK; 
+        }
+
         LOG(("nsInputStreamTeeWriteEvent::Run() [%p]"
-                "will write %u bytes to %p\n",
-                this, mCount, mSink.get()));
+             "will write %u bytes to %p\n",
+              this, mCount, mSink.get()));
 
         PRUint32 totalBytesWritten = 0;
         while (mCount) {
             nsresult rv;
             PRUint32 bytesWritten = 0;
             rv = mSink->Write(mBuf + totalBytesWritten, mCount, &bytesWritten);
             if (NS_FAILED(rv)) {
                 LOG(("nsInputStreamTeeWriteEvent::Run[%p] error %x in writing",
                      this,rv));
+                mTee->InvalidateSink();
                 break;
             }
             totalBytesWritten += bytesWritten;
             NS_ASSERTION(bytesWritten <= mCount, "wrote too much");
             mCount -= bytesWritten;
         }
         return NS_OK;
     }
@@ -130,56 +146,76 @@ protected:
         if (mBuf) free(mBuf);
         mBuf = nsnull;
     }
     
 private:
     char *mBuf;
     PRUint32 mCount;
     nsCOMPtr<nsIOutputStream> mSink;
+    // back pointer to the tee that created this runnable
+    nsRefPtr<nsInputStreamTee> mTee;
 };
 
-nsInputStreamTee::nsInputStreamTee()
+nsInputStreamTee::nsInputStreamTee(): mLock(nsnull)
+                                    , mSinkIsValid(true)
+{
+}
+
+bool
+nsInputStreamTee::SinkIsValid()
 {
+    nsAutoLock lock(mLock); 
+    return mSinkIsValid; 
+}
+
+void
+nsInputStreamTee::InvalidateSink()
+{
+    nsAutoLock lock(mLock);
+    mSinkIsValid = false;
 }
 
 nsresult
 nsInputStreamTee::TeeSegment(const char *buf, PRUint32 count)
 {
-    if (!mSink)
-        return NS_OK; // nothing to do
-
-    if (mEventTarget) {
+    if (!mSink) return NS_OK; // nothing to do
+    if (mLock) { // asynchronous case
+        NS_ASSERTION(mEventTarget, "mEventTarget is null, mLock is not null.");
+        if (!SinkIsValid()) {
+            return NS_OK; // nothing to do
+        }
         nsRefPtr<nsIRunnable> event =
-            new nsInputStreamTeeWriteEvent(buf, count, mSink);
+            new nsInputStreamTeeWriteEvent(buf, count, mSink, this);
         NS_ENSURE_TRUE(event, NS_ERROR_OUT_OF_MEMORY);
         LOG(("nsInputStreamTee::TeeSegment [%p] dispatching write %u bytes\n",
-                this, count));
+              this, count));
         return mEventTarget->Dispatch(event, NS_DISPATCH_NORMAL);
+    } else { // synchronous case
+        NS_ASSERTION(!mEventTarget, "mEventTarget is not null, mLock is null.");
+        nsresult rv;
+        PRUint32 totalBytesWritten = 0;
+        while (count) {
+            PRUint32 bytesWritten = 0;
+            rv = mSink->Write(buf + totalBytesWritten, count, &bytesWritten);
+            if (NS_FAILED(rv)) {
+                // ok, this is not a fatal error... just drop our reference to mSink
+                // and continue on as if nothing happened.
+                NS_WARNING("Write failed (non-fatal)");
+                // catch possible misuse of the input stream tee
+                NS_ASSERTION(rv != NS_BASE_STREAM_WOULD_BLOCK, "sink must be a blocking stream");
+                mSink = 0;
+                break;
+            }
+            totalBytesWritten += bytesWritten;
+            NS_ASSERTION(bytesWritten <= count, "wrote too much");
+            count -= bytesWritten;
+        }
+        return NS_OK;
     }
-
-    nsresult rv;
-    PRUint32 totalBytesWritten = 0;
-    while (count) {
-        PRUint32 bytesWritten = 0;
-        rv = mSink->Write(buf + totalBytesWritten, count, &bytesWritten);
-        if (NS_FAILED(rv)) {
-            // ok, this is not a fatal error... just drop our reference to mSink
-            // and continue on as if nothing happened.
-            NS_WARNING("Write failed (non-fatal)");
-            // catch possible misuse of the input stream tee
-            NS_ASSERTION(rv != NS_BASE_STREAM_WOULD_BLOCK, "sink must be a blocking stream");
-            mSink = 0;
-            break;
-        }
-        totalBytesWritten += bytesWritten;
-        NS_ASSERTION(bytesWritten <= count, "wrote too much");
-        count -= bytesWritten;
-    }
-    return NS_OK;
 }
 
 NS_METHOD
 nsInputStreamTee::WriteSegmentFun(nsIInputStream *in, void *closure, const char *fromSegment,
                                   PRUint32 offset, PRUint32 count, PRUint32 *writeCount)
 {
     nsInputStreamTee *tee = reinterpret_cast<nsInputStreamTee *>(closure);
 
@@ -188,20 +224,19 @@ nsInputStreamTee::WriteSegmentFun(nsIInp
         NS_ASSERTION((NS_FAILED(rv) ? (*writeCount == 0) : PR_TRUE),
                 "writer returned an error with non-zero writeCount");
         return rv;
     }
 
     return tee->TeeSegment(fromSegment, *writeCount);
 }
 
-NS_IMPL_ISUPPORTS2(nsInputStreamTee,
-                   nsIInputStreamTee,
-                   nsIInputStream)
-
+NS_IMPL_THREADSAFE_ISUPPORTS2(nsInputStreamTee,
+                              nsIInputStreamTee,
+                              nsIInputStream)
 NS_IMETHODIMP
 nsInputStreamTee::Close()
 {
     NS_ENSURE_TRUE(mSource, NS_ERROR_NOT_INITIALIZED);
     nsresult rv = mSource->Close();
     mSource = 0;
     mSink = 0;
     return rv;
@@ -282,16 +317,24 @@ nsInputStreamTee::GetSink(nsIOutputStrea
     NS_IF_ADDREF(*sink = mSink);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsInputStreamTee::SetEventTarget(nsIEventTarget *anEventTarget)
 {
     mEventTarget = anEventTarget;
+    if (mEventTarget) {
+        // Only need synchronization if this is an async tee
+        mLock = PR_NewLock();
+        if (!mLock) {
+            NS_ERROR("Failed to allocate lock for nsInputStreamTee");
+            return NS_ERROR_OUT_OF_MEMORY;
+        }
+    }
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsInputStreamTee::GetEventTarget(nsIEventTarget **anEventTarget)
 {
     NS_IF_ADDREF(*anEventTarget = mEventTarget);
     return NS_OK;