diff --git a/dom/base/EventSource.cpp b/dom/base/EventSource.cpp index d0507aacce19..741d442f5238 100644 --- a/dom/base/EventSource.cpp +++ b/dom/base/EventSource.cpp @@ -15,6 +15,7 @@ #include "mozilla/dom/MessageEventBinding.h" #include "mozilla/dom/ScriptSettings.h" #include "mozilla/dom/WorkerPrivate.h" +#include "mozilla/dom/WorkerRef.h" #include "mozilla/dom/WorkerRunnable.h" #include "mozilla/dom/WorkerScope.h" #include "mozilla/UniquePtrExtensions.h" @@ -143,8 +144,8 @@ public: void AddRefObject(); void ReleaseObject(); - bool RegisterWorkerHolder(); - void UnregisterWorkerHolder(); + bool CreateWorkerRef(WorkerPrivate* aWorkerPrivate); + void ReleaseWorkerRef(); void AssertIsOnTargetThread() const { @@ -282,11 +283,8 @@ public: nsString mLastFieldValue; // EventSourceImpl internal states. - // The worker private where the EventSource is created. nullptr if created on - // main thread. (accessed on worker thread only) - WorkerPrivate* mWorkerPrivate; - // Holder to worker to keep worker alive. (accessed on worker thread only) - nsAutoPtr mWorkerHolder; + // WorkerRef to keep the worker alive. (accessed on worker thread only) + RefPtr mWorkerRef; // This mutex protects mFrozen and mEventSource->mReadyState that are used in // different threads. mozilla::Mutex mMutex; @@ -353,8 +351,6 @@ EventSourceImpl::EventSourceImpl(EventSource* aEventSource) { MOZ_ASSERT(mEventSource); if (!mIsMainThread) { - mWorkerPrivate = GetCurrentThreadWorkerPrivate(); - MOZ_ASSERT(mWorkerPrivate); mEventSource->mIsMainThread = false; } SetReadyState(CONNECTING); @@ -364,11 +360,11 @@ class CleanupRunnable final : public WorkerMainThreadRunnable { public: explicit CleanupRunnable(EventSourceImpl* aEventSourceImpl) - : WorkerMainThreadRunnable(aEventSourceImpl->mWorkerPrivate, + : WorkerMainThreadRunnable(GetCurrentThreadWorkerPrivate(), NS_LITERAL_CSTRING("EventSource :: Cleanup")) , mImpl(aEventSourceImpl) { - mImpl->mWorkerPrivate->AssertIsOnWorkerThread(); + mWorkerPrivate->AssertIsOnWorkerThread(); } bool MainThreadRun() override @@ -413,7 +409,7 @@ EventSourceImpl::CloseInternal() RefPtr runnable = new CleanupRunnable(this); runnable->Dispatch(Killing, rv); MOZ_ASSERT(!rv.Failed()); - UnregisterWorkerHolder(); + ReleaseWorkerRef(); } while (mMessagesToDispatch.GetSize() != 0) { @@ -452,20 +448,22 @@ void EventSourceImpl::CleanupOnMainThread() class InitRunnable final : public WorkerMainThreadRunnable { public: - explicit InitRunnable(EventSourceImpl* aEventSourceImpl, - const nsAString& aURL) - : WorkerMainThreadRunnable(aEventSourceImpl->mWorkerPrivate, + InitRunnable(WorkerPrivate* aWorkerPrivate, + EventSourceImpl* aEventSourceImpl, + const nsAString& aURL) + : WorkerMainThreadRunnable(aWorkerPrivate, NS_LITERAL_CSTRING("EventSource :: Init")) , mImpl(aEventSourceImpl) , mURL(aURL) { - mImpl->mWorkerPrivate->AssertIsOnWorkerThread(); + MOZ_ASSERT(aWorkerPrivate); + aWorkerPrivate->AssertIsOnWorkerThread(); } bool MainThreadRun() override { // Get principal from worker's owner document or from worker. - WorkerPrivate* wp = mImpl->mWorkerPrivate; + WorkerPrivate* wp = mWorkerPrivate; while (wp->GetParent()) { wp = wp->GetParent(); } @@ -485,13 +483,36 @@ public: nsresult ErrorCode() const { return mRv; } -protected: +private: // Raw pointer because this runnable is sync. EventSourceImpl* mImpl; const nsAString& mURL; nsresult mRv; }; +class ConnectRunnable final : public WorkerMainThreadRunnable +{ +public: + explicit ConnectRunnable(WorkerPrivate* aWorkerPrivate, + EventSourceImpl* aEventSourceImpl) + : WorkerMainThreadRunnable(aWorkerPrivate, + NS_LITERAL_CSTRING("EventSource :: Connect")) + , mImpl(aEventSourceImpl) + { + MOZ_ASSERT(aWorkerPrivate); + aWorkerPrivate->AssertIsOnWorkerThread(); + } + + bool MainThreadRun() override + { + mImpl->InitChannelAndRequestEventSource(); + return true; + } + +private: + RefPtr mImpl; +}; + nsresult EventSourceImpl::ParseURL(const nsAString& aURL) { @@ -585,11 +606,6 @@ EventSourceImpl::Init(nsIPrincipal* aPrincipal, DEFAULT_RECONNECTION_TIME_VALUE); mUnicodeDecoder = UTF_8_ENCODING->NewDecoderWithBOMRemoval(); - - // the constructor should throw a SYNTAX_ERROR only if it fails resolving the - // url parameter, so we don't care about the InitChannelAndRequestEventSource - // result. - InitChannelAndRequestEventSource(); } //----------------------------------------------------------------------------- @@ -1131,11 +1147,11 @@ class CallRestartConnection final : public WorkerMainThreadRunnable public: explicit CallRestartConnection(EventSourceImpl* aEventSourceImpl) : WorkerMainThreadRunnable( - aEventSourceImpl->mWorkerPrivate, + aEventSourceImpl->mWorkerRef->Private(), NS_LITERAL_CSTRING("EventSource :: RestartConnection")) , mImpl(aEventSourceImpl) { - mImpl->mWorkerPrivate->AssertIsOnWorkerThread(); + mWorkerPrivate->AssertIsOnWorkerThread(); } bool MainThreadRun() override @@ -1476,8 +1492,8 @@ EventSourceImpl::DispatchAllMessageEvents() return; } } else { - MOZ_ASSERT(mWorkerPrivate); - if (NS_WARN_IF(!jsapi.Init(mWorkerPrivate->GlobalScope()))) { + MOZ_ASSERT(mWorkerRef); + if (NS_WARN_IF(!jsapi.Init(mWorkerRef->Private()->GlobalScope()))) { return; } } @@ -1777,28 +1793,6 @@ EventSourceImpl::ReleaseObject() } namespace { -class EventSourceWorkerHolder final : public WorkerHolder -{ -public: - explicit EventSourceWorkerHolder(EventSourceImpl* aEventSourceImpl) - : WorkerHolder("EventSourceWorkerHolder") - , mEventSourceImpl(aEventSourceImpl) - { - } - - bool Notify(WorkerStatus aStatus) override - { - MOZ_ASSERT(aStatus > Running); - if (aStatus >= Canceling) { - mEventSourceImpl->Close(); - } - return true; - } - -private: - // Raw pointer because the EventSourceImpl object keeps alive the holder. - EventSourceImpl* mEventSourceImpl; -}; class WorkerRunnableDispatcher final : public WorkerRunnable { @@ -1849,30 +1843,32 @@ private: } // namespace -bool EventSourceImpl::RegisterWorkerHolder() +bool EventSourceImpl::CreateWorkerRef(WorkerPrivate* aWorkerPrivate) { MOZ_ASSERT(!IsShutDown()); - MOZ_ASSERT(mWorkerPrivate); - mWorkerPrivate->AssertIsOnWorkerThread(); - MOZ_ASSERT(!mWorkerHolder); - mWorkerHolder = new EventSourceWorkerHolder(this); - if (NS_WARN_IF(!mWorkerHolder->HoldWorker(mWorkerPrivate, Canceling))) { - mWorkerHolder = nullptr; + MOZ_ASSERT(!mWorkerRef); + MOZ_ASSERT(aWorkerPrivate); + aWorkerPrivate->AssertIsOnWorkerThread(); + + RefPtr self = this; + RefPtr workerRef = + StrongWorkerRef::Create(aWorkerPrivate, "EventSource", [self]() { + self->Close(); + }); + + if (NS_WARN_IF(!workerRef)) { return false; } + + mWorkerRef = new ThreadSafeWorkerRef(workerRef); return true; } -void EventSourceImpl::UnregisterWorkerHolder() +void EventSourceImpl::ReleaseWorkerRef() { - // RegisterWorkerHolder fail will destroy EventSourceImpl and invoke - // UnregisterWorkerHolder. MOZ_ASSERT(IsClosed()); - MOZ_ASSERT(mWorkerPrivate); - mWorkerPrivate->AssertIsOnWorkerThread(); - // The DTOR of this WorkerHolder will release the worker for us. - mWorkerHolder = nullptr; - mWorkerPrivate = nullptr; + MOZ_ASSERT(IsCurrentThreadRunningWorker()); + mWorkerRef = nullptr; } //----------------------------------------------------------------------------- @@ -1896,11 +1892,11 @@ EventSourceImpl::Dispatch(already_AddRefed aEvent, uint32_t aFlags) if (IsShutDown()) { return NS_OK; } - MOZ_ASSERT(mWorkerPrivate); + // If the target is a worker, we have to use a custom WorkerRunnableDispatcher // runnable. RefPtr event = - new WorkerRunnableDispatcher(this, mWorkerPrivate, event_ref.forget()); + new WorkerRunnableDispatcher(this, mWorkerRef->Private(), event_ref.forget()); if (!event->Dispatch()) { return NS_ERROR_FAILURE; @@ -1982,23 +1978,53 @@ EventSource::Constructor(const GlobalObject& aGlobal, const nsAString& aURL, return nullptr; } eventSourceImp->Init(principal, aURL, aRv); - } else { - // In workers we have to keep the worker alive using a WorkerHolder in order - // to dispatch messages correctly. - if (!eventSourceImp->RegisterWorkerHolder()) { - aRv.Throw(NS_ERROR_FAILURE); - return nullptr; - } - RefPtr runnable = new InitRunnable(eventSourceImp, aURL); - runnable->Dispatch(Terminating, aRv); if (NS_WARN_IF(aRv.Failed())) { return nullptr; } - aRv = runnable->ErrorCode(); + + eventSourceImp->InitChannelAndRequestEventSource(); + return eventSource.forget(); } + + // Worker side. + WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate(); + MOZ_ASSERT(workerPrivate); + + RefPtr initRunnable = + new InitRunnable(workerPrivate, eventSourceImp, aURL); + initRunnable->Dispatch(Terminating, aRv); if (NS_WARN_IF(aRv.Failed())) { return nullptr; } + + aRv = initRunnable->ErrorCode(); + if (NS_WARN_IF(aRv.Failed())) { + return nullptr; + } + + // In workers we have to keep the worker alive using a WorkerRef in order + // to dispatch messages correctly. + if (!eventSourceImp->CreateWorkerRef(workerPrivate)) { + // The worker is already shutting down. Let's return an already closed + // object, but marked as Connecting. + eventSource->Close(); + + // EventSourceImpl must be released before returning the object, otherwise + // it will set EventSource to a CLOSED state in its DTOR. + eventSourceImp = nullptr; + + eventSource->mReadyState = EventSourceImpl::CONNECTING; + return eventSource.forget(); + } + + // Let's connect to the server. + RefPtr connectRunnable = + new ConnectRunnable(workerPrivate, eventSourceImp); + connectRunnable->Dispatch(Terminating, aRv); + if (NS_WARN_IF(aRv.Failed())) { + return nullptr; + } + return eventSource.forget(); } diff --git a/testing/web-platform/meta/MANIFEST.json b/testing/web-platform/meta/MANIFEST.json index a01a51370d20..9e7ead007e53 100644 --- a/testing/web-platform/meta/MANIFEST.json +++ b/testing/web-platform/meta/MANIFEST.json @@ -319484,6 +319484,12 @@ {} ] ], + "eventsource/dedicated-worker/eventsource-close2.htm": [ + [ + "/eventsource/dedicated-worker/eventsource-close2.htm", + {} + ] + ], "eventsource/dedicated-worker/eventsource-constructor-non-same-origin.htm": [ [ "/eventsource/dedicated-worker/eventsource-constructor-non-same-origin.htm", @@ -390178,7 +390184,7 @@ "manual" ], "FileAPI/FileReader/workers.html": [ - "7e9f00c9af5bb982a1b549ebc9f5a3d0b5cf4387", + "d7894a0abb064411d4811d8cfb9c3ce65f99babd", "testharness" ], "FileAPI/FileReaderSync.worker.js": [ @@ -542317,6 +542323,10 @@ "700107771158b22fa280f30a5a52d1aac617ff6e", "testharness" ], + "eventsource/dedicated-worker/eventsource-close2.htm": [ + "a3d13b0261b05eba56effb9ca3f6c31e312e777a", + "testharness" + ], "eventsource/dedicated-worker/eventsource-constructor-non-same-origin.htm": [ "9614ac5ce1967bbbcae6a1cc8d64465579f6410d", "testharness" @@ -546826,7 +546836,7 @@ "testharness" ], "html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/open-features-non-integer-height.html": [ - "caaede75e5c16cc78023ce410f48e37e612cffbb", + "6da68164fdba8986d4dd217ad48198f675e83165", "testharness" ], "html/browsers/the-window-object/apis-for-creating-and-navigating-browsing-contexts-by-name/open-features-non-integer-innerheight.html": [ @@ -594606,7 +594616,7 @@ "testharness" ], "websockets/Create-on-worker-shutdown.html": [ - "6c1e57e92b617fdb3d8dd86528af7073d21ece03", + "e710493c0cd84630a1c853ada23c37908bece9cb", "testharness" ], "websockets/Create-protocol-with-space.htm": [ diff --git a/testing/web-platform/tests/eventsource/dedicated-worker/eventsource-close2.htm b/testing/web-platform/tests/eventsource/dedicated-worker/eventsource-close2.htm new file mode 100644 index 000000000000..b1b74e88b82d --- /dev/null +++ b/testing/web-platform/tests/eventsource/dedicated-worker/eventsource-close2.htm @@ -0,0 +1,29 @@ + + + + + dedicated worker - EventSource created after: worker.close() + + + + +
+ + + +