Bug 1032254 - Generic way to pin reasource in the HTTP cache, r=michal

* * *
Bug NNNNNNN - message, r=reviewer

--HG--
rename : netwerk/test/unit/test_cache2-28-concurrent_read_resumable_entry_size_zero.js => netwerk/test/unit/test_cache2-29a-concurrent_read_resumable_entry_size_zero.js
rename : netwerk/test/unit/test_cache2-29-concurrent_read_non-resumable_entry_size_zero.js => netwerk/test/unit/test_cache2-29b-concurrent_read_non-resumable_entry_size_zero.js

Author: mayhemer

netwerk/base/nsICachingChannel.idl

@@ -17,7 +17,7 @@ interface nsIFile;
  *   3) Support for uniquely identifying cached data in cases when the URL
  *      is insufficient (e.g., HTTP form submission).
  */
-[scriptable, uuid(436b939d-e391-48e5-ba64-ab0e496e3400)]
+[scriptable, uuid(dd1d6122-5ecf-4fe4-8f0f-995e7ab3121a)]
 interface nsICachingChannel : nsICacheInfoChannel
 {
     /**
@@ -53,6 +53,11 @@ interface nsICachingChannel : nsICacheInfoChannel
      */
     attribute boolean cacheOnlyMetadata;
 
+    /**
+     * Tells the channel to use the pinning storage.
+     */
+    attribute boolean pin;
+
     /**************************************************************************
      * Caching channel specific load flags:
      */

netwerk/cache2/AppCacheStorage.cpp

@@ -25,7 +25,7 @@ NS_IMPL_ISUPPORTS_INHERITED0(AppCacheStorage, CacheStorage)
 
 AppCacheStorage::AppCacheStorage(nsILoadContextInfo* aInfo,
                                  nsIApplicationCache* aAppCache)
-: CacheStorage(aInfo, true /* disk */, false /* lookup app cache */, false /* skip size check */)
+: CacheStorage(aInfo, true /* disk */, false /* lookup app cache */, false /* skip size check */, false /* pin */)
 , mAppCache(aAppCache)
 {
   MOZ_COUNT_CTOR(AppCacheStorage);

netwerk/cache2/CacheEntry.cpp

@@ -87,6 +87,8 @@ CacheEntry::Callback::Callback(CacheEntry* aEntry,
 , mRecheckAfterWrite(false)
 , mNotWanted(false)
 , mSecret(aSecret)
+, mDoomWhenFoundPinned(false)
+, mDoomWhenFoundNonPinned(false)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
@@ -96,6 +98,22 @@ CacheEntry::Callback::Callback(CacheEntry* aEntry,
   mEntry->AddHandleRef();
 }
 
+CacheEntry::Callback::Callback(CacheEntry* aEntry, bool aDoomWhenFoundInPinStatus)
+: mEntry(aEntry)
+, mReadOnly(false)
+, mRevalidating(false)
+, mCheckOnAnyThread(true)
+, mRecheckAfterWrite(false)
+, mNotWanted(false)
+, mSecret(false)
+, mDoomWhenFoundPinned(aDoomWhenFoundInPinStatus == true)
+, mDoomWhenFoundNonPinned(aDoomWhenFoundInPinStatus == false)
+{
+  MOZ_COUNT_CTOR(CacheEntry::Callback);
+  MOZ_ASSERT(mEntry->HandlesCount());
+  mEntry->AddHandleRef();
+}
+
 CacheEntry::Callback::Callback(CacheEntry::Callback const &aThat)
 : mEntry(aThat.mEntry)
 , mCallback(aThat.mCallback)
@@ -106,6 +124,8 @@ CacheEntry::Callback::Callback(CacheEntry::Callback const &aThat)
 , mRecheckAfterWrite(aThat.mRecheckAfterWrite)
 , mNotWanted(aThat.mNotWanted)
 , mSecret(aThat.mSecret)
+, mDoomWhenFoundPinned(aThat.mDoomWhenFoundPinned)
+, mDoomWhenFoundNonPinned(aThat.mDoomWhenFoundNonPinned)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
@@ -136,6 +156,20 @@ void CacheEntry::Callback::ExchangeEntry(CacheEntry* aEntry)
   mEntry = aEntry;
 }
 
+bool CacheEntry::Callback::DeferDoom(bool *aDoom) const
+{
+  MOZ_ASSERT(mEntry->mPinningKnown);
+
+  if (MOZ_UNLIKELY(mDoomWhenFoundNonPinned) || MOZ_UNLIKELY(mDoomWhenFoundPinned)) {
+    *aDoom = (MOZ_UNLIKELY(mDoomWhenFoundNonPinned) && MOZ_LIKELY(!mEntry->mPinned)) ||
+             (MOZ_UNLIKELY(mDoomWhenFoundPinned) && MOZ_UNLIKELY(mEntry->mPinned));
+
+    return true;
+  }
+
+  return false;
+}
+
 nsresult CacheEntry::Callback::OnCheckThread(bool *aOnCheckThread) const
 {
   if (!mCheckOnAnyThread) {
@@ -164,7 +198,8 @@ CacheEntry::CacheEntry(const nsACString& aStorageID,
                        nsIURI* aURI,
                        const nsACString& aEnhanceID,
                        bool aUseDisk,
-                       bool aSkipSizeCheck)
+                       bool aSkipSizeCheck,
+                       bool aPin)
 : mFrecency(0)
 , mSortingExpirationTime(uint32_t(-1))
 , mLock("CacheEntry")
@@ -174,10 +209,12 @@ CacheEntry::CacheEntry(const nsACString& aStorageID,
 , mStorageID(aStorageID)
 , mUseDisk(aUseDisk)
 , mSkipSizeCheck(aSkipSizeCheck)
-, mIsDoomed(false)
 , mSecurityInfoLoaded(false)
 , mPreventCallbacks(false)
 , mHasData(false)
+, mPinned(aPin)
+, mPinningKnown(false)
+, mIsDoomed(false)
 , mState(NOTLOADED)
 , mRegistration(NEVERREGISTERED)
 , mWriter(nullptr)
@@ -350,16 +387,18 @@ bool CacheEntry::Load(bool aTruncate, bool aPriority)
     if (NS_SUCCEEDED(CacheIndex::HasEntry(fileKey, &status))) {
       switch (status) {
       case CacheIndex::DOES_NOT_EXIST:
-        LOG(("  entry doesn't exist according information from the index, truncating"));
+        // Doesn't apply to memory-only entries, Load() is called only once for them
+        // and never again for their session lifetime.
         if (!aTruncate && mUseDisk) {
+          LOG(("  entry doesn't exist according information from the index, truncating"));
           reportMiss = true;
+          aTruncate = true;
         }
-        aTruncate = true;
         break;
       case CacheIndex::EXISTS:
       case CacheIndex::DO_NOT_KNOW:
         if (!mUseDisk) {
-          LOG(("  entry open as memory-only, but there is (status=%d) a file, dooming it", status));
+          LOG(("  entry open as memory-only, but there is a file, status=%d, dooming it", status));
           CacheFileIOManager::DoomFileByKey(fileKey, nullptr);
         }
         break;
@@ -376,6 +415,7 @@ bool CacheEntry::Load(bool aTruncate, bool aPriority)
     // mLoadStart will be used to calculate telemetry of life-time of this entry.
     // Low resulution is then enough.
     mLoadStart = TimeStamp::NowLoRes();
+    mPinningKnown = true;
   } else {
     mLoadStart = TimeStamp::Now();
   }
@@ -395,6 +435,7 @@ bool CacheEntry::Load(bool aTruncate, bool aPriority)
                        !mUseDisk,
                        mSkipSizeCheck,
                        aPriority,
+                       mPinned,
                        directLoad ? nullptr : this);
     }
 
@@ -432,9 +473,10 @@ NS_IMETHODIMP CacheEntry::OnFileReady(nsresult aResult, bool aIsNew)
   }
 
   // OnFileReady, that is the only code that can transit from LOADING
-  // to any follow-on state, can only be invoked ones on an entry,
-  // thus no need to lock.  Until this moment there is no consumer that
-  // could manipulate the entry state.
+  // to any follow-on state and can only be invoked ones on an entry.
+  // Until this moment there is no consumer that could manipulate
+  // the entry state.
+
   mozilla::MutexAutoLock lock(mLock);
 
   MOZ_ASSERT(mState == LOADING);
@@ -445,6 +487,10 @@ NS_IMETHODIMP CacheEntry::OnFileReady(nsresult aResult, bool aIsNew)
 
   mFileStatus = aResult;
 
+  mPinned = mFile->IsPinned();;
+  mPinningKnown = true;
+  LOG(("  pinning=%d", mPinned));
+
   if (mState == READY) {
     mHasData = true;
 
@@ -456,6 +502,7 @@ NS_IMETHODIMP CacheEntry::OnFileReady(nsresult aResult, bool aIsNew)
   }
 
   InvokeCallbacks();
+
   return NS_OK;
 }
 
@@ -483,13 +530,21 @@ already_AddRefed<CacheEntryHandle> CacheEntry::ReopenTruncated(bool aMemoryOnly,
   RefPtr<CacheEntryHandle> handle;
   RefPtr<CacheEntry> newEntry;
   {
+    if (mPinned) {
+      MOZ_ASSERT(mUseDisk);
+      // We want to pin even no-store entries (the case we recreate a disk entry as
+      // a memory-only entry.)
+      aMemoryOnly = false;
+    }
+
     mozilla::MutexAutoUnlock unlock(mLock);
 
     // The following call dooms this entry (calls DoomAlreadyRemoved on us)
     nsresult rv = CacheStorageService::Self()->AddStorageEntry(
       GetStorageID(), GetURI(), GetEnhanceID(),
       mUseDisk && !aMemoryOnly,
       mSkipSizeCheck,
+      mPinned,
       true, // always create
       true, // truncate existing (this one)
       getter_AddRefs(handle));
@@ -577,6 +632,8 @@ bool CacheEntry::InvokeCallbacks(bool aReadOnly)
 {
   mLock.AssertCurrentThreadOwns();
 
+  RefPtr<CacheEntryHandle> recreatedHandle;
+
   uint32_t i = 0;
   while (i < mCallbacks.Length()) {
     if (mPreventCallbacks) {
@@ -589,6 +646,18 @@ bool CacheEntry::InvokeCallbacks(bool aReadOnly)
       return false;
     }
 
+    bool recreate;
+    if (mCallbacks[i].DeferDoom(&recreate)) {
+      mCallbacks.RemoveElementAt(i);
+      if (!recreate) {
+        continue;
+      }
+
+      LOG(("  defer doom marker callback hit positive, recreating"));
+      recreatedHandle = ReopenTruncated(!mUseDisk, nullptr);
+      break;
+    }
+
     if (mCallbacks[i].mReadOnly != aReadOnly) {
       // Callback is not r/w or r/o, go to another one in line
       ++i;
@@ -625,6 +694,12 @@ bool CacheEntry::InvokeCallbacks(bool aReadOnly)
     }
   }
 
+  if (recreatedHandle) {
+    // Must be released outside of the lock, enters InvokeCallback on the new entry
+    mozilla::MutexAutoUnlock unlock(mLock);
+    recreatedHandle = nullptr;
+  }
+
   return true;
 }
 
@@ -991,6 +1066,13 @@ NS_IMETHODIMP CacheEntry::GetIsForcedValid(bool *aIsForcedValid)
 {
   NS_ENSURE_ARG(aIsForcedValid);
 
+  MOZ_ASSERT(mState > LOADING);
+
+  if (mPinned) {
+    *aIsForcedValid = true;
+    return NS_OK;
+  }
+
   nsAutoCString key;
 
   nsresult rv = HashingKeyWithStorage(key);
@@ -1442,6 +1524,28 @@ void CacheEntry::SetRegistered(bool aRegistered)
   }
 }
 
+bool CacheEntry::DeferOrBypassRemovalOnPinStatus(bool aPinned)
+{
+  LOG(("CacheEntry::DeferOrBypassRemovalOnPinStatus [this=%p]", this));
+
+  mozilla::MutexAutoLock lock(mLock);
+
+  if (mPinningKnown) {
+    LOG(("  pinned=%d, caller=%d", mPinned, aPinned));
+    // Bypass when the pin status of this entry doesn't match the pin status
+    // caller wants to remove
+    return mPinned != aPinned;
+  }
+
+  LOG(("  pinning unknown, caller=%d", aPinned));
+  // Oterwise, remember to doom after the status is determined for any
+  // callback opening the entry after this point...
+  Callback c(this, aPinned);
+  RememberCallback(c);
+  // ...and always bypass
+  return true;
+}
+
 bool CacheEntry::Purge(uint32_t aWhat)
 {
   LOG(("CacheEntry::Purge [this=%p, what=%d]", this, aWhat));
@@ -1523,6 +1627,10 @@ void CacheEntry::DoomAlreadyRemoved()
 
   mIsDoomed = true;
 
+  // Pretend pinning is know.  This entry is now doomed for good, so don't
+  // bother with defering doom because of unknown pinning state any more.
+  mPinningKnown = true;
+
   // This schedules dooming of the file, dooming is ensured to happen
   // sooner than demand to open the same file made after this point
   // so that we don't get this file for any newer opened entry(s).

netwerk/cache2/CacheEntry.h

@@ -55,7 +55,7 @@ class CacheEntry final : public nsICacheEntry
   NS_DECL_NSIRUNNABLE
 
   CacheEntry(const nsACString& aStorageID, nsIURI* aURI, const nsACString& aEnhanceID,
-             bool aUseDisk, bool aSkipSizeCheck);
+             bool aUseDisk, bool aSkipSizeCheck, bool aPin);
 
   void AsyncOpen(nsICacheEntryOpenCallback* aCallback, uint32_t aFlags);
 
@@ -92,6 +92,7 @@ class CacheEntry final : public nsICacheEntry
     PURGE_WHOLE,
   };
 
+  bool DeferOrBypassRemovalOnPinStatus(bool aPinned);
   bool Purge(uint32_t aWhat);
   void PurgeAndDoom();
   void DoomAlreadyRemoved();
@@ -136,13 +137,20 @@ class CacheEntry final : public nsICacheEntry
     Callback(CacheEntry* aEntry,
              nsICacheEntryOpenCallback *aCallback,
              bool aReadOnly, bool aCheckOnAnyThread, bool aSecret);
+    // Special constructor for Callback objects added to the chain
+    // just to ensure proper defer dooming (recreation) of this entry.
+    Callback(CacheEntry* aEntry, bool aDoomWhenFoundInPinStatus);
     Callback(Callback const &aThat);
     ~Callback();
 
     // Called when this callback record changes it's owning entry,
     // mainly during recreation.
     void ExchangeEntry(CacheEntry* aEntry);
 
+    // Returns true when an entry is about to be "defer" doomed and this is
+    // a "defer" callback.
+    bool DeferDoom(bool *aDoom) const;
+
     // We are raising reference count here to take into account the pending
     // callback (that virtually holds a ref to this entry before it gets
     // it's pointer).
@@ -156,6 +164,13 @@ class CacheEntry final : public nsICacheEntry
     bool mNotWanted : 1;
     bool mSecret : 1;
 
+    // These are set only for the defer-doomer Callback instance inserted
+    // to the callback chain.  When any of these is set and also any of
+    // the corressponding flags on the entry is set, this callback will
+    // indicate (via DeferDoom()) the entry have to be recreated/doomed.
+    bool mDoomWhenFoundPinned : 1;
+    bool mDoomWhenFoundNonPinned : 1;
+
     nsresult OnCheckThread(bool *aOnCheckThread) const;
     nsresult OnAvailThread(bool *aOnAvailThread) const;
   };
