Bug 938186 - introduce FORCE_ASYNC_CALLBACK for opening cache entries, r=michal
authorHonza Bambas <honzab.moz@firemni.cz>
Thu, 05 Jun 2014 18:27:38 +0200
changeset 186854 940d041073c7a9bb2b1ec6bff600ada76ac94444
parent 186853 927b20a21b19a6309697803c2ebf70b4fc8ccf2b
child 186855 d405928cb93469b7559d9599a719dae9ba670586
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersmichal
bugs938186
milestone32.0a1
Bug 938186 - introduce FORCE_ASYNC_CALLBACK for opening cache entries, r=michal
netwerk/cache2/CacheEntry.cpp
netwerk/cache2/CacheEntry.h
netwerk/cache2/nsICacheStorage.idl
netwerk/test/unit/test_cache2-01f-basic-async.js
netwerk/test/unit/test_cache2-24-force-async.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/cache2/CacheEntry.cpp
+++ b/netwerk/cache2/CacheEntry.cpp
@@ -71,24 +71,25 @@ CacheEntryHandle::~CacheEntryHandle()
 
   MOZ_COUNT_DTOR(CacheEntryHandle);
 }
 
 // CacheEntry::Callback
 
 CacheEntry::Callback::Callback(CacheEntry* aEntry,
                                nsICacheEntryOpenCallback *aCallback,
-                               bool aReadOnly, bool aCheckOnAnyThread)
+                               bool aReadOnly, bool aCheckOnAnyThread, bool aForceAsync)
 : mEntry(aEntry)
 , mCallback(aCallback)
 , mTargetThread(do_GetCurrentThread())
 , mReadOnly(aReadOnly)
 , mCheckOnAnyThread(aCheckOnAnyThread)
 , mRecheckAfterWrite(false)
 , mNotWanted(false)
+, mForceAsync(aForceAsync)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
   // The counter may go from zero to non-null only under the service lock
   // but here we expect it to be already positive.
   MOZ_ASSERT(mEntry->HandlesCount());
   mEntry->AddHandleRef();
 }
