Bug 1472303 - Block self-update from top level scripts. r=asuth
authorBlake Kaplan <mrbkap@gmail.com>
Wed, 22 Aug 2018 20:00:19 +0000
changeset 490671 8a40d04dfcbb0f272bfa6696354fd87d982b7a9a
parent 490670 dfd436600d63ce0a741c014e042e166be97e2e73
child 490672 e2cedc15272d164e81a4f48cb7115ff012430745
push id1815
push userffxbld-merge
push dateMon, 15 Oct 2018 10:40:45 +0000
treeherdermozilla-release@18d4c09e9378 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1472303
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1472303 - Block self-update from top level scripts. r=asuth Differential Revision: https://phabricator.services.mozilla.com/D3221
dom/serviceworkers/ServiceWorkerRegistration.cpp
dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp
dom/workers/WorkerScope.cpp
dom/workers/WorkerScope.h
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/service-workers/service-worker/resources/update-top-level-worker.py
testing/web-platform/tests/service-workers/service-worker/update-top-level.https.html
--- a/dom/serviceworkers/ServiceWorkerRegistration.cpp
+++ b/dom/serviceworkers/ServiceWorkerRegistration.cpp
@@ -217,16 +217,27 @@ ServiceWorkerRegistration::Update(ErrorR
     return nullptr;
   }
 
   RefPtr<Promise> outer = Promise::Create(global, aRv);
   if (NS_WARN_IF(aRv.Failed())) {
     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;
 
   mInner->Update(
     [outer, self](const ServiceWorkerRegistrationDescriptor& aDesc) {
       auto scopeExit = MakeScopeExit([&] { self->UpdatePromiseSettled(); });
       nsIGlobalObject* global = self->GetParentObject();
--- a/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp
+++ b/dom/serviceworkers/ServiceWorkerRegistrationImpl.cpp
@@ -821,23 +821,18 @@ ServiceWorkerRegistrationWorkerThread::U
 
   // Eventually we need to support all workers, but for right now this
   // code assumes we're on a service worker global as self.registration.
   if (NS_WARN_IF(!workerRef->Private()->IsServiceWorker())) {
     aFailureCB(CopyableErrorResult(NS_ERROR_DOM_INVALID_STATE_ERR));
     return;
   }
 
-  // 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;
-  }
+  // This is ensured by the binding layer.
+  MOZ_ASSERT(!workerRef->Private()->IsLoadingWorkerScript());
 
   auto promise = MakeRefPtr<ServiceWorkerRegistrationPromise::Private>(__func__);
   auto holder =
     MakeRefPtr<DOMMozPromiseRequestHolder<ServiceWorkerRegistrationPromise>>(global);
 
   promise->Then(
     global->EventTargetFor(TaskCategory::Other), __func__,
     [successCB = std::move(aSuccessCB), holder] (const ServiceWorkerRegistrationDescriptor& aDescriptor) {
--- a/dom/workers/WorkerScope.cpp
+++ b/dom/workers/WorkerScope.cpp
@@ -672,16 +672,17 @@ SharedWorkerGlobalScope::Close()
 {
   mWorkerPrivate->AssertIsOnWorkerThread();
   mWorkerPrivate->CloseInternal();
 }
 
 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)
 NS_IMPL_RELEASE_INHERITED(ServiceWorkerGlobalScope, WorkerGlobalScope)
 
 ServiceWorkerGlobalScope::ServiceWorkerGlobalScope(WorkerPrivate* aWorkerPrivate,
                                                    const ServiceWorkerRegistrationDescriptor& aRegistrationDescriptor)
   : WorkerGlobalScope(aWorkerPrivate)
--- a/dom/workers/WorkerScope.h
+++ b/dom/workers/WorkerScope.h
@@ -297,25 +297,29 @@ public:
   }
 
   void
   Close();
 
   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;
   RefPtr<Clients> mClients;
   RefPtr<ServiceWorkerRegistration> mRegistration;
 
   ~ServiceWorkerGlobalScope();
 
 public:
+  NS_DECLARE_STATIC_IID_ACCESSOR(NS_DOM_SERVICEWORKERGLOBALSCOPE_IID)
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(ServiceWorkerGlobalScope,
                                            WorkerGlobalScope)
   IMPL_EVENT_HANDLER(notificationclick)
   IMPL_EVENT_HANDLER(notificationclose)
 
   ServiceWorkerGlobalScope(WorkerPrivate* aWorkerPrivate,
                            const ServiceWorkerRegistrationDescriptor& aRegistrationDescriptor);
@@ -350,16 +354,18 @@ public:
   GetOnfetch();
 
   void
   SetOnfetch(mozilla::dom::EventHandlerNonNull* aCallback);
 
   void EventListenerAdded(nsAtom* aType) override;
 };
 
+NS_DEFINE_STATIC_IID_ACCESSOR(ServiceWorkerGlobalScope, NS_DOM_SERVICEWORKERGLOBALSCOPE_IID)
+
 class WorkerDebuggerGlobalScope final : public DOMEventTargetHelper,
                                         public nsIGlobalObject
 {
   WorkerPrivate* mWorkerPrivate;
   RefPtr<Console> mConsole;
   nsCOMPtr<nsISerialEventTarget> mSerialEventTarget;
 
 public:
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -300154,16 +300154,21 @@
      {}
     ]
    ],
    "service-workers/service-worker/resources/update-recovery-worker.py": [
     [
      {}
     ]
    ],
+   "service-workers/service-worker/resources/update-top-level-worker.py": [
+    [
+     {}
+    ]
+   ],
    "service-workers/service-worker/resources/update-worker.py": [
     [
      {}
     ]
    ],
    "service-workers/service-worker/resources/update/update-after-oneday.https.html": [
     [
      {}
@@ -390345,16 +390350,22 @@
     ]
    ],
    "service-workers/service-worker/update-result.https.html": [
     [
      "/service-workers/service-worker/update-result.https.html",
      {}
     ]
    ],
+   "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",
      {}
     ]
    ],
    "service-workers/service-worker/waiting.https.html": [
     [
@@ -632132,16 +632143,20 @@
   "service-workers/service-worker/resources/update-nocookie-worker.py": [
    "0f09b7e32c03c3724cbb55e88f19c0b9b4d78db7",
    "support"
   ],
   "service-workers/service-worker/resources/update-recovery-worker.py": [
    "8aaa5ca934457714ee0e529ad4b2b1740d9758dd",
    "support"
   ],
+  "service-workers/service-worker/resources/update-top-level-worker.py": [
+   "f77ef284ac0745bd6d31e642742438766f14e32e",
+   "support"
+  ],
   "service-workers/service-worker/resources/update-worker.py": [
    "bc9b32ad3e68870d9f540524e70cd7947346e5c8",
    "support"
   ],
   "service-workers/service-worker/resources/update/update-after-oneday.https.html": [
    "9d4c98272187e52947e710189c9fa1cfafbf4b22",
    "support"
   ],
@@ -632312,16 +632327,20 @@
   "service-workers/service-worker/update-recovery.https.html": [
    "3b3d955b142bff67c3d2c2a2d1742888c661c69d",
    "testharness"
   ],
   "service-workers/service-worker/update-result.https.html": [
    "d8ed94f776650c8a40ba82df9ca5e909b460bb79",
    "testharness"
   ],
+  "service-workers/service-worker/update-top-level.https.html": [
+   "1f0bdff65597832c7e3ba27c37681b760decf456",
+   "testharness"
+  ],
   "service-workers/service-worker/update.https.html": [
    "6717d4d7ac289c8a18b1500e21795fd16c5321e7",
    "testharness"
   ],
   "service-workers/service-worker/waiting.https.html": [
    "499e581eb353a2e433580e413a730bea2a9b7ad9",
    "testharness"
   ],
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/service-workers/service-worker/resources/update-top-level-worker.py
@@ -0,0 +1,18 @@
+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)
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/service-workers/service-worker/update-top-level.https.html
@@ -0,0 +1,32 @@
+<!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>