Bug 1438211 P7 Hold the ServiceWorker object alive until either the window is closed or the backing ServiceWorkerInfo becomes redundant. r=asuth
authorBen Kelly <ben@wanderview.com>
Fri, 02 Mar 2018 13:02:50 -0800
changeset 407229 076cd67616db
parent 407228 43c8c3b917ea
child 407230 bcea03ca0cda
push id33596
push userncsoregi@mozilla.com
push dateFri, 09 Mar 2018 00:18:11 +0000
treeherdermozilla-central@31a33fc61956 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1438211
milestone60.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 1438211 P7 Hold the ServiceWorker object alive until either the window is closed or the backing ServiceWorkerInfo becomes redundant. r=asuth
dom/serviceworkers/ServiceWorker.cpp
dom/serviceworkers/ServiceWorker.h
dom/serviceworkers/ServiceWorkerInfo.cpp
dom/serviceworkers/ServiceWorkerInfo.h
--- a/dom/serviceworkers/ServiceWorker.cpp
+++ b/dom/serviceworkers/ServiceWorker.cpp
@@ -78,17 +78,19 @@ ServiceWorker::ServiceWorker(nsIGlobalOb
 
   // This will update our state too.
   mInner->AddServiceWorker(this);
 }
 
 ServiceWorker::~ServiceWorker()
 {
   MOZ_ASSERT(NS_IsMainThread());
-  mInner->RemoveServiceWorker(this);
+  if (mInner) {
+    mInner->RemoveServiceWorker(this);
+  }
 }
 
 NS_IMPL_ADDREF_INHERITED(ServiceWorker, DOMEventTargetHelper)
 NS_IMPL_RELEASE_INHERITED(ServiceWorker, DOMEventTargetHelper)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(ServiceWorker)
   NS_INTERFACE_MAP_ENTRY(ServiceWorker)
 NS_INTERFACE_MAP_END_INHERITING(DOMEventTargetHelper)
@@ -123,17 +125,17 @@ ServiceWorker::GetScriptURL(nsString& aU
   CopyUTF8toUTF16(mDescriptor.ScriptURL(), aURL);
 }
 
 void
 ServiceWorker::PostMessage(JSContext* aCx, JS::Handle<JS::Value> aMessage,
                            const Sequence<JSObject*>& aTransferable,
                            ErrorResult& aRv)
 {
-  if (State() == ServiceWorkerState::Redundant) {
+  if (State() == ServiceWorkerState::Redundant || !mInner) {
     aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
     return;
   }
 
   mInner->PostMessage(GetParentObject(), aCx, aMessage, aTransferable, aRv);
 }
 
 
@@ -141,13 +143,17 @@ const ServiceWorkerDescriptor&
 ServiceWorker::Descriptor() const
 {
   return mDescriptor;
 }
 
 void
 ServiceWorker::DisconnectFromOwner()
 {
+  if (mInner) {
+    mInner->RemoveServiceWorker(this);
+    mInner = nullptr;
+  }
   DOMEventTargetHelper::DisconnectFromOwner();
 }
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/serviceworkers/ServiceWorker.h
+++ b/dom/serviceworkers/ServiceWorker.h
@@ -97,17 +97,22 @@ private:
   ServiceWorker(nsIGlobalObject* aWindow,
                 const ServiceWorkerDescriptor& aDescriptor,
                 Inner* aInner);
 
   // This class is reference-counted and will be destroyed from Release().
   ~ServiceWorker();
 
   ServiceWorkerDescriptor mDescriptor;
-  const RefPtr<Inner> mInner;
+
+  // Hold a strong reference to the inner service worker object.  This will
+  // create a ref-cycle.  The cycle is broken when either DisconnectFromOwner()
+  // is called due to the global tearing down or when the underlying service
+  // worker transitions to the redundant state.
+  RefPtr<Inner> mInner;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(ServiceWorker, NS_DOM_SERVICEWORKER_IID)
 
 } // namespace dom
 } // namespace mozilla
 
 #endif // mozilla_dom_serviceworker_h__