@@ -96,16 +97,17 @@ CacheEntry::Callback::Callback(CacheEntr
 CacheEntry::Callback::Callback(CacheEntry::Callback const &aThat)
 : mEntry(aThat.mEntry)
 , mCallback(aThat.mCallback)
 , mTargetThread(aThat.mTargetThread)
 , mReadOnly(aThat.mReadOnly)
 , mCheckOnAnyThread(aThat.mCheckOnAnyThread)
 , mRecheckAfterWrite(aThat.mRecheckAfterWrite)
 , mNotWanted(aThat.mNotWanted)
+, mForceAsync(aThat.mForceAsync)
 {
   MOZ_COUNT_CTOR(CacheEntry::Callback);
 
   // The counter may go from zero to non-null only under the service lock
   // but here we expect it to be already positive.
   MOZ_ASSERT(mEntry->HandlesCount());
   mEntry->AddHandleRef();
 }
@@ -126,30 +128,58 @@ void CacheEntry::Callback::ExchangeEntry
   // The counter may go from zero to non-null only under the service lock
   // but here we expect it to be already positive.
   MOZ_ASSERT(aEntry->HandlesCount());
   aEntry->AddHandleRef();
   mEntry->ReleaseHandleRef();
   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);
   }
 
   // We can invoke check anywhere
   *aOnCheckThread = true;
   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);
 }
 
 // CacheEntry
 
 NS_IMPL_ISUPPORTS(CacheEntry,
                   nsICacheEntry,
                   nsIRunnable,
@@ -263,21 +293,22 @@ void CacheEntry::AsyncOpen(nsICacheEntry
   LOG(("CacheEntry::AsyncOpen [this=%p, state=%s, flags=%d, callback=%p]",
     this, StateString(mState), aFlags, aCallback));
 
   bool readonly = aFlags & nsICacheStorage::OPEN_READONLY;
   bool bypassIfBusy = aFlags & nsICacheStorage::OPEN_BYPASS_IF_BUSY;
   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);
 
   RememberCallback(callback, bypassIfBusy);
 
   // Load() opens the lock
   if (Load(truncate, priority)) {
     // Loading is in progress...
@@ -693,17 +724,17 @@ bool CacheEntry::InvokeCallback(Callback
 
     mozilla::MutexAutoUnlock unlock(mLock);
     InvokeAvailableCallback(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));
 
   nsresult rv;
 
   uint32_t const state = mState;
 
--- a/netwerk/cache2/CacheEntry.h
+++ b/netwerk/cache2/CacheEntry.h
@@ -130,17 +130,17 @@ private:
   // We must monitor when a cache entry whose consumer is responsible
   // for writing it the first time gets released.  We must then invoke
   // waiting callbacks to not break the chain.
   class Callback
   {
   public:
     Callback(CacheEntry* aEntry,
              nsICacheEntryOpenCallback *aCallback,
-             bool aReadOnly, bool aCheckOnAnyThread);
+             bool aReadOnly, bool aCheckOnAnyThread, bool aForceAsync);
     Callback(Callback const &aThat);
     ~Callback();
 
     // Called when this callback record changes it's owning entry,
     // mainly during recreation.
     void ExchangeEntry(CacheEntry* aEntry);
 
     // We are raising reference count here to take into account the pending
@@ -148,19 +148,21 @@ private:
     // it's pointer).
     nsRefPtr<CacheEntry> mEntry;
     nsCOMPtr<nsICacheEntryOpenCallback> mCallback;
     nsCOMPtr<nsIThread> mTargetThread;
     bool mReadOnly : 1;
     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
   // we need a runnable for it...
   class AvailableCallbackRunnable : public nsRunnable
   {
   public:
     AvailableCallbackRunnable(CacheEntry* aEntry,
@@ -210,17 +212,17 @@ private:
   bool Load(bool aTruncate, bool aPriority);
   void OnLoaded();
 
   void RememberCallback(Callback & aCallback, bool aBypassIfBusy);
   void InvokeCallbacksLock();
   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);
 
   // When this entry is new and recreated w/o a callback, we need to wrap it
   // with a handle to detect writing consumer is gone.
   CacheEntryHandle* NewWriteHandle();
   void OnHandleClosed(CacheEntryHandle const* aHandle);
 
--- a/netwerk/cache2/nsICacheStorage.idl
+++ b/netwerk/cache2/nsICacheStorage.idl
@@ -46,16 +46,23 @@ interface nsICacheStorage : nsISupports
    * Perform the cache entry check (onCacheEntryCheck invocation) on any thread 
    * for optimal perfomance optimization.  If this flag is not specified it is
    * ensured that onCacheEntryCheck is called on the same thread as respective 
    * asyncOpen has been called.
    */
   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.
    *
    * @param aURI
    *    The URI to search in cache or to open for writting.
    * @param aIdExtension
    *    Any string that will extend (distinguish) the entry.  Two entries
    *    with the same aURI but different aIdExtension will be comletely
@@ -64,16 +71,17 @@ interface nsICacheStorage : nsISupports
    * @param aFlags
    *    OPEN_NORMALLY - open cache entry normally for read and write
    *    OPEN_TRUNCATE - delete any existing entry before opening it
    *    OPEN_READONLY - don't create an entry if there is none
    *    OPEN_PRIORITY - give this request a priority over others
    *    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.
    */
   void asyncOpenURI(in nsIURI aURI, in ACString aIdExtension,
                     in uint32_t aFlags,
                     in nsICacheEntryOpenCallback aCallback);
 
new file mode 100644
--- /dev/null
+++ b/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();
+}
new file mode 100644
--- /dev/null
+++ b/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();
+}
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -20,16 +20,17 @@ support-files =
 
 [test_nsIBufferedOutputStream_writeFrom_block.js]
 [test_cache2-01-basic.js]
 [test_cache2-01a-basic-readonly.js]
 [test_cache2-01b-basic-datasize.js]
 [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]
 [test_cache2-05-visit.js]
 [test_cache2-06-pb-mode.js]
 # Bug 675039, comment 6: "The difference is that the memory cache is disabled in Armv6 builds."
 skip-if = os == "android"
 [test_cache2-07-visit-memory.js]
@@ -52,16 +53,17 @@ skip-if = os == "android"
 [test_cache2-16-conditional-200.js]
 [test_cache2-17-evict-all.js]
 [test_cache2-18-not-valid.js]
 [test_cache2-19-range-206.js]
 [test_cache2-20-range-200.js]
 [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"
 [test_cacheForOfflineUse_no-store.js]
 [test_307_redirect.js]
 [test_NetUtil.js]
 [test_URIs.js]
 [test_aboutblank.js]