Bug 1640091 - Make sure we TRRServiceChannel::mProxyRequest is initialized and used r=kershaw,necko-reviewers

Differential Revision: https://phabricator.services.mozilla.com/D78525

netwerk/ipc/ProxyConfigLookup.cpp

@@ -20,12 +20,12 @@ namespace net {
 // static
 nsresult ProxyConfigLookup::Create(
     std::function<void(nsIProxyInfo*, nsresult)>&& aCallback, nsIURI* aURI,
-    uint32_t aProxyResolveFlags) {
+    uint32_t aProxyResolveFlags, nsICancelable** aLookupCancellable) {
   MOZ_ASSERT(NS_IsMainThread());
 
   RefPtr<ProxyConfigLookup> lookUp =
       new ProxyConfigLookup(std::move(aCallback), aURI, aProxyResolveFlags);
-  return lookUp->DoProxyResolve();
+  return lookUp->DoProxyResolve(aLookupCancellable);
 }
 
 ProxyConfigLookup::ProxyConfigLookup(
@@ -37,7 +37,7 @@ ProxyConfigLookup::ProxyConfigLookup(
 
 ProxyConfigLookup::~ProxyConfigLookup() = default;
 
-nsresult ProxyConfigLookup::DoProxyResolve() {
+nsresult ProxyConfigLookup::DoProxyResolve(nsICancelable** aLookupCancellable) {
   if (!XRE_IsParentProcess()) {
     RefPtr<ProxyConfigLookup> self = this;
     bool result = ProxyConfigLookupChild::Create(
@@ -77,6 +77,10 @@ nsresult ProxyConfigLookup::DoProxyResolve() {
                            getter_AddRefs(proxyRequest));
   }
 
+  if (aLookupCancellable) {
+    proxyRequest.forget(aLookupCancellable);
+  }
+
   return rv;
 }
 

netwerk/ipc/ProxyConfigLookup.h

@@ -22,14 +22,15 @@ class ProxyConfigLookup final : public nsIProtocolProxyCallback {
 
   static nsresult Create(
       std::function<void(nsIProxyInfo*, nsresult)>&& aCallback, nsIURI* aURI,
-      uint32_t aProxyResolveFlags);
+      uint32_t aProxyResolveFlags,
+      nsICancelable** aLookupCancellable = nullptr);
 
  private:
   explicit ProxyConfigLookup(
       std::function<void(nsIProxyInfo*, nsresult)>&& aCallback, nsIURI* aURI,
       uint32_t aProxyResolveFlags);
   virtual ~ProxyConfigLookup();
-  nsresult DoProxyResolve();
+  nsresult DoProxyResolve(nsICancelable** aLookupCancellable);
 
   std::function<void(nsIProxyInfo*, nsresult)> mCallback;
   nsCOMPtr<nsIURI> mURI;

netwerk/protocol/http/TRRServiceChannel.cpp

@@ -88,6 +88,7 @@ TRRServiceChannel::TRRServiceChannel()
     : HttpAsyncAborter<TRRServiceChannel>(this),
       mTopWindowOriginComputed(false),
       mPushedStreamId(0),
+      mProxyRequest(nullptr, "TRRServiceChannel::mProxyRequest"),
       mCurrentEventTarget(GetCurrentThreadEventTarget()) {
   LOG(("TRRServiceChannel ctor [this=%p]\n", this));
 }
@@ -107,15 +108,21 @@ TRRServiceChannel::Cancel(nsresult status) {
 
   mCanceled = true;
   mStatus = status;
-  if (mProxyRequest) {
+
-    nsCOMPtr<nsICancelable> proxyRequet;
+  nsCOMPtr<nsICancelable> proxyRequest;
-    proxyRequet.swap(mProxyRequest);
+  {
+    auto req = mProxyRequest.Lock();
+    proxyRequest.swap(*req);
+  }
+
+  if (proxyRequest) {
     NS_DispatchToMainThread(
         NS_NewRunnableFunction(
             "CancelProxyRequest",
-            [proxyRequet, status]() { proxyRequet->Cancel(status); }),
+            [proxyRequest, status]() { proxyRequest->Cancel(status); }),
         NS_DISPATCH_NORMAL);
   }
+
   CancelNetworkRequest(status);
   return NS_OK;
 }
@@ -248,21 +255,37 @@ nsresult TRRServiceChannel::ResolveProxy() {
 
   // TODO: bug 1625171. Consider moving proxy resolution to socket process.
   RefPtr<TRRServiceChannel> self = this;
+  nsCOMPtr<nsICancelable> proxyRequest;
   nsresult rv = ProxyConfigLookup::Create(
       [self](nsIProxyInfo* aProxyInfo, nsresult aStatus) {
         self->OnProxyAvailable(nullptr, nullptr, aProxyInfo, aStatus);
       },
-      mURI, mProxyResolveFlags);
+      mURI, mProxyResolveFlags, getter_AddRefs(proxyRequest));
 
   if (NS_FAILED(rv)) {
     if (!mCurrentEventTarget->IsOnCurrentThread()) {
-      mCurrentEventTarget->Dispatch(
+      return mCurrentEventTarget->Dispatch(
           NewRunnableMethod<nsresult>("TRRServiceChannel::AsyncAbort", this,
                                       &TRRServiceChannel::AsyncAbort, rv),
           NS_DISPATCH_NORMAL);
     }
   }
 
+  {
+    auto req = mProxyRequest.Lock();
+    // We only set mProxyRequest if the channel hasn't already been cancelled
+    // on another thread.
+    if (!mCanceled) {
+      *req = proxyRequest.forget();
+    }
+  }
+
+  // If the channel has been cancelled, we go ahead and cancel the proxy
+  // request right here.
+  if (proxyRequest) {
+    proxyRequest->Cancel(mStatus);
+  }
+
   return rv;
 }
 
@@ -288,7 +311,10 @@ TRRServiceChannel::OnProxyAvailable(nsICancelable* request, nsIChannel* channel,
 
   MOZ_ASSERT(mCurrentEventTarget->IsOnCurrentThread());
 
-  mProxyRequest = nullptr;
+  {
+    auto proxyRequest = mProxyRequest.Lock();
+    *proxyRequest = nullptr;
+  }
 
   nsresult rv;
 

netwerk/protocol/http/TRRServiceChannel.h

@@ -151,7 +151,7 @@ class TRRServiceChannel : public HttpBaseChannel,
   RefPtr<HttpTransactionShell> mTransaction;
   uint32_t mPushedStreamId;
   RefPtr<HttpTransactionShell> mTransWithPushedStream;
-  nsCOMPtr<nsICancelable> mProxyRequest;
+  DataMutex<nsCOMPtr<nsICancelable>> mProxyRequest;
   nsCOMPtr<nsIEventTarget> mCurrentEventTarget;
 
   friend class HttpAsyncAborter<TRRServiceChannel>;