#559: M1547266

netwerk/protocol/http/Http2Push.cpp

@@ -30,8 +30,8 @@ class CallChannelOnPush final : public nsRunnable {
                     Http2PushedStream *pushStream)
     : mAssociatedChannel(associatedChannel)
     , mPushedURI(pushedURI)
-    , mPushedStream(pushStream)
   {
+    mPushedStreamWrapper = new Http2PushedStreamWrapper(pushStream);
   }
 
   NS_IMETHOD Run()
@@ -40,21 +40,97 @@ class CallChannelOnPush final : public nsRunnable {
     RefPtr<nsHttpChannel> channel;
     CallQueryInterface(mAssociatedChannel, channel.StartAssignment());
     MOZ_ASSERT(channel);
-    if (channel && NS_SUCCEEDED(channel->OnPush(mPushedURI, mPushedStream))) {
+    if (channel &&
+        NS_SUCCEEDED(channel->OnPush(mPushedURI, mPushedStreamWrapper))) {
       return NS_OK;
     }
 
     LOG3(("Http2PushedStream Orphan %p failed OnPush\n", this));
-    mPushedStream->OnPushFailed();
+    mPushedStreamWrapper->OnPushFailed();
     return NS_OK;
   }
 
 private:
   nsCOMPtr<nsIHttpChannelInternal> mAssociatedChannel;
   const nsCString mPushedURI;
-  Http2PushedStream *mPushedStream;
+  RefPtr<Http2PushedStreamWrapper> mPushedStreamWrapper;
 };
 
+// Because WeakPtr isn't thread-safe we must ensure that the object is destroyed
+// on the socket thread, so any Release() called on a different thread is
+// dispatched to the socket thread.
+bool Http2PushedStreamWrapper::DispatchRelease() {
+  if (PR_GetCurrentThread() == gSocketThread) { // OnSocketThread()
+    return false;
+  }
+
+  gSocketTransportService->Dispatch(
+      NS_NewNonOwningRunnableMethod(this,
+                                    &Http2PushedStreamWrapper::Release),
+      NS_DISPATCH_NORMAL);
+
+  return true;
+}
+
+NS_IMPL_ADDREF(Http2PushedStreamWrapper)
+NS_IMETHODIMP_(MozExternalRefCountType)
+Http2PushedStreamWrapper::Release() {
+  nsrefcnt count = mRefCnt - 1;
+  if (DispatchRelease()) {
+    // Redispatched to the socket thread.
+    return count;
+  }
+
+  MOZ_ASSERT(0 != mRefCnt, "dup release");
+  count = --mRefCnt;
+  NS_LOG_RELEASE(this, count, "Http2PushedStreamWrapper");
+
+  if (0 == count) {
+    mRefCnt = 1;
+    delete (this);
+    return 0;
+  }
+
+  return count;
+}
+
+NS_INTERFACE_MAP_BEGIN(Http2PushedStreamWrapper)
+NS_INTERFACE_MAP_END
+
+Http2PushedStreamWrapper::Http2PushedStreamWrapper(
+    Http2PushedStream* aPushStream) {
+  MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not on socket thread");
+  mStream = aPushStream;
+  mRequestString = aPushStream->GetRequestString();
+}
+
+Http2PushedStreamWrapper::~Http2PushedStreamWrapper() {
+  MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not on socket thread");
+}
+
+Http2PushedStream* Http2PushedStreamWrapper::GetStream() {
+  MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread, "not on socket thread");
+  if (mStream) {
+    Http2Stream* stream = mStream;
+    return static_cast<Http2PushedStream*>(stream);
+  }
+  return nullptr;
+}
+
+void Http2PushedStreamWrapper::OnPushFailed() {
+  if (PR_GetCurrentThread() == gSocketThread) { // OnSocketThread()
+    if (mStream) {
+      Http2Stream* stream = mStream;
+      static_cast<Http2PushedStream*>(stream)->OnPushFailed();
+    }
+  } else {
+    gSocketTransportService->Dispatch(
+        NS_NewRunnableMethod(this,
+                             &Http2PushedStreamWrapper::OnPushFailed),
+        NS_DISPATCH_NORMAL);
+  }
+}
+
 //////////////////////////////////////////
 // Http2PushedStream
 //////////////////////////////////////////

netwerk/protocol/http/Http2Push.h

@@ -121,6 +121,24 @@ private:
   uint32_t mBufferedHTTP1Consumed;
 };
 
+class Http2PushedStreamWrapper : public nsISupports {
+ public:
+  NS_DECL_THREADSAFE_ISUPPORTS
+  bool DispatchRelease();
+
+  explicit Http2PushedStreamWrapper(Http2PushedStream* aPushStream);
+
+  nsCString& GetRequestString() { return mRequestString; }
+  Http2PushedStream* GetStream();
+  void OnPushFailed();
+
+ private:
+  virtual ~Http2PushedStreamWrapper();
+
+  nsCString mRequestString;
+  WeakPtr<Http2Stream> mStream;
+};
+
 } // namespace net
 } // namespace mozilla
 

netwerk/protocol/http/Http2Stream.cpp

@@ -460,12 +460,14 @@ Http2Stream::ParseHttpRequestHeaders(const char *buf,
       schedulingContext->GetSpdyPushCache(&cache);
     }
 
