diff --git a/dom/cache/Manager.cpp b/dom/cache/Manager.cpp index 3285ecd4408f..332576cccd3b 100644 --- a/dom/cache/Manager.cpp +++ b/dom/cache/Manager.cpp @@ -1411,9 +1411,7 @@ Manager::RemoveListener(Listener* aListener) mListeners.RemoveElement(aListener, ListenerEntryListenerComparator()); MOZ_ASSERT(!mListeners.Contains(aListener, ListenerEntryListenerComparator())); - if (mListeners.IsEmpty() && mContext) { - mContext->AllowToClose(); - } + MaybeAllowContextToClose(); } void @@ -1422,6 +1420,12 @@ Manager::RemoveContext(Context* aContext) NS_ASSERT_OWNINGTHREAD(Manager); MOZ_ASSERT(mContext); MOZ_ASSERT(mContext == aContext); + + // Whether the Context destruction was triggered from the Manager going + // idle or the underlying storage being invalidated, we should be invalid + // when the context is destroyed. + MOZ_ASSERT(!mValid); + mContext = nullptr; // If we're trying to shutdown, then note that we're done. This is the @@ -1483,6 +1487,7 @@ Manager::ReleaseCacheId(CacheId aCacheId) context->Dispatch(mIOThread, action); } } + MaybeAllowContextToClose(); return; } } @@ -1524,6 +1529,7 @@ Manager::ReleaseBodyId(const nsID& aBodyId) context->Dispatch(mIOThread, action); } } + MaybeAllowContextToClose(); return; } } @@ -1906,6 +1912,29 @@ Manager::NoteOrphanedBodyIdList(const nsTArray& aDeletedBodyIdList) } } +void +Manager::MaybeAllowContextToClose() +{ + NS_ASSERT_OWNINGTHREAD(Manager); + + // If we have an active context, but we have no more users of the Manager, + // then let it shut itself down. We must wait for all possible users of + // Cache state information to complete before doing this. Once we allow + // the Context to close we may not reliably get notified of storage + // invalidation. + if (mContext && mListeners.IsEmpty() + && mCacheIdRefs.IsEmpty() + && mBodyIdRefs.IsEmpty()) { + + // Mark this Manager as invalid so that it won't get used again. We don't + // want to start any new operations once we allow the Context to close since + // it may race with the underlying storage getting invalidated. + mValid = false; + + mContext->AllowToClose(); + } +} + } // namespace cache } // namespace dom } // namespace mozilla diff --git a/dom/cache/Manager.h b/dom/cache/Manager.h index b59dc1b24519..55ea5e460c4e 100644 --- a/dom/cache/Manager.h +++ b/dom/cache/Manager.h @@ -41,23 +41,25 @@ class StreamList; // Cache API. This uniqueness is defined by the ManagerId equality operator. // The uniqueness is enforced by the Manager GetOrCreate() factory method. // -// The Manager object can out live the IPC actors in the case where the child -// process is killed; e.g a child process OOM. The Manager object can -// The Manager object can potentially use non-trivial resources. Long lived -// DOM objects and their actors should not maintain a reference to the Manager -// while idle. Transient DOM objects that may keep a reference for their -// lifetimes. +// The life cycle of Manager objects is somewhat complex. While code may +// hold a strong reference to the Manager, it will invalidate itself once it +// believes it has become completely idle. This is currently determined when +// all of the following conditions occur: // -// For example, once a CacheStorage DOM object is access it will live until its -// global is released. Therefore, CacheStorage should release its Manager -// reference after operations complete and it becomes idle. Cache objects, -// however, can be GC'd once content are done using them and can therefore keep -// their Manager reference alive. Its expected that more operations are -// performed on a Cache object, so keeping the Manager reference will help -// minimize overhead for each reference. +// 1) There are no more Manager::Listener objects registered with the Manager +// by performing a Cache or Storage operation. +// 2) There are no more CacheId references noted via Manager::AddRefCacheId(). +// 3) There are no more BodyId references noted via Manager::AddRefBodyId(). +// +// In order to keep your Manager alive you should perform an operation to set +// a Listener, call AddRefCacheId(), or call AddRefBodyId(). +// +// Even once a Manager becomes invalid, however, it may still continue to +// exist. This is allowed so that any in-progress Actions can gracefully +// complete. // // As an invariant, all Manager objects must cease all IO before shutdown. This -// is enforced by the ShutdownObserver. If content still holds references to +// is enforced by the Manager::Factory. If content still holds references to // Cache DOM objects during shutdown, then all operations will begin rejecting. class Manager final { @@ -209,6 +211,8 @@ private: bool SetBodyIdOrphanedIfRefed(const nsID& aBodyId); void NoteOrphanedBodyIdList(const nsTArray& aDeletedBodyIdList); + void MaybeAllowContextToClose(); + nsRefPtr mManagerId; nsCOMPtr mIOThread;