Bug 938186 - introduce FORCE_ASYNC_CALLBACK for opening cache entries, r=michal

netwerk/cache2/CacheEntry.cpp

@@ -76,7 +76,7 @@ CacheEntryHandle::~CacheEntryHandle()
 
 CacheEntry::Callback::Callback(CacheEntry* aEntry,
                                nsICacheEntryOpenCallback *aCallback,
-                               bool aReadOnly, bool aCheckOnAnyThread)
+                               bool aReadOnly, bool aCheckOnAnyThread, bool aForceAsync)
 : mEntry(aEntry)
 , mCallback(aCallback)
 , mTargetThread(do_GetCurrentThread())
@@ -84,6 +84,7 @@ CacheEntry::Callback::Callback(CacheEntry* aEntry,
 , mCheckOnAnyThread(aCheckOnAnyThread)
 , mRecheckAfterWrite(false)
 , mNotWanted(false)
+, mForceAsync(aForceAsync)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
@@ -101,6 +102,7 @@ CacheEntry::Callback::Callback(CacheEntry::Callback const &aThat)
 , mCheckOnAnyThread(aThat.mCheckOnAnyThread)
 , mRecheckAfterWrite(aThat.mRecheckAfterWrite)
 , mNotWanted(aThat.mNotWanted)
+, mForceAsync(aThat.mForceAsync)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
@@ -131,8 +133,31 @@ void CacheEntry::Callback::ExchangeEntry(CacheEntry* aEntry)
   mEntry = aEntry;
 }
 
-nsresult CacheEntry::Callback::OnCheckThread(bool *aOnCheckThread) const
+bool CacheEntry::Callback::ForceAsync()
 {
+  if (!mForceAsync) {
+    return false;
+  }
+
+  // Drop the flag now.  First time we must claim we are not on the proper thread
+  // what will simply force a post.  But, the post does the check again and that
+  // time we must already tell the true we are on the proper thread otherwise we
+  // just loop indefinitely.  Also, we need to post only once the first callback
+  // of OnCheck or OnAvail.  OnAvail after OnCheck can already go directly.
+  // Note on thread safety: when called from OnCheckThread we are definitely under
+  // the lock, when called from OnAvailThread we don't anymore need to be under
+  // the lock since all concurrency risks are over by that time.
+  mForceAsync = false;
+  return true;
+}
+
+nsresult CacheEntry::Callback::OnCheckThread(bool *aOnCheckThread)
+{
+  if (ForceAsync()) {
+    *aOnCheckThread = false;
+    return NS_OK;
+  }
+
   if (!mCheckOnAnyThread) {
     // Check we are on the target
     return mTargetThread->IsOnCurrentThread(aOnCheckThread);
@@ -143,8 +168,13 @@ nsresult CacheEntry::Callback::OnCheckThread(bool *aOnCheckThread) const
   return NS_OK;
 }
 
-nsresult CacheEntry::Callback::OnAvailThread(bool *aOnAvailThread) const
+nsresult CacheEntry::Callback::OnAvailThread(bool *aOnAvailThread)
 {
+  if (ForceAsync()) {
+    *aOnAvailThread = false;
+    return NS_OK;
+  }
+
   return mTargetThread->IsOnCurrentThread(aOnAvailThread);
 }
 
@@ -268,11 +298,12 @@ void CacheEntry::AsyncOpen(nsICacheEntryOpenCallback* aCallback, uint32_t aFlags
   bool truncate = aFlags & nsICacheStorage::OPEN_TRUNCATE;
   bool priority = aFlags & nsICacheStorage::OPEN_PRIORITY;
   bool multithread = aFlags & nsICacheStorage::CHECK_MULTITHREADED;
+  bool async = aFlags & nsICacheStorage::FORCE_ASYNC_CALLBACK;
 
   MOZ_ASSERT(!readonly || !truncate, "Bad flags combination");
   MOZ_ASSERT(!(truncate && mState > LOADING), "Must not call truncate on already loaded entry");
 
-  Callback callback(this, aCallback, readonly, multithread);
+  Callback callback(this, aCallback, readonly, multithread, async);
 
   mozilla::MutexAutoLock lock(mLock);
 
@@ -698,7 +729,7 @@ bool CacheEntry::InvokeCallback(Callback & aCallback)
   return true;
 }
 
-void CacheEntry::InvokeAvailableCallback(Callback const & aCallback)
+void CacheEntry::InvokeAvailableCallback(Callback & aCallback)
 {
   LOG(("CacheEntry::InvokeAvailableCallback [this=%p, state=%s, cb=%p, r/o=%d, n/w=%d]",
     this, StateString(mState), aCallback.mCallback.get(), aCallback.mReadOnly, aCallback.mNotWanted));

netwerk/cache2/CacheEntry.h

@@ -135,7 +135,7 @@ private:
   public:
     Callback(CacheEntry* aEntry,
              nsICacheEntryOpenCallback *aCallback,
-             bool aReadOnly, bool aCheckOnAnyThread);
+             bool aReadOnly, bool aCheckOnAnyThread, bool aForceAsync);
     Callback(Callback const &aThat);
     ~Callback();
 
@@ -153,9 +153,11 @@ private:
     bool mCheckOnAnyThread : 1;
     bool mRecheckAfterWrite : 1;
     bool mNotWanted : 1;
+    bool mForceAsync : 1;
 
-    nsresult OnCheckThread(bool *aOnCheckThread) const;
-    nsresult OnAvailThread(bool *aOnAvailThread) const;
+    bool ForceAsync();
+    nsresult OnCheckThread(bool *aOnCheckThread);
+    nsresult OnAvailThread(bool *aOnAvailThread);
   };
 
   // Since OnCacheEntryAvailable must be invoked on the main thread