@@ -274,14 +289,9 @@ class CacheEntry final : public nsICacheEntry
   nsCString mStorageID;
 
   // Whether it's allowed to persist the data to disk
-  bool const mUseDisk;
-
+  bool const mUseDisk : 1;
   // Whether it should skip max size check.
-  bool const mSkipSizeCheck;
-
-  // Set when entry is doomed with AsyncDoom() or DoomAlreadyRemoved().
-  // Left as a standalone flag to not bother with locking (there is no need).
-  bool mIsDoomed;
+  bool const mSkipSizeCheck : 1;
 
   // Following flags are all synchronized with the cache entry lock.
 
@@ -296,6 +306,15 @@ class CacheEntry final : public nsICacheEntry
   // false: after load and a new file, or dropped to back to false when a writer
   //        fails to open an output stream.
   bool mHasData : 1;
+  // The indication of pinning this entry was open with
+  bool mPinned : 1;
+  // Whether the pinning state of the entry is known (equals to the actual state
+  // of the cache file)
+  bool mPinningKnown : 1;
+
+  // Set when entry is doomed with AsyncDoom() or DoomAlreadyRemoved().
+  // Left as a standalone flag to not bother with locking (there is no need).
+  bool mIsDoomed;
 
   static char const * StateString(uint32_t aState);