--- a/dom/serviceworkers/ServiceWorkerInfo.cpp
+++ b/dom/serviceworkers/ServiceWorkerInfo.cpp
@@ -113,17 +113,17 @@ ServiceWorkerInfo::DetachDebugger()
   return mServiceWorkerPrivate->DetachDebugger();
 }
 
 namespace {
 
 class ChangeStateUpdater final : public Runnable
 {
 public:
-  ChangeStateUpdater(const nsTArray<ServiceWorker*>& aInstances,
+  ChangeStateUpdater(const nsTArray<RefPtr<ServiceWorker>>& aInstances,
                      ServiceWorkerState aState)
     : Runnable("dom::ChangeStateUpdater")
     , mState(aState)
   {
     for (size_t i = 0; i < aInstances.Length(); ++i) {
       mInstances.AppendElement(aInstances[i]);
     }
   }
@@ -171,16 +171,21 @@ ServiceWorkerInfo::UpdateState(ServiceWo
   if (State() != aState) {
     mServiceWorkerPrivate->UpdateState(aState);
   }
   mDescriptor.SetState(aState);
   nsCOMPtr<nsIRunnable> r = new ChangeStateUpdater(mInstances, State());
   MOZ_ALWAYS_SUCCEEDS(NS_DispatchToMainThread(r.forget()));
   if (State() == ServiceWorkerState::Redundant) {
     serviceWorkerScriptCache::PurgeCache(mPrincipal, mCacheName);
+
+    // Break the ref-cycle with the binding objects.  We won't need to
+    // fire any more events and they should be able to GC once content
+    // script no longer references them.
+    mInstances.Clear();
   }
 }
 
 ServiceWorkerInfo::ServiceWorkerInfo(nsIPrincipal* aPrincipal,
                                      const nsACString& aScope,
                                      const nsACString& aScriptSpec,
                                      const nsAString& aCacheName,
                                      nsLoadFlags aImportsLoadFlags)
@@ -245,18 +250,21 @@ ServiceWorkerInfo::RemoveServiceWorker(S
 {
   MOZ_DIAGNOSTIC_ASSERT(aWorker);
 #ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
   nsAutoString workerURL;
   aWorker->GetScriptURL(workerURL);
   MOZ_DIAGNOSTIC_ASSERT(
     workerURL.Equals(NS_ConvertUTF8toUTF16(mDescriptor.ScriptURL())));
 #endif
-  MOZ_ASSERT(mInstances.Contains(aWorker));
 
+  // If the binding layer initiates this call by disconnecting the global,
+  // then we will find an entry in mInstances here.  If the worker transitions
+  // to redundant and we clear mInstances, then we will not find an entry
+  // here.
   mInstances.RemoveElement(aWorker);
 }
 
 void
 ServiceWorkerInfo::PostMessage(nsIGlobalObject* aGlobal,
                                JSContext* aCx, JS::Handle<JS::Value> aMessage,
                                const Sequence<JSObject*>& aTransferable,
                                ErrorResult& aRv)
--- a/dom/serviceworkers/ServiceWorkerInfo.h
+++ b/dom/serviceworkers/ServiceWorkerInfo.h
@@ -49,21 +49,26 @@ private:
   TimeStamp mCreationTimeStamp;
 
   // The time of states are 0, if SW has not reached that state yet. Besides, we
   // update each of them after UpdateState() is called in SWRegistrationInfo.
   PRTime mInstalledTime;
   PRTime mActivatedTime;
   PRTime mRedundantTime;
 
-  // We hold rawptrs since the ServiceWorker constructor and destructor ensure
-  // addition and removal.
+  // Track the list of known binding objects so we can fire events on them
+  // when appropriate.  These are held using strong references so that they
+  // are not GC'd while an event handler is registered and could observe an
+  // event.  This reference will create a cycle with the binding object.  The
+  // cycle is broken when either the global is detached or the service worker
+  // transitions to the redundant state.
+  //
   // There is a high chance of there being at least one ServiceWorker
   // associated with this all the time.
-  AutoTArray<ServiceWorker*, 1> mInstances;
+  AutoTArray<RefPtr<ServiceWorker>, 1> mInstances;
 
   RefPtr<ServiceWorkerPrivate> mServiceWorkerPrivate;
   bool mSkipWaitingFlag;
 
   enum {
     Unknown,
     Enabled,
     Disabled