@@ -215,7 +217,7 @@ private:
   void InvokeCallbacks();
   bool InvokeCallbacks(bool aReadOnly);
   bool InvokeCallback(Callback & aCallback);
-  void InvokeAvailableCallback(Callback const & aCallback);
+  void InvokeAvailableCallback(Callback & aCallback);
 
   nsresult OpenOutputStreamInternal(int64_t offset, nsIOutputStream * *_retval);
 

netwerk/cache2/nsICacheStorage.idl

@@ -50,6 +50,13 @@ interface nsICacheStorage : nsISupports
    */
   const uint32_t CHECK_MULTITHREADED = 1 << 4;
 
+  /**
+   * Forces the callback to be invoked always only asynchronously regardless
+   * we have all the information to invoke it directly from inside asyncOpenURI
+   * method.
+   */
+  const uint32_t FORCE_ASYNC_CALLBACK = 1 << 5;
+
   /**
    * Asynchronously opens a cache entry for the specified URI.
    * Result is fetched asynchronously via the callback.
@@ -69,6 +76,7 @@ interface nsICacheStorage : nsISupports
    *    OPEN_BYPASS_IF_BUSY - backward compatibility only, LOAD_BYPASS_LOCAL_CACHE_IF_BUSY
    *    CHECK_MULTITHREADED - onCacheEntryCheck may be called on any thread, consumer 
    *                          implementation is thread-safe
+   *    FORCE_ASYNC_CALLBACK - always call the callback asynchronously
    * @param aCallback
    *    The consumer that receives the result.
    *    IMPORTANT: The callback may be called sooner the method returns.

netwerk/test/unit/test_cache2-01f-basic-async.js

@@ -0,0 +1,28 @@
+function run_test()
+{
+  do_get_profile();
+
+  // Open for write, write
+  asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+    new OpenCallback(NEW, "a1m", "a1d", function(entry) {
+      // Open for read and check
+      asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+        new OpenCallback(NORMAL, "a1m", "a1d", function(entry) {
+          // Open for rewrite (truncate), write different meta and data
+          asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_TRUNCATE | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+            new OpenCallback(NEW, "a2m", "a2d", function(entry) {
+              // Open for read and check
+              asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+                new OpenCallback(NORMAL, "a2m", "a2d", function(entry) {
+                  finish_cache2_test();
+                })
+              );
+            })
+          );
+        })
+      );
+    })
+  );
+
+  do_test_pending();
+}

netwerk/test/unit/test_cache2-24-force-async.js

@@ -0,0 +1,42 @@
+function run_test()
+{
+  do_get_profile();
+
+  // Open for write, write, and wait for finishing it before notification to avoid concurrent write
+  // since we want to get as much as possible the scenario when an entry is left in the pool
+  // and a new consumer comes to open it later.
+  var outOfAsyncOpen0 = false;
+  asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+    new OpenCallback(NEW|WAITFORWRITE, "a1m", "a1d", function(entry) {
+      do_check_true(outOfAsyncOpen0);
+      // Open for read, expect callback happen from inside asyncOpenURI
+      var outOfAsyncOpen1 = false;
+      asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY, null,
+        function(entry) {
+          do_check_false(outOfAsyncOpen1);
+          var outOfAsyncOpen2 = false;
+          // Open for read, again should be sync
+          asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY, null,
+            function(entry) {
+              do_check_false(outOfAsyncOpen2);
+              var outOfAsyncOpen3 = false;
+              // Open for read, expect callback happen from outside of asyncOpenURI
+              asyncOpenCacheEntry("http://a/", "disk", Ci.nsICacheStorage.OPEN_NORMALLY | Ci.nsICacheStorage.FORCE_ASYNC_CALLBACK, null,
+                function(entry) {
+                  do_check_true(outOfAsyncOpen3);
+                  finish_cache2_test();
+                }
+              );
+              outOfAsyncOpen3 = true;
+            }
+          );
+          outOfAsyncOpen2 = true;
+        }
+      );
+      outOfAsyncOpen1 = true;
+    })
+  );
+  outOfAsyncOpen0 = true;
+
+  do_test_pending();
+}

netwerk/test/unit/xpcshell.ini

@@ -25,6 +25,7 @@ support-files =
 [test_cache2-01c-basic-hasmeta-only.js]
 [test_cache2-01d-basic-not-wanted.js]
 [test_cache2-01e-basic-bypass-if-busy.js]
+[test_cache2-01f-basic-async.js]
 [test_cache2-02-open-non-existing.js]
 [test_cache2-03-oncacheentryavail-throws.js]
 [test_cache2-04-oncacheentryavail-throws2x.js]
@@ -57,6 +58,7 @@ skip-if = os == "android"
 [test_cache2-21-anon-storage.js]
 [test_cache2-22-anon-visit.js]
 [test_cache2-23-read-over-chunk.js]
+[test_cache2-24-force-async.js]
 [test_304_responses.js]
 # Bug 675039: test hangs on Android-armv6 
 skip-if = os == "android"