Bug 1154494 - Hit network only once. r=baku,bkelly

--HG--
extra : rebase_source : d2720c0bbef92595228658d92db3bbaef051cced

dom/workers/ServiceWorkerManager.cpp

@@ -603,7 +603,7 @@ public:
   }
 
   void
-  ComparisonResult(nsresult aStatus, bool aInCacheAndEqual) override
+  ComparisonResult(nsresult aStatus, bool aInCacheAndEqual, const nsAString& aNewCacheName) override
   {
     if (NS_WARN_IF(NS_FAILED(aStatus))) {
       Fail(NS_ERROR_DOM_TYPE_ERR);
@@ -633,12 +633,6 @@ public:
     }
 
     nsAutoString cacheName;
-    rv = serviceWorkerScriptCache::GenerateCacheName(cacheName);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      Fail(NS_ERROR_DOM_TYPE_ERR);
-      return;
-    }
-
     // We have to create a ServiceWorker here simply to ensure there are no
     // errors. Ideally we should just pass this worker on to ContinueInstall.
     MOZ_ASSERT(!swm->mSetOfScopesBeingUpdated.Contains(mRegistration->mScope));
@@ -647,7 +641,7 @@ public:
     MOZ_ASSERT(!mUpdateAndInstallInfo);
     mUpdateAndInstallInfo =
       new ServiceWorkerInfo(mRegistration, mRegistration->mScriptSpec,
-                            cacheName);
+                            aNewCacheName);
     nsRefPtr<ServiceWorker> serviceWorker;
     rv = swm->CreateServiceWorker(mRegistration->mPrincipal,
                                   mUpdateAndInstallInfo,

dom/workers/ServiceWorkerScriptCache.cpp

@@ -158,7 +158,7 @@ NS_IMPL_ISUPPORTS(CompareNetwork, nsIStreamLoaderObserver)
 // This class gets a cached Response from the CacheStorage and then it calls
 // CacheFinished() in the CompareManager.
 class CompareCache final : public PromiseNativeHandler
-                             , public nsIStreamLoaderObserver
+                         , public nsIStreamLoaderObserver
 {
 public:
   NS_DECL_ISUPPORTS
@@ -175,27 +175,7 @@ public:
 
   nsresult
   Initialize(nsIPrincipal* aPrincipal, const nsAString& aURL,
-             const nsAString& aCacheName)
-  {
-    MOZ_ASSERT(aPrincipal);
-    AssertIsOnMainThread();
-
-    mURL = aURL;
-
-    ErrorResult rv;
-    nsRefPtr<CacheStorage> cacheStorage = CreateCacheStorage(aPrincipal, rv);
-    if (NS_WARN_IF(rv.Failed())) {
-      return rv.StealNSResult();
-    }
-
-    nsRefPtr<Promise> promise = cacheStorage->Open(aCacheName, rv);
-    if (NS_WARN_IF(rv.Failed())) {
-      return rv.StealNSResult();
-    }
-
-    promise->AppendNativeHandler(this);
-    return NS_OK;
-  }
+             const nsAString& aCacheName);
 
   void
   Abort()
@@ -242,6 +222,12 @@ public:
     return mBuffer;
   }
 
+  const nsString& URL() const
+  {
+    AssertIsOnMainThread();
+    return mURL;
+  }
+
 private:
   ~CompareCache()
   {
@@ -270,13 +256,12 @@ private:
 
 NS_IMPL_ISUPPORTS(CompareCache, nsIStreamLoaderObserver)
 
-class CompareManager final
+class CompareManager final : public PromiseNativeHandler
 {
 public:
-  NS_INLINE_DECL_REFCOUNTING(CompareManager)
-
   explicit CompareManager(CompareCallback* aCallback)
     : mCallback(aCallback)
+    , mState(WaitingForOpen)
     , mNetworkFinished(false)
     , mCacheFinished(false)
     , mInCache(false)
@@ -291,6 +276,17 @@ public:
     AssertIsOnMainThread();
     MOZ_ASSERT(aPrincipal);
 
+    mURL = aURL;
+
+    // Always create a CacheStorage since we want to write the network entry to
+    // the cache even if there isn't an existing one.
+    ErrorResult result;
+    mCacheStorage = CreateCacheStorage(aPrincipal, result);
+    if (NS_WARN_IF(result.Failed())) {
+      MOZ_ASSERT(!result.IsErrorWithMessage());
+      return result.StealNSResult();
+    }
+
     mCN = new CompareNetwork(this);
     nsresult rv = mCN->Initialize(aPrincipal, aURL);
     if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -299,7 +295,7 @@ public:
 
     if (!aCacheName.IsEmpty()) {
       mCC = new CompareCache(this);
-      mCC->Initialize(aPrincipal, aURL, aCacheName);
+      rv = mCC->Initialize(aPrincipal, aURL, aCacheName);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         mCN->Abort();
         return rv;
@@ -309,6 +305,13 @@ public:
     return NS_OK;
   }
 
+  const nsAString&
+  URL() const
+  {
+    AssertIsOnMainThread();
+    return mURL;
+  }
+
   void
   NetworkFinished(nsresult aStatus)
   {
@@ -365,6 +368,65 @@ public:
     ComparisonFinished(NS_OK, mCC->Buffer().Equals(mCN->Buffer()));
   }
 
+  // This class manages 2 promises: 1 is to retrieve Cache object, and 2 is to
+  // Put the value in the cache. For this reason we have mState to know what
+  // callback we are handling.
+  void
+  ResolvedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(mCallback);
+
+    if (mState == WaitingForOpen) {
+      if (NS_WARN_IF(!aValue.isObject())) {
+        Fail(NS_ERROR_FAILURE);
+        return;
+      }
+
+      JS::Rooted<JSObject*> obj(aCx, &aValue.toObject());
+      if (NS_WARN_IF(!obj)) {
+        Fail(NS_ERROR_FAILURE);
+        return;
+      }
+
+      Cache* cache = nullptr;
+      nsresult rv = UNWRAP_OBJECT(Cache, obj, cache);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        Fail(rv);
+        return;
+      }
+
+      // Just to be safe.
+      nsRefPtr<Cache> kungfuDeathGrip = cache;
+      WriteToCache(cache);
+      return;
+    }
+
+    MOZ_ASSERT(mState == WaitingForPut);
+    mCallback->ComparisonResult(NS_OK, false /* aIsEqual */, mNewCacheName);
+    Cleanup();
+  }
+
+  void
+  RejectedCallback(JSContext* aCx, JS::Handle<JS::Value> aValue) override
+  {
+    AssertIsOnMainThread();
+    if (mState == WaitingForOpen) {
+      NS_WARNING("Could not open cache.");
+    } else {
+      NS_WARNING("Could not write to cache.");
+    }
+    Fail(NS_ERROR_FAILURE);
+  }
+
+  CacheStorage*
+  CacheStorage_()
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(mCacheStorage);
+    return mCacheStorage;
+  }
+
 private:
   ~CompareManager()
   {
@@ -374,22 +436,124 @@ private:
   }
 
   void
-  ComparisonFinished(nsresult aStatus, bool aIsEqual)
+  Fail(nsresult aStatus)
+  {
+    AssertIsOnMainThread();
+    mCallback->ComparisonResult(aStatus, false /* aIsEqual */, EmptyString());
+    Cleanup();
+  }
+
+  void
+  Cleanup()
   {
     AssertIsOnMainThread();
     MOZ_ASSERT(mCallback);
-
-    mCallback->ComparisonResult(aStatus, aIsEqual);
     mCallback = nullptr;
     mCN = nullptr;
     mCC = nullptr;
   }
 
+  void
+  ComparisonFinished(nsresult aStatus, bool aIsEqual)
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(mCallback);
+
+    if (NS_FAILED(aStatus)) {
+      Fail(aStatus);
+      return;
+    }
+
+    if (aIsEqual) {
+      mCallback->ComparisonResult(aStatus, aIsEqual, EmptyString());
+      Cleanup();
+      return;
+    }
+
+    // Write to Cache so ScriptLoader reads succeed.
+    WriteNetworkBufferToNewCache();
+  }
+
+  void
+  WriteNetworkBufferToNewCache()
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(mCN);
+    MOZ_ASSERT(mCacheStorage);
+    MOZ_ASSERT(mNewCacheName.IsEmpty());
+
+    ErrorResult result;
+    result = serviceWorkerScriptCache::GenerateCacheName(mNewCacheName);
+    if (NS_WARN_IF(result.Failed())) {
+      MOZ_ASSERT(!result.IsErrorWithMessage());
+      Fail(result.StealNSResult());
+      return;
+    }
+
+    nsRefPtr<Promise> cacheOpenPromise = mCacheStorage->Open(mNewCacheName, result);
+    if (NS_WARN_IF(result.Failed())) {
+      MOZ_ASSERT(!result.IsErrorWithMessage());
+      Fail(result.StealNSResult());
+      return;
+    }
+
+    cacheOpenPromise->AppendNativeHandler(this);
+  }
+
+  void
+  WriteToCache(Cache* aCache)
+  {
+    AssertIsOnMainThread();
+    MOZ_ASSERT(aCache);
+    MOZ_ASSERT(mState == WaitingForOpen);
+
+    ErrorResult result;
+    nsCOMPtr<nsIInputStream> body;
+    result = NS_NewStringInputStream(getter_AddRefs(body), mCN->Buffer());
+    if (NS_WARN_IF(result.Failed())) {
+      MOZ_ASSERT(!result.IsErrorWithMessage());
+      Fail(result.StealNSResult());
+      return;
+    }
+
+    nsRefPtr<InternalResponse> ir =
+      new InternalResponse(200, NS_LITERAL_CSTRING("OK"));
+    ir->SetBody(body);
+
+    nsRefPtr<Response> response = new Response(aCache->GetGlobalObject(), ir);
+
+    RequestOrUSVString request;
+    request.SetAsUSVString().Rebind(URL().Data(), URL().Length());
+
+    // For now we have to wait until the Put Promise is fulfilled before we can
+    // continue since Cache does not yet support starting a read that is being
+    // written to.
+    nsRefPtr<Promise> cachePromise = aCache->Put(request, *response, result);
+    if (NS_WARN_IF(result.Failed())) {
+      MOZ_ASSERT(!result.IsErrorWithMessage());
+      Fail(result.StealNSResult());
+      return;
+    }
+
+    mState = WaitingForPut;
+    cachePromise->AppendNativeHandler(this);
+  }
+
   nsRefPtr<CompareCallback> mCallback;
+  nsRefPtr<CacheStorage> mCacheStorage;
 
   nsRefPtr<CompareNetwork> mCN;
   nsRefPtr<CompareCache> mCC;
 
+  nsString mURL;
+  // Only used if the network script has changed and needs to be cached.
+  nsString mNewCacheName;
+
+  enum {
+    WaitingForOpen,
+    WaitingForPut
+  } mState;
+
   bool mNetworkFinished;
   bool mCacheFinished;
   bool mInCache;
@@ -482,6 +646,27 @@ CompareNetwork::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext
   return NS_OK;
 }
 
+nsresult
+CompareCache::Initialize(nsIPrincipal* aPrincipal, const nsAString& aURL,
+                         const nsAString& aCacheName)
+{
+  MOZ_ASSERT(aPrincipal);
+  AssertIsOnMainThread();
+
+  mURL = aURL;
+
+  ErrorResult rv;
+
+  nsRefPtr<Promise> promise = mManager->CacheStorage_()->Open(aCacheName, rv);
+  if (NS_WARN_IF(rv.Failed())) {
+    MOZ_ASSERT(!rv.IsErrorWithMessage());
+    return rv.StealNSResult();
+  }
+
+  promise->AppendNativeHandler(this);
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 CompareCache::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext,
                                nsresult aStatus, uint32_t aLen,

dom/workers/ServiceWorkerScriptCache.h

@@ -26,7 +26,16 @@ GenerateCacheName(nsAString& aName);
 class CompareCallback
 {
 public:
-  virtual void ComparisonResult(nsresult aStatus, bool aInCacheAndEqual) = 0;
+  /*
+   * If there is an error, ignore aInCacheAndEqual and aNewCacheName.
+   * On success, if the cached result and network result matched,
+   * aInCacheAndEqual will be true and no new cache name is passed, otherwise
+   * use the new cache name to load the ServiceWorker.
+   */
+  virtual void
+  ComparisonResult(nsresult aStatus,
+                   bool aInCacheAndEqual,
+                   const nsAString& aNewCacheName) = 0;
 
   NS_IMETHOD_(MozExternalRefCountType) AddRef() = 0;
   NS_IMETHOD_(MozExternalRefCountType) Release() = 0;

dom/workers/test/serviceworkers/test_periodic_update.html

@@ -25,7 +25,7 @@
       // Verify that the service worker has been correctly updated.
       testFrame("periodic/frame.html").then(function(body) {
         newSWVersion = parseInt(body);
-        todo_is(newSWVersion, "2", "Expected correct new version");
+        is(newSWVersion, 2, "Expected correct new version");
         ok(newSWVersion > oldSWVersion,
            "The SW should be successfully updated, old: " + oldSWVersion +
            ", new: " + newSWVersion);
@@ -38,7 +38,7 @@
     registerSW().then(function() {
       return testFrame("periodic/frame.html").then(function(body) {
         oldSWVersion = parseInt(body);
-        todo_is(oldSWVersion, "1", "Expected correct old version");
+        is(oldSWVersion, 1, "Expected correct old version");
       });
     }).then(function() {
       return navigator.serviceWorker.getRegistration("periodic/foo");