Bug 588507: Skip caching if Content-Length is greater than eviction size. r=michal, a=betaN+

netwerk/cache/nsCacheEntry.h

@@ -104,6 +104,9 @@ public:
     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; }
 
@@ -239,6 +242,7 @@ private:
     PRUint32                mLastValidated;  // 4
     PRUint32                mExpirationTime; // 4
     PRUint32                mFlags;          // 4
+    PRInt64                 mPredictedDataSize;  // Size given by ContentLength.
     PRUint32                mDataSize;       // 4
     nsCacheDevice *         mCacheDevice;    // 4
     nsCOMPtr<nsISupports>   mSecurityInfo;   // 

netwerk/cache/nsCacheEntryDescriptor.cpp

@@ -186,6 +186,25 @@ NS_IMETHODIMP nsCacheEntryDescriptor::IsStreamBased(PRBool *result)
     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)
 {

netwerk/cache/nsCacheService.cpp

@@ -1481,6 +1481,7 @@ 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
@@ -1489,6 +1490,14 @@ nsCacheService::EnsureEntryHasDevice(nsCacheEntry * entry)
         }
 
         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
@@ -1497,13 +1506,21 @@ nsCacheService::EnsureEntryHasDevice(nsCacheEntry * entry)
         }
     }
 #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

netwerk/cache/nsDiskCacheDevice.cpp

@@ -776,7 +776,7 @@ nsDiskCacheDevice::OnDataSizeChange(nsCacheEntry * entry, PRInt32 deltaSize)
 
     // 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
@@ -859,6 +859,13 @@ nsDiskCacheDevice::Visit(nsICacheVisitor * visitor)
     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)

netwerk/cache/nsDiskCacheDevice.h

@@ -87,6 +87,8 @@ public:
 
     virtual nsresult        EvictEntries(const char * clientID);
 
+    bool                    EntryIsTooBig(PRInt64 entrySize);
+
     /**
      * Preference accessors
      */

netwerk/cache/nsDiskCacheDeviceSQL.h

@@ -134,7 +134,6 @@ public:
 
   virtual nsresult        EvictEntries(const char * clientID);
 
-
   /* Entry ownership */
   nsresult                GetOwnerDomains(const char *        clientID,
                                           PRUint32 *          count,

netwerk/cache/nsICacheEntryDescriptor.idl

@@ -103,7 +103,15 @@ interface nsICacheEntryDescriptor : nsICacheEntryInfo
      * 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

netwerk/cache/nsMemoryCacheDevice.cpp

@@ -294,6 +294,12 @@ nsMemoryCacheDevice::GetFileForEntry( nsCacheEntry *    entry,
     return NS_ERROR_NOT_IMPLEMENTED;
 }
 
+bool
+nsMemoryCacheDevice::EntryIsTooBig(PRInt64 entrySize)
+{
+    return entrySize > mSoftLimit;
+}
+
 
 nsresult
 nsMemoryCacheDevice::OnDataSizeChange( nsCacheEntry * entry, PRInt32 deltaSize)
@@ -301,7 +307,7 @@ 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

netwerk/cache/nsMemoryCacheDevice.h

@@ -88,7 +88,8 @@ public:
     virtual nsresult EvictEntries(const char * clientID);
     
     void             SetCapacity(PRInt32  capacity);
- 
+
+    bool             EntryIsTooBig(PRInt64 entrySize);
 private:
     friend class nsMemoryCacheDeviceInfo;
     enum      { DELETE_ENTRY        = PR_TRUE,

netwerk/protocol/http/nsHttpChannel.cpp

@@ -2798,6 +2798,13 @@ nsHttpChannel::InitCacheEntry()
     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;
 }

xpcom/io/nsInputStreamTee.cpp

@@ -46,6 +46,8 @@
 #include "nsAutoPtr.h"
 #include "nsIEventTarget.h"
 #include "nsThreadUtils.h"
+#include <prlock.h>
+#include "nsAutoLock.h"
 
 #ifdef PR_LOGGING
 static PRLogModuleInfo* gInputStreamTeeLog = PR_NewLogModule("nsInputStreamTee");
@@ -62,12 +64,14 @@ public:
     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 *);
 
@@ -76,13 +80,17 @@ private:
     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);
@@ -92,20 +100,27 @@ public:
         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) {
@@ -115,6 +130,7 @@ public:
             if (NS_FAILED(rv)) {
                 LOG(("nsInputStreamTeeWriteEvent::Run[%p] error %x in writing",
                      this,rv));
+                mTee->InvalidateSink();
                 break;
             }
             totalBytesWritten += bytesWritten;
@@ -135,46 +151,66 @@ 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);
-    }
-
-    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;
+    } 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;
         }
-        totalBytesWritten += bytesWritten;
-        NS_ASSERTION(bytesWritten <= count, "wrote too much");
-        count -= bytesWritten;
+        return NS_OK;
     }
-    return NS_OK;
 }
 
 NS_METHOD
@@ -193,10 +229,9 @@ nsInputStreamTee::WriteSegmentFun(nsIInputStream *in, void *closure, const char
     return tee->TeeSegment(fromSegment, *writeCount);
 }
 
-NS_IMPL_ISUPPORTS2(nsInputStreamTee,
-                   nsIInputStreamTee,
-                   nsIInputStream)
-
+NS_IMPL_THREADSAFE_ISUPPORTS2(nsInputStreamTee,
+                              nsIInputStreamTee,
+                              nsIInputStream)
 NS_IMETHODIMP
 nsInputStreamTee::Close()
 {
@@ -287,6 +322,14 @@ 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;
 }