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

dom/serviceworkers/ServiceWorkerRegistration.cpp

@@ -222,17 +222,6 @@ ServiceWorkerRegistration::Update(ErrorResult& aRv)
     return nullptr;
   }
 
-  if (RefPtr<ServiceWorkerGlobalScope> serviceWorkerGlobal =
-        do_QueryObject(global)) {
-    WorkerPrivate* wp;
-    if (serviceWorkerGlobal->Registration() == this &&
-        (wp = GetCurrentThreadWorkerPrivate()) &&
-        wp->IsLoadingWorkerScript()) {
-      outer->MaybeResolve(*this);
-      return outer.forget();
-    }
-  }
-
   RefPtr<ServiceWorkerRegistration> self = this;
 
   mPendingUpdatePromises += 1;

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<ServiceWorkerRegistrationPromise::Private>(__func__);
   auto holder =

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)

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
 {

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"

testing/web-platform/tests/service-workers/service-worker/resources/update-top-level-worker.py

@@ -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)

testing/web-platform/tests/service-workers/service-worker/update-top-level.https.html

@@ -1,32 +0,0 @@
-<!DOCTYPE html>
-<title>Service Worker: Registration update()</title>
-<script src="/resources/testharness.js"></script>
-<script src="/resources/testharnessreport.js"></script>
-<script src="resources/test-helpers.sub.js"></script>
-<script>
-'use strict';
-
-function wait_for_message() {
-  return new Promise(resolve => {
-    navigator.serviceWorker.addEventListener("message",
-      e => {
-        resolve(e.data);
-      }, { once: true });
-  });
-}
-
-promise_test(async t => {
-  const script = './resources/update-top-level-worker.py';
-  const scope = './resources/empty.html?update-result';
-
-  let reg = await navigator.serviceWorker.register(script, { scope });
-  t.add_cleanup(async _ => await reg.unregister());
-  await wait_for_state(t, reg.installing, 'activated');
-
-  reg.addEventListener("updatefound",
-    () => assert_unreached("shouldn't find an update"));
-
-  reg.active.postMessage("ping");
-  assert_equals(await wait_for_message(), 'PASS', 'did not hang');
-}, 'A serviceworker with a top-level update should not hang');
-</script>