+    RefPtr<Http2PushedStreamWrapper> pushedStreamWrapper;
     Http2PushedStream *pushedStream = nullptr;
 
     // If a push stream is attached to the transaction via onPush, match only with that
     // one. This occurs when a push was made with in conjunction with a nsIHttpPushListener
     nsHttpTransaction *trans = mTransaction->QueryHttpTransaction();
-    if (trans && (pushedStream = trans->TakePushedStream())) {
+    if (trans && (pushedStreamWrapper = trans->TakePushedStream()) &&
+        (pushedStream = pushedStreamWrapper->GetStream())) {
       if (pushedStream->mSession == mSession) {
         LOG3(("Pushed Stream match based on OnPush correlation %p", pushedStream));
       } else {

netwerk/protocol/http/nsHttpChannel.cpp

@@ -7070,7 +7070,7 @@ nsHttpChannel::AwaitingCacheCallbacks()
 }
 
 void
-nsHttpChannel::SetPushedStream(Http2PushedStream *stream)
+nsHttpChannel::SetPushedStream(Http2PushedStreamWrapper *stream)
 {
     MOZ_ASSERT(stream);
     MOZ_ASSERT(!mPushedStream);
@@ -7078,7 +7078,8 @@ nsHttpChannel::SetPushedStream(Http2PushedStream *stream)
 }
 
 nsresult
-nsHttpChannel::OnPush(const nsACString &url, Http2PushedStream *pushedStream)
+nsHttpChannel::OnPush(const nsACString &url,
+                      Http2PushedStreamWrapper *pushedStream)
 {
     MOZ_ASSERT(NS_IsMainThread());
     LOG(("nsHttpChannel::OnPush [this=%p]\n", this));

netwerk/protocol/http/nsHttpChannel.h

@@ -120,7 +120,8 @@ public:
                           uint32_t aProxyResolveFlags,
                           nsIURI *aProxyURI) override;
 
-    nsresult OnPush(const nsACString &uri, Http2PushedStream *pushedStream);
+    nsresult OnPush(const nsACString &uri,
+                    Http2PushedStreamWrapper *pushedStream);
 
     static bool IsRedirectStatus(uint32_t status);
 
@@ -411,7 +412,7 @@ private:
     nsresult OpenCacheInputStream(nsICacheEntry* cacheEntry, bool startBuffering,
                                   bool checkingAppCacheEntry);
 
-    void SetPushedStream(Http2PushedStream *stream);
+    void SetPushedStream(Http2PushedStreamWrapper *stream);
 
     void MaybeWarnAboutAppCache();
 
@@ -522,7 +523,7 @@ private:
     // Needed for accurate DNS timing
     RefPtr<nsDNSPrefetch>           mDNSPrefetch;
 
-    Http2PushedStream                 *mPushedStream;
+    RefPtr<Http2PushedStreamWrapper> mPushedStream;
     // True if the channel's principal was found on a phishing, malware, or
     // tracking (if tracking protection is enabled) blocklist
     bool                              mLocalBlocklist;

netwerk/protocol/http/nsHttpConnectionMgr.cpp

@@ -2060,11 +2060,15 @@ nsHttpConnectionMgr::ProcessNewTransaction(nsHttpTransaction *trans)
 
     trans->SetPendingTime();
 
-    Http2PushedStream *pushedStream = trans->GetPushedStream();
-    if (pushedStream) {
-        return pushedStream->Session()->
-            AddStream(trans, trans->Priority(), false, nullptr) ?
-            NS_OK : NS_ERROR_UNEXPECTED;
+    RefPtr<Http2PushedStreamWrapper> pushedStreamWrapper =
+        trans->GetPushedStream();
+    if (pushedStreamWrapper) {
+        Http2PushedStream* pushedStream = pushedStreamWrapper->GetStream();
+        if (pushedStream) {
+            return pushedStream->Session()->
+                AddStream(trans, trans->Priority(), false, nullptr) ?
+                    NS_OK : NS_ERROR_UNEXPECTED;
+        }
     }
 
     nsresult rv = NS_OK;

netwerk/protocol/http/nsHttpTransaction.h

@@ -134,14 +134,16 @@ public:
 
     nsHttpTransaction *QueryHttpTransaction() override { return this; }
 
-    Http2PushedStream *GetPushedStream() { return mPushedStream; }
-    Http2PushedStream *TakePushedStream()
-    {
-        Http2PushedStream *r = mPushedStream;
-        mPushedStream = nullptr;
-        return r;
+    already_AddRefed<Http2PushedStreamWrapper> GetPushedStream() {
+      //return do_AddRef(mPushedStream); // XXX: add this support to RefPtr.h
+      RefPtr<Http2PushedStreamWrapper> ref(mPushedStream);
+      return ref.forget();
     }
-    void SetPushedStream(Http2PushedStream *push) { mPushedStream = push; }
+    already_AddRefed<Http2PushedStreamWrapper> TakePushedStream() {
+      return mPushedStream.forget();
+    }
+
+    void SetPushedStream(Http2PushedStreamWrapper* push) { mPushedStream = push; }
     uint32_t InitialRwin() const { return mInitialRwin; };
 
     // Locked methods to get and set timing info
@@ -253,7 +255,7 @@ private:
     // so far been skipped.
     uint32_t                        mInvalidResponseBytesRead;
 
-    Http2PushedStream               *mPushedStream;
+    RefPtr<Http2PushedStreamWrapper> mPushedStream;
     uint32_t                        mInitialRwin;
 
     nsHttpChunkedDecoder            *mChunkedDecoder;