mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-11-27 23:02:20 +00:00
Bug 1858033 - Refactor Proxy::mXMLHttpRequestPrivate to become a WeakPtr and use more RefPtr. r=dom-worker-reviewers,smaug,asuth
This patch also clarifies that mXMLHttpRequestPrivate is only ever dereferenced on the worker thread and avoids to copy its raw pointer on the main thread by relying always on Proxy to access it. Differential Revision: https://phabricator.services.mozilla.com/D194077
This commit is contained in:
parent
7ac9b7561d
commit
47e680fe25
@ -94,10 +94,13 @@ class Proxy final : public nsIDOMEventListener {
|
||||
public:
|
||||
// Read on multiple threads.
|
||||
WorkerPrivate* mWorkerPrivate;
|
||||
XMLHttpRequestWorker* mXMLHttpRequestPrivate;
|
||||
const ClientInfo mClientInfo;
|
||||
const Maybe<ServiceWorkerDescriptor> mController;
|
||||
|
||||
// Only ever dereferenced and/or checked on the worker thread. Cleared
|
||||
// explicitly on the worker thread inside XMLHttpRequestWorker::ReleaseProxy.
|
||||
WeakPtr<XMLHttpRequestWorker> mXMLHttpRequestPrivate;
|
||||
|
||||
// XHR Params:
|
||||
bool mMozAnon;
|
||||
bool mMozSystem;
|
||||
@ -138,9 +141,9 @@ class Proxy final : public nsIDOMEventListener {
|
||||
const Maybe<ServiceWorkerDescriptor>& aController, bool aMozAnon,
|
||||
bool aMozSystem)
|
||||
: mWorkerPrivate(nullptr),
|
||||
mXMLHttpRequestPrivate(aXHRPrivate),
|
||||
mClientInfo(aClientInfo),
|
||||
mController(aController),
|
||||
mXMLHttpRequestPrivate(aXHRPrivate),
|
||||
mMozAnon(aMozAnon),
|
||||
mMozSystem(aMozSystem),
|
||||
mInnerEventStreamId(0),
|
||||
@ -315,22 +318,21 @@ class MainThreadProxyRunnable : public MainThreadWorkerSyncRunnable {
|
||||
};
|
||||
|
||||
class XHRUnpinRunnable final : public MainThreadWorkerControlRunnable {
|
||||
XMLHttpRequestWorker* mXMLHttpRequestPrivate;
|
||||
RefPtr<Proxy> mXHRProxy;
|
||||
|
||||
public:
|
||||
XHRUnpinRunnable(WorkerPrivate* aWorkerPrivate,
|
||||
XMLHttpRequestWorker* aXHRPrivate)
|
||||
: MainThreadWorkerControlRunnable(aWorkerPrivate),
|
||||
mXMLHttpRequestPrivate(aXHRPrivate) {
|
||||
MOZ_ASSERT(aXHRPrivate);
|
||||
XHRUnpinRunnable(WorkerPrivate* aWorkerPrivate, Proxy* aXHRProxy)
|
||||
: MainThreadWorkerControlRunnable(aWorkerPrivate), mXHRProxy(aXHRProxy) {
|
||||
MOZ_ASSERT(aXHRProxy);
|
||||
}
|
||||
|
||||
private:
|
||||
~XHRUnpinRunnable() = default;
|
||||
|
||||
bool WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override {
|
||||
if (mXMLHttpRequestPrivate->SendInProgress()) {
|
||||
mXMLHttpRequestPrivate->Unpin();
|
||||
XMLHttpRequestWorker* xhrw = mXHRProxy->mXMLHttpRequestPrivate.get();
|
||||
if (xhrw && xhrw->SendInProgress()) {
|
||||
xhrw->Unpin();
|
||||
}
|
||||
|
||||
return true;
|
||||
@ -367,21 +369,17 @@ class LoadStartDetectionRunnable final : public Runnable,
|
||||
WorkerPrivate* mWorkerPrivate;
|
||||
RefPtr<Proxy> mProxy;
|
||||
RefPtr<XMLHttpRequest> mXHR;
|
||||
XMLHttpRequestWorker* mXMLHttpRequestPrivate;
|
||||
nsString mEventType;
|
||||
uint32_t mChannelId;
|
||||
bool mReceivedLoadStart;
|
||||
|
||||
class ProxyCompleteRunnable final : public MainThreadProxyRunnable {
|
||||
XMLHttpRequestWorker* mXMLHttpRequestPrivate;
|
||||
uint32_t mChannelId;
|
||||
|
||||
public:
|
||||
ProxyCompleteRunnable(WorkerPrivate* aWorkerPrivate, Proxy* aProxy,
|
||||
XMLHttpRequestWorker* aXHRPrivate,
|
||||
uint32_t aChannelId)
|
||||
: MainThreadProxyRunnable(aWorkerPrivate, aProxy),
|
||||
mXMLHttpRequestPrivate(aXHRPrivate),
|
||||
mChannelId(aChannelId) {}
|
||||
|
||||
private:
|
||||
@ -398,8 +396,9 @@ class LoadStartDetectionRunnable final : public Runnable,
|
||||
aWorkerPrivate->StopSyncLoop(mSyncLoopTarget, NS_OK);
|
||||
}
|
||||
|
||||
if (mXMLHttpRequestPrivate->SendInProgress()) {
|
||||
mXMLHttpRequestPrivate->Unpin();
|
||||
XMLHttpRequestWorker* xhrw = mProxy->mXMLHttpRequestPrivate.get();
|
||||
if (xhrw && xhrw->SendInProgress()) {
|
||||
xhrw->Unpin();
|
||||
}
|
||||
|
||||
return true;
|
||||
@ -409,12 +408,11 @@ class LoadStartDetectionRunnable final : public Runnable,
|
||||
};
|
||||
|
||||
public:
|
||||
LoadStartDetectionRunnable(Proxy* aProxy, XMLHttpRequestWorker* aXHRPrivate)
|
||||
explicit LoadStartDetectionRunnable(Proxy* aProxy)
|
||||
: Runnable("dom::LoadStartDetectionRunnable"),
|
||||
mWorkerPrivate(aProxy->mWorkerPrivate),
|
||||
mProxy(aProxy),
|
||||
mXHR(aProxy->mXHR),
|
||||
mXMLHttpRequestPrivate(aXHRPrivate),
|
||||
mChannelId(mProxy->mInnerChannelId),
|
||||
mReceivedLoadStart(false) {
|
||||
AssertIsOnMainThread();
|
||||
@ -818,7 +816,7 @@ void Proxy::Teardown(bool aSendUnpin) {
|
||||
if (mOutstandingSendCount) {
|
||||
if (aSendUnpin) {
|
||||
RefPtr<XHRUnpinRunnable> runnable =
|
||||
new XHRUnpinRunnable(mWorkerPrivate, mXMLHttpRequestPrivate);
|
||||
new XHRUnpinRunnable(mWorkerPrivate, this);
|
||||
MOZ_ALWAYS_TRUE(runnable->Dispatch());
|
||||
}
|
||||
|
||||
@ -885,7 +883,10 @@ NS_IMETHODIMP
|
||||
Proxy::HandleEvent(Event* aEvent) {
|
||||
AssertIsOnMainThread();
|
||||
|
||||
if (!mWorkerPrivate || !mXMLHttpRequestPrivate) {
|
||||
// EventRunnable::WorkerRun will bail out if mXMLHttpRequestWorker is null,
|
||||
// so we do not need to prevent the dispatch from the main thread such that
|
||||
// we do not need to touch it off-worker-thread.
|
||||
if (!mWorkerPrivate) {
|
||||
NS_ERROR("Shouldn't get here!");
|
||||
return NS_OK;
|
||||
}
|
||||
@ -941,7 +942,7 @@ Proxy::HandleEvent(Event* aEvent) {
|
||||
mMainThreadSeenLoadStart = false;
|
||||
|
||||
RefPtr<LoadStartDetectionRunnable> runnable =
|
||||
new LoadStartDetectionRunnable(this, mXMLHttpRequestPrivate);
|
||||
new LoadStartDetectionRunnable(this);
|
||||
if (!runnable->RegisterAndDispatch()) {
|
||||
NS_WARNING("Failed to dispatch LoadStartDetectionRunnable!");
|
||||
}
|
||||
@ -966,8 +967,8 @@ LoadStartDetectionRunnable::Run() {
|
||||
} else if (mProxy->mOutstandingSendCount == 1) {
|
||||
mProxy->Reset();
|
||||
|
||||
RefPtr<ProxyCompleteRunnable> runnable = new ProxyCompleteRunnable(
|
||||
mWorkerPrivate, mProxy, mXMLHttpRequestPrivate, mChannelId);
|
||||
RefPtr<ProxyCompleteRunnable> runnable =
|
||||
new ProxyCompleteRunnable(mWorkerPrivate, mProxy, mChannelId);
|
||||
if (runnable->Dispatch()) {
|
||||
mProxy->mWorkerPrivate = nullptr;
|
||||
mProxy->mSyncLoopTarget = nullptr;
|
||||
@ -978,7 +979,6 @@ LoadStartDetectionRunnable::Run() {
|
||||
|
||||
mProxy = nullptr;
|
||||
mXHR = nullptr;
|
||||
mXMLHttpRequestPrivate = nullptr;
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
@ -1449,6 +1449,10 @@ void XMLHttpRequestWorker::ReleaseProxy(ReleaseType aType) {
|
||||
// may be gone.
|
||||
|
||||
if (mProxy) {
|
||||
// We need to clear our weak pointer on the worker thread, let's do it now
|
||||
// before doing it implicitly in the Proxy dtor on the wrong thread.
|
||||
mProxy->mXMLHttpRequestPrivate = nullptr;
|
||||
|
||||
if (aType == XHRIsGoingAway) {
|
||||
// We're in a GC finalizer, so we can't do a sync call here (and we don't
|
||||
// need to).
|
||||
@ -1500,7 +1504,7 @@ void XMLHttpRequestWorker::MaybePin(ErrorResult& aRv) {
|
||||
return;
|
||||
}
|
||||
|
||||
NS_ADDREF_THIS();
|
||||
mPinnedSelfRef = this;
|
||||
}
|
||||
|
||||
void XMLHttpRequestWorker::MaybeDispatchPrematureAbortEvents(ErrorResult& aRv) {
|
||||
@ -1626,7 +1630,7 @@ void XMLHttpRequestWorker::Unpin() {
|
||||
MOZ_ASSERT(mWorkerRef, "Mismatched calls to Unpin!");
|
||||
mWorkerRef = nullptr;
|
||||
|
||||
NS_RELEASE_THIS();
|
||||
mPinnedSelfRef = nullptr;
|
||||
}
|
||||
|
||||
void XMLHttpRequestWorker::SendInternal(const BodyExtractorBase* aBody,
|
||||
@ -1673,6 +1677,7 @@ void XMLHttpRequestWorker::SendInternal(const BodyExtractorBase* aBody,
|
||||
return;
|
||||
}
|
||||
|
||||
RefPtr<XMLHttpRequestWorker> selfRef = this;
|
||||
AutoUnpinXHR autoUnpin(this);
|
||||
Maybe<AutoSyncLoopHolder> autoSyncLoop;
|
||||
|
||||
@ -1953,7 +1958,7 @@ void XMLHttpRequestWorker::Send(
|
||||
return;
|
||||
}
|
||||
|
||||
if (!mProxy || mStateData->mFlagSend) {
|
||||
if (!mProxy || !mProxy->mXMLHttpRequestPrivate || mStateData->mFlagSend) {
|
||||
aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR);
|
||||
return;
|
||||
}
|
||||
|
@ -9,6 +9,7 @@
|
||||
|
||||
#include "XMLHttpRequest.h"
|
||||
#include "XMLHttpRequestString.h"
|
||||
#include "mozilla/WeakPtr.h"
|
||||
#include "mozilla/dom/BodyExtractor.h"
|
||||
#include "mozilla/dom/TypedArray.h"
|
||||
|
||||
@ -23,7 +24,8 @@ class SendRunnable;
|
||||
class StrongWorkerRef;
|
||||
class WorkerPrivate;
|
||||
|
||||
class XMLHttpRequestWorker final : public XMLHttpRequest {
|
||||
class XMLHttpRequestWorker final : public SupportsWeakPtr,
|
||||
public XMLHttpRequest {
|
||||
public:
|
||||
// This defines the xhr.response value.
|
||||
struct ResponseData {
|
||||
@ -60,6 +62,7 @@ class XMLHttpRequestWorker final : public XMLHttpRequest {
|
||||
RefPtr<XMLHttpRequestUpload> mUpload;
|
||||
WorkerPrivate* mWorkerPrivate;
|
||||
RefPtr<StrongWorkerRef> mWorkerRef;
|
||||
RefPtr<XMLHttpRequestWorker> mPinnedSelfRef;
|
||||
RefPtr<Proxy> mProxy;
|
||||
|
||||
XMLHttpRequestResponseType mResponseType;
|
||||
|
Loading…
Reference in New Issue
Block a user