From c7df8d193b5d6282d9eb2bcf89bd7ef596ef8216 Mon Sep 17 00:00:00 2001 From: Nikhil Marathe Date: Thu, 23 Apr 2015 16:00:58 -0400 Subject: [PATCH] Bug 1154494 - Hit network only once. r=baku,bkelly --- dom/workers/ServiceWorkerManager.cpp | 10 +- dom/workers/ServiceWorkerScriptCache.cpp | 239 ++++++++++++++++-- dom/workers/ServiceWorkerScriptCache.h | 11 +- .../serviceworkers/test_periodic_update.html | 4 +- 4 files changed, 226 insertions(+), 38 deletions(-) diff --git a/dom/workers/ServiceWorkerManager.cpp b/dom/workers/ServiceWorkerManager.cpp index 849898a5f4fe..f0b5d2aca117 100644 --- a/dom/workers/ServiceWorkerManager.cpp +++ b/dom/workers/ServiceWorkerManager.cpp @@ -593,7 +593,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); @@ -623,12 +623,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)); @@ -637,7 +631,7 @@ public: MOZ_ASSERT(!mUpdateAndInstallInfo); mUpdateAndInstallInfo = new ServiceWorkerInfo(mRegistration, mRegistration->mScriptSpec, - cacheName); + aNewCacheName); nsRefPtr serviceWorker; rv = swm->CreateServiceWorker(mRegistration->mPrincipal, mUpdateAndInstallInfo, diff --git a/dom/workers/ServiceWorkerScriptCache.cpp b/dom/workers/ServiceWorkerScriptCache.cpp index 97bf4950c64b..4778169e4e31 100644 --- a/dom/workers/ServiceWorkerScriptCache.cpp +++ b/dom/workers/ServiceWorkerScriptCache.cpp @@ -156,7 +156,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 @@ -173,27 +173,7 @@ public: nsresult Initialize(nsIPrincipal* aPrincipal, const nsAString& aURL, - const nsAString& aCacheName) - { - MOZ_ASSERT(aPrincipal); - AssertIsOnMainThread(); - - mURL = aURL; - - ErrorResult rv; - nsRefPtr cacheStorage = CreateCacheStorage(aPrincipal, rv); - if (NS_WARN_IF(rv.Failed())) { - return rv.ErrorCode(); - } - - nsRefPtr promise = cacheStorage->Open(aCacheName, rv); - if (NS_WARN_IF(rv.Failed())) { - return rv.ErrorCode(); - } - - promise->AppendNativeHandler(this); - return NS_OK; - } + const nsAString& aCacheName); void Abort() @@ -240,6 +220,12 @@ public: return mBuffer; } + const nsString& URL() const + { + AssertIsOnMainThread(); + return mURL; + } + private: ~CompareCache() { @@ -268,13 +254,14 @@ 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) @@ -289,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.ErrorCode(); + } + mCN = new CompareNetwork(this); nsresult rv = mCN->Initialize(aPrincipal, aURL); if (NS_WARN_IF(NS_FAILED(rv))) { @@ -297,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; @@ -307,6 +305,13 @@ public: return NS_OK; } + const nsAString& + URL() const + { + AssertIsOnMainThread(); + return mURL; + } + void NetworkFinished(nsresult aStatus) { @@ -363,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 aValue) override + { + AssertIsOnMainThread(); + MOZ_ASSERT(mCallback); + + if (mState == WaitingForOpen) { + if (NS_WARN_IF(!aValue.isObject())) { + Fail(NS_ERROR_FAILURE); + return; + } + + JS::Rooted 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 kungfuDeathGrip = cache; + WriteToCache(cache); + return; + } + + MOZ_ASSERT(mState == WaitingForPut); + mCallback->ComparisonResult(NS_OK, false /* aIsEqual */, mNewCacheName); + Cleanup(); + } + + void + RejectedCallback(JSContext* aCx, JS::Handle 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() { @@ -372,22 +436,119 @@ 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 (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.ErrorCode()); + return; + } + + nsRefPtr cacheOpenPromise = mCacheStorage->Open(mNewCacheName, result); + if (NS_WARN_IF(result.Failed())) { + MOZ_ASSERT(!result.IsErrorWithMessage()); + Fail(result.ErrorCode()); + return; + } + + cacheOpenPromise->AppendNativeHandler(this); + } + + void + WriteToCache(Cache* aCache) + { + AssertIsOnMainThread(); + MOZ_ASSERT(aCache); + MOZ_ASSERT(mState == WaitingForOpen); + + ErrorResult result; + nsCOMPtr body; + result = NS_NewStringInputStream(getter_AddRefs(body), mCN->Buffer()); + if (NS_WARN_IF(result.Failed())) { + MOZ_ASSERT(!result.IsErrorWithMessage()); + Fail(result.ErrorCode()); + return; + } + + nsRefPtr ir = + new InternalResponse(200, NS_LITERAL_CSTRING("OK")); + ir->SetBody(body); + + nsRefPtr 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 cachePromise = aCache->Put(request, *response, result); + if (NS_WARN_IF(result.Failed())) { + MOZ_ASSERT(!result.IsErrorWithMessage()); + Fail(result.ErrorCode()); + return; + } + + mState = WaitingForPut; + cachePromise->AppendNativeHandler(this); + } + nsRefPtr mCallback; + nsRefPtr mCacheStorage; nsRefPtr mCN; nsRefPtr 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; @@ -454,6 +615,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 = mManager->CacheStorage_()->Open(aCacheName, rv); + if (NS_WARN_IF(rv.Failed())) { + MOZ_ASSERT(!rv.IsErrorWithMessage()); + return rv.ErrorCode(); + } + + promise->AppendNativeHandler(this); + return NS_OK; +} + NS_IMETHODIMP CompareCache::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext, nsresult aStatus, uint32_t aLen, @@ -528,6 +710,7 @@ CompareCache::ManageCacheResult(JSContext* aCx, JS::Handle aValue) CacheQueryOptions params; nsRefPtr promise = cache->Match(request, params, error); if (NS_WARN_IF(error.Failed())) { + MOZ_ASSERT(!error.IsErrorWithMessage()); mManager->CacheFinished(error.ErrorCode(), false); return; } @@ -617,6 +800,7 @@ PurgeCache(nsIPrincipal* aPrincipal, const nsAString& aCacheName) ErrorResult rv; nsRefPtr cacheStorage = CreateCacheStorage(aPrincipal, rv); if (NS_WARN_IF(rv.Failed())) { + MOZ_ASSERT(!rv.IsErrorWithMessage()); return rv.ErrorCode(); } @@ -624,6 +808,7 @@ PurgeCache(nsIPrincipal* aPrincipal, const nsAString& aCacheName) nsRefPtr promise = cacheStorage->Delete(aCacheName, rv); if (NS_WARN_IF(rv.Failed())) { + MOZ_ASSERT(!rv.IsErrorWithMessage()); return rv.ErrorCode(); } diff --git a/dom/workers/ServiceWorkerScriptCache.h b/dom/workers/ServiceWorkerScriptCache.h index 9ba0cbf65e61..f24924ad1f2e 100644 --- a/dom/workers/ServiceWorkerScriptCache.h +++ b/dom/workers/ServiceWorkerScriptCache.h @@ -24,7 +24,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; diff --git a/dom/workers/test/serviceworkers/test_periodic_update.html b/dom/workers/test/serviceworkers/test_periodic_update.html index 8e7264703402..c272254e0ad4 100644 --- a/dom/workers/test/serviceworkers/test_periodic_update.html +++ b/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");