Bug 1193414 - SharedWorkers thread should be kept alive also when the SharedWorker object is CCed, r=khuey, a=lizzard
authorAndrea Marchesini <amarchesini@mozilla.com>
Wed, 23 Sep 2015 06:54:29 +0100
changeset 296259 36b3f1e21a29b4429a913cb5db8eb7a21202d93d
parent 296258 719d1e17617147517985be6fed77446b9df1af70
child 296260 5bacade79fc5d26ce8c2fc50adc1d084d342eab2
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, lizzard
bugs1193414
milestone43.0a2
Bug 1193414 - SharedWorkers thread should be kept alive also when the SharedWorker object is CCed, r=khuey, a=lizzard
dom/workers/RuntimeService.cpp
dom/workers/SharedWorker.cpp
dom/workers/SharedWorker.h
dom/workers/WorkerPrivate.cpp
dom/workers/WorkerPrivate.h
dom/workers/test/mochitest.ini
dom/workers/test/sharedWorker_lifetime.js
dom/workers/test/test_sharedWorker_lifetime.html
--- a/dom/workers/RuntimeService.cpp
+++ b/dom/workers/RuntimeService.cpp
@@ -1689,26 +1689,20 @@ RuntimeService::UnregisterWorker(JSConte
   }
 
   if (aWorkerPrivate->IsServiceWorker()) {
     AssertIsOnMainThread();
     Telemetry::AccumulateTimeDelta(Telemetry::SERVICE_WORKER_LIFE_TIME,
                                    aWorkerPrivate->CreationTimeStamp());
   }
 
-  if (aWorkerPrivate->IsSharedWorker()) {
+  if (aWorkerPrivate->IsSharedWorker() ||
+      aWorkerPrivate->IsServiceWorker()) {
     AssertIsOnMainThread();
-
-    nsAutoTArray<nsRefPtr<SharedWorker>, 5> sharedWorkersToNotify;
-    aWorkerPrivate->GetAllSharedWorkers(sharedWorkersToNotify);
-
-    for (uint32_t index = 0; index < sharedWorkersToNotify.Length(); index++) {
-      MOZ_ASSERT(sharedWorkersToNotify[index]);
-      sharedWorkersToNotify[index]->NoteDeadWorker(aCx);
-    }
+    aWorkerPrivate->CloseAllSharedWorkers();
   }
 
   if (parent) {
     parent->RemoveChildWorker(aCx, aWorkerPrivate);
   }
   else if (aWorkerPrivate->IsSharedWorker() || aWorkerPrivate->IsServiceWorker()) {
     mWindowMap.Enumerate(RemoveSharedWorkerFromWindowMap, aWorkerPrivate);
   }
--- a/dom/workers/SharedWorker.cpp
+++ b/dom/workers/SharedWorker.cpp
@@ -23,29 +23,29 @@ using mozilla::dom::Optional;
 using mozilla::dom::Sequence;
 using namespace mozilla;
 
 USING_WORKERS_NAMESPACE
 
 SharedWorker::SharedWorker(nsPIDOMWindow* aWindow,
                            WorkerPrivate* aWorkerPrivate,
                            MessagePort* aMessagePort)
-: DOMEventTargetHelper(aWindow), mWorkerPrivate(aWorkerPrivate)
-, mMessagePort(aMessagePort)
-, mFrozen(false)
+  : DOMEventTargetHelper(aWindow)
+  , mWorkerPrivate(aWorkerPrivate)
+  , mMessagePort(aMessagePort)
+  , mFrozen(false)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(aWorkerPrivate);
+  MOZ_ASSERT(aMessagePort);
 }
 
 SharedWorker::~SharedWorker()
 {
   AssertIsOnMainThread();
-  Close();
-  MOZ_ASSERT(!mWorkerPrivate);
 }
 
 // static
 already_AddRefed<SharedWorker>
 SharedWorker::Constructor(const GlobalObject& aGlobal, JSContext* aCx,
                           const nsAString& aScriptURL,
                           const mozilla::dom::Optional<nsAString>& aName,
                           ErrorResult& aRv)
@@ -131,63 +131,48 @@ SharedWorker::QueueEvent(nsIDOMEvent* aE
 
 void
 SharedWorker::Close()
 {
   AssertIsOnMainThread();
 
   if (mMessagePort) {
     mMessagePort->Close();
-  }
-
-  if (mWorkerPrivate) {
-    AutoSafeJSContext cx;
-    NoteDeadWorker(cx);
+    mMessagePort = nullptr;
   }
 }
 
 void
 SharedWorker::PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
                           const Optional<Sequence<JS::Value>>& aTransferable,
                           ErrorResult& aRv)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(mWorkerPrivate);
   MOZ_ASSERT(mMessagePort);
 
   mMessagePort->PostMessage(aCx, aMessage, aTransferable, aRv);
 }
 
-void
-SharedWorker::NoteDeadWorker(JSContext* aCx)
-{
-  AssertIsOnMainThread();
-  MOZ_ASSERT(mWorkerPrivate);
-
-  mWorkerPrivate->UnregisterSharedWorker(aCx, this);
-  mWorkerPrivate = nullptr;
-}
-
 NS_IMPL_ADDREF_INHERITED(SharedWorker, DOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(SharedWorker, DOMEventTargetHelper)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION_INHERITED(SharedWorker)
 NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
 
 NS_IMPL_CYCLE_COLLECTION_CLASS(SharedWorker)
 
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED(SharedWorker,
                                                   DOMEventTargetHelper)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mMessagePort)
   NS_IMPL_CYCLE_COLLECTION_TRAVERSE(mFrozenEvents)
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END
 
 NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(SharedWorker,
                                                 DOMEventTargetHelper)
-  tmp->Close();
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mMessagePort)
   NS_IMPL_CYCLE_COLLECTION_UNLINK(mFrozenEvents)
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 
 JSObject*
 SharedWorker::WrapObject(JSContext* aCx, JS::Handle<JSObject*> aGivenProto)
 {
   AssertIsOnMainThread();
--- a/dom/workers/SharedWorker.h
+++ b/dom/workers/SharedWorker.h
@@ -93,17 +93,13 @@ private:
   // This class is reference-counted and will be destroyed from Release().
   ~SharedWorker();
 
   // Only called by MessagePort.
   void
   PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
               const Optional<Sequence<JS::Value>>& aTransferable,
               ErrorResult& aRv);
-
-  // Only called by RuntimeService.
-  void
-  NoteDeadWorker(JSContext* aCx);
 };
 
 END_WORKERS_NAMESPACE
 
 #endif // mozilla_dom_workers_sharedworker_h__
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -2962,41 +2962,16 @@ WorkerPrivateParent<Derived>::RegisterSh
     return false;
   }
 
   return true;
 }
 
 template <class Derived>
 void
-WorkerPrivateParent<Derived>::UnregisterSharedWorker(
-                                                    JSContext* aCx,
-                                                    SharedWorker* aSharedWorker)
-{
-  AssertIsOnMainThread();
-  MOZ_ASSERT(aSharedWorker);
-  MOZ_ASSERT(IsSharedWorker() || IsServiceWorker());
-  MOZ_ASSERT(mSharedWorkers.Contains(aSharedWorker));
-
-  mSharedWorkers.RemoveElement(aSharedWorker);
-
-  // If there are still SharedWorker objects attached to this worker then they
-  // may all be frozen and this worker would need to be frozen. Otherwise,
-  // if that was the last SharedWorker then it's time to cancel this worker.
-  if (!mSharedWorkers.IsEmpty()) {
-    if (!Freeze(aCx, nullptr)) {
-      JS_ReportPendingException(aCx);
-    }
-  } else if (!Cancel(aCx)) {
-    JS_ReportPendingException(aCx);
-  }
-}
-
-template <class Derived>
-void
 WorkerPrivateParent<Derived>::BroadcastErrorToSharedWorkers(
                                                     JSContext* aCx,
                                                     const nsAString& aMessage,
                                                     const nsAString& aFilename,
                                                     const nsAString& aLine,
                                                     uint32_t aLineNumber,
                                                     uint32_t aColumnNumber,
                                                     uint32_t aFlags)
@@ -3132,29 +3107,65 @@ template <class Derived>
 void
 WorkerPrivateParent<Derived>::CloseSharedWorkersForWindow(
                                                          nsPIDOMWindow* aWindow)
 {
   AssertIsOnMainThread();
   MOZ_ASSERT(IsSharedWorker() || IsServiceWorker());
   MOZ_ASSERT(aWindow);
 
-  nsAutoTArray<nsRefPtr<SharedWorker>, 10> sharedWorkers;
-
-  for (uint32_t i = 0; i < mSharedWorkers.Length(); ++i) {
+  bool someRemoved = false;
+
+  for (uint32_t i = 0; i < mSharedWorkers.Length();) {
     if (mSharedWorkers[i]->GetOwner() == aWindow) {
-      sharedWorkers.AppendElement(mSharedWorkers[i]);
+      mSharedWorkers[i]->Close();
+      mSharedWorkers.RemoveElementAt(i);
+      someRemoved = true;
     } else {
       MOZ_ASSERT(!SameCOMIdentity(mSharedWorkers[i]->GetOwner(),
                                   aWindow));
-    }
-  }
-
-  for (uint32_t index = 0; index < sharedWorkers.Length(); index++) {
-    sharedWorkers[index]->Close();
+      ++i;
+    }
+  }
+
+  if (!someRemoved) {
+    return;
+  }
+
+  // If there are still SharedWorker objects attached to this worker then they
+  // may all be frozen and this worker would need to be frozen. Otherwise,
+  // if that was the last SharedWorker then it's time to cancel this worker.
+
+  AutoSafeJSContext cx;
+
+  if (!mSharedWorkers.IsEmpty()) {
+    if (!Freeze(cx, nullptr)) {
+      JS_ReportPendingException(cx);
+    }
+  } else if (!Cancel(cx)) {
+    JS_ReportPendingException(cx);
+  }
+}
+
+template <class Derived>
+void
+WorkerPrivateParent<Derived>::CloseAllSharedWorkers()
+{
+  AssertIsOnMainThread();
+  MOZ_ASSERT(IsSharedWorker() || IsServiceWorker());
+
+  for (uint32_t i = 0; i < mSharedWorkers.Length(); ++i) {
+    mSharedWorkers[i]->Close();
+  }
+
+  mSharedWorkers.Clear();
+
+  AutoSafeJSContext cx;
+  if (!Cancel(cx)) {
+    JS_ReportPendingException(cx);
   }
 }
 
 template <class Derived>
 void
 WorkerPrivateParent<Derived>::WorkerScriptLoaded()
 {
   AssertIsOnMainThread();
--- a/dom/workers/WorkerPrivate.h
+++ b/dom/workers/WorkerPrivate.h
@@ -173,17 +173,17 @@ private:
   // Only used for top level workers.
   nsTArray<nsCOMPtr<nsIRunnable>> mQueuedRunnables;
 
   // Protected by mMutex.
   JSSettings mJSSettings;
 
   // Only touched on the parent thread (currently this is always the main
   // thread as SharedWorkers are always top-level).
-  nsTArray<SharedWorker*> mSharedWorkers;
+  nsTArray<nsRefPtr<SharedWorker>> mSharedWorkers;
 
   uint64_t mBusyCount;
   Status mParentStatus;
   bool mParentFrozen;
   bool mIsChromeWorker;
   bool mMainThreadObjectsForgotten;
   WorkerType mWorkerType;
   TimeStamp mCreationTimeStamp;
@@ -364,19 +364,16 @@ public:
   void
   OfflineStatusChangeEvent(JSContext* aCx, bool aIsOffline);
 
   bool
   RegisterSharedWorker(JSContext* aCx, SharedWorker* aSharedWorker,
                        MessagePort* aPort);
 
   void
-  UnregisterSharedWorker(JSContext* aCx, SharedWorker* aSharedWorker);
-
-  void
   BroadcastErrorToSharedWorkers(JSContext* aCx,
                                 const nsAString& aMessage,
                                 const nsAString& aFilename,
                                 const nsAString& aLine,
                                 uint32_t aLineNumber,
                                 uint32_t aColumnNumber,
                                 uint32_t aFlags);
 
@@ -766,16 +763,19 @@ public:
 
   void
   GetAllSharedWorkers(nsTArray<nsRefPtr<SharedWorker>>& aSharedWorkers);
 
   void
   CloseSharedWorkersForWindow(nsPIDOMWindow* aWindow);
 
   void
+  CloseAllSharedWorkers();
+
+  void
   UpdateOverridenLoadGroup(nsILoadGroup* aBaseLoadGroup);
 
   already_AddRefed<nsIRunnable>
   StealLoadFailedAsyncRunnable()
   {
     return mLoadInfo.mLoadFailedAsyncRunnable.forget();
   }
 
--- a/dom/workers/test/mochitest.ini
+++ b/dom/workers/test/mochitest.ini
@@ -106,16 +106,17 @@ support-files =
   bug1132924_worker.js
   empty.html
   worker_performance_user_timing.js
   worker_performance_observer.js
   sharedworker_performance_user_timing.js
   referrer.sjs
   performance_observer.html
   sharedWorker_ports.js
+  sharedWorker_lifetime.js
 
 [test_404.html]
 [test_atob.html]
 [test_blobConstructor.html]
 [test_blobWorkers.html]
 [test_bug949946.html]
 [test_bug978260.html]
 [test_bug998474.html]
@@ -218,8 +219,9 @@ skip-if = buildapp == 'b2g' || e10s
 [test_xhr_responseURL.html]
 [test_xhr_system.html]
 skip-if = buildapp == 'b2g' || e10s
 [test_xhr_timeout.html]
 skip-if = (os == "win") || (os == "mac") || toolkit == 'android' || e10s #bug 798220
 [test_xhrAbort.html]
 [test_referrer.html]
 [test_sharedWorker_ports.html]
+[test_sharedWorker_lifetime.html]
new file mode 100644
--- /dev/null
+++ b/dom/workers/test/sharedWorker_lifetime.js
@@ -0,0 +1,5 @@
+onconnect = function(e) {
+  setTimeout(function() {
+    e.ports[0].postMessage("Still alive!");
+  }, 500);
+}
new file mode 100644
--- /dev/null
+++ b/dom/workers/test/test_sharedWorker_lifetime.html
@@ -0,0 +1,32 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <title>Test for MessagePort and SharedWorkers</title>
+    <meta http-equiv="content-type" content="text/html; charset=UTF-8">
+    <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  </head>
+  <body>
+    <script class="testbody" type="text/javascript">
+
+var gced = false;
+SpecialPowers.pushPrefEnv({ set: [["dom.workers.sharedWorkers.enabled", true]]},
+function() {
+  var sw = new SharedWorker('sharedWorker_lifetime.js');
+  sw.port.onmessage = function(event) {
+    ok(gced, "The SW is still alive also after GC");
+    SimpleTest.finish();
+  }
+
+  sw = null;
+  SpecialPowers.forceGC();
+  gced = true;
+});
+
+SimpleTest.waitForExplicitFinish();
+    </script>
+  </body>
+</html>