From 812da7fa44a84ca9858ad7af0abe1fbd53004e25 Mon Sep 17 00:00:00 2001 From: Andrew Sutherland Date: Fri, 24 Aug 2018 10:24:28 -0400 Subject: [PATCH] Bug 1472303 - Backed out changeset 8a40d04dfcbb. r=asuth The logic here to move our check was right, but our check was wrong. Also, we landed a test that checked for our wrong implementation. We need to correct our implementation and re-think the test. The right test might just be a mochitest, possibly with some Firefox-only hooks involved. --HG-- extra : rebase_source : 4d6b9a120adcee835f626098e8547c440a39f595 --- .../ServiceWorkerRegistration.cpp | 11 ------- .../ServiceWorkerRegistrationImpl.cpp | 9 ++++-- dom/workers/WorkerScope.cpp | 1 - dom/workers/WorkerScope.h | 6 ---- testing/web-platform/meta/MANIFEST.json | 19 ----------- .../resources/update-top-level-worker.py | 18 ----------- .../update-top-level.https.html | 32 ------------------- 7 files changed, 7 insertions(+), 89 deletions(-) delete mode 100644 testing/web-platform/tests/service-workers/service-worker/resources/update-top-level-worker.py delete mode 100644 testing/web-platform/tests/service-workers/service-worker/update-top-level.https.html diff --git a/dom/serviceworkers/ServiceWorkerRegistration.cpp b/dom/serviceworkers/ServiceWorkerRegistration.cpp index 16d1c8e3b0db..14ab1e4e4a0a 100644 --- a/dom/serviceworkers/ServiceWorkerRegistration.cpp +++ b/dom/serviceworkers/ServiceWorkerRegistration.cpp @@ -222,17 +222,6 @@ ServiceWorkerRegistration::Update(ErrorResult& aRv) return nullptr; } - if (RefPtr serviceWorkerGlobal = - do_QueryObject(global)) { - WorkerPrivate* wp; - if (serviceWorkerGlobal->Registration() == this && - (wp = GetCurrentThreadWorkerPrivate()) && - wp->IsLoadingWorkerScript()) { - outer->MaybeResolve(*this); - return outer.forget(); - } - } - RefPtr self = this; mPendingUpdatePromises += 1; diff --git a/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp b/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp index d9f5a5b92507..78ec762daf21 100644 --- a/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp +++ b/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp @@ -826,8 +826,13 @@ ServiceWorkerRegistrationWorkerThread::Update(ServiceWorkerRegistrationCallback& return; } - // This is ensured by the binding layer. - MOZ_ASSERT(!workerRef->Private()->IsLoadingWorkerScript()); + // Avoid infinite update loops by ignoring update() calls during top + // level script evaluation. See: + // https://github.com/slightlyoff/ServiceWorker/issues/800 + if (workerRef->Private()->IsLoadingWorkerScript()) { + aSuccessCB(mDescriptor); + return; + } auto promise = MakeRefPtr(__func__); auto holder = diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index 93aa66e7ba01..c46fd01cd3f3 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -677,7 +677,6 @@ SharedWorkerGlobalScope::Close() NS_IMPL_CYCLE_COLLECTION_INHERITED(ServiceWorkerGlobalScope, WorkerGlobalScope, mClients, mRegistration) NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ServiceWorkerGlobalScope) - NS_INTERFACE_MAP_ENTRY_CONCRETE(ServiceWorkerGlobalScope) NS_INTERFACE_MAP_END_INHERITING(WorkerGlobalScope) NS_IMPL_ADDREF_INHERITED(ServiceWorkerGlobalScope, WorkerGlobalScope) diff --git a/dom/workers/WorkerScope.h b/dom/workers/WorkerScope.h index 15ae99bcad05..c0712ed6ac4d 100644 --- a/dom/workers/WorkerScope.h +++ b/dom/workers/WorkerScope.h @@ -302,9 +302,6 @@ public: IMPL_EVENT_HANDLER(connect) }; -#define NS_DOM_SERVICEWORKERGLOBALSCOPE_IID \ - {0x552bfa7e, 0x0dd5, 0x4e94, {0xa0, 0x43, 0xff, 0x34, 0x6b, 0x6e, 0x04, 0x46}} - class ServiceWorkerGlobalScope final : public WorkerGlobalScope { const nsString mScope; @@ -314,7 +311,6 @@ class ServiceWorkerGlobalScope final : public WorkerGlobalScope ~ServiceWorkerGlobalScope(); public: - NS_DECLARE_STATIC_IID_ACCESSOR(NS_DOM_SERVICEWORKERGLOBALSCOPE_IID) NS_DECL_ISUPPORTS_INHERITED NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ServiceWorkerGlobalScope, WorkerGlobalScope) @@ -359,8 +355,6 @@ public: void EventListenerAdded(nsAtom* aType) override; }; -NS_DEFINE_STATIC_IID_ACCESSOR(ServiceWorkerGlobalScope, NS_DOM_SERVICEWORKERGLOBALSCOPE_IID) - class WorkerDebuggerGlobalScope final : public DOMEventTargetHelper, public nsIGlobalObject { diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json index ba33055c15d3..709ea158aef3 100644 --- a/testing/web-platform/meta/MANIFEST.json +++ b/testing/web-platform/meta/MANIFEST.json @@ -300278,11 +300278,6 @@ {} ] ], - "service-workers/service-worker/resources/update-top-level-worker.py": [ - [ - {} - ] - ], "service-workers/service-worker/resources/update-worker.py": [ [ {} @@ -390576,12 +390571,6 @@ {} ] ], - "service-workers/service-worker/update-top-level.https.html": [ - [ - "/service-workers/service-worker/update-top-level.https.html", - {} - ] - ], "service-workers/service-worker/update.https.html": [ [ "/service-workers/service-worker/update.https.html", @@ -632493,10 +632482,6 @@ "8aaa5ca934457714ee0e529ad4b2b1740d9758dd", "support" ], - "service-workers/service-worker/resources/update-top-level-worker.py": [ - "f77ef284ac0745bd6d31e642742438766f14e32e", - "support" - ], "service-workers/service-worker/resources/update-worker.py": [ "bc9b32ad3e68870d9f540524e70cd7947346e5c8", "support" @@ -632677,10 +632662,6 @@ "d8ed94f776650c8a40ba82df9ca5e909b460bb79", "testharness" ], - "service-workers/service-worker/update-top-level.https.html": [ - "e382028b44a9d19b26b3c15a3bba17fa6a0d9bcb", - "testharness" - ], "service-workers/service-worker/update.https.html": [ "6717d4d7ac289c8a18b1500e21795fd16c5321e7", "testharness" diff --git a/testing/web-platform/tests/service-workers/service-worker/resources/update-top-level-worker.py b/testing/web-platform/tests/service-workers/service-worker/resources/update-top-level-worker.py deleted file mode 100644 index f77ef284ac07..000000000000 --- a/testing/web-platform/tests/service-workers/service-worker/resources/update-top-level-worker.py +++ /dev/null @@ -1,18 +0,0 @@ -import time - -def main(request, response): - # no-cache itself to ensure the user agent finds a new version for each update. - headers = [('Cache-Control', 'no-cache, must-revalidate'), - ('Pragma', 'no-cache')] - content_type = 'application/javascript' - - headers.append(('Content-Type', content_type)) - - body = ''' -let promise = self.registration.update() -onmessage = (evt) => { - promise.then(r => { - evt.source.postMessage(self.registration === r ? 'PASS' : 'FAIL'); - }); -};''' - return headers, '/* %s %s */ %s' % (time.time(), time.clock(), body) diff --git a/testing/web-platform/tests/service-workers/service-worker/update-top-level.https.html b/testing/web-platform/tests/service-workers/service-worker/update-top-level.https.html deleted file mode 100644 index e382028b44a9..000000000000 --- a/testing/web-platform/tests/service-workers/service-worker/update-top-level.https.html +++ /dev/null @@ -1,32 +0,0 @@ - -Service Worker: Registration update() - - - -