Bug 905460 - Convert http legacy refs to smart pointers r=dragana a=kwierso
authorPatrick McManus <mcmanus@ducksong.com>
Tue, 29 Mar 2016 15:35:56 -0400
changeset 291099 2204c405d07c203a32bad351200e81b43ac062c4
parent 291098 afb08f0036485e6840155eb3104327345670cd63
child 291100 2f39deb1b3e2865ced9cead27a03e97d729fbcfb
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, kwierso
bugs905460
milestone48.0a1
Bug 905460 - Convert http legacy refs to smart pointers r=dragana a=kwierso MozReview-Commit-ID: G86Mrv0Cdgk
netwerk/protocol/http/Http2Session.cpp
netwerk/protocol/http/SpdySession31.cpp
netwerk/protocol/http/nsAHttpConnection.h
netwerk/protocol/http/nsHttpConnectionMgr.cpp
netwerk/protocol/http/nsHttpConnectionMgr.h
netwerk/protocol/http/nsHttpPipeline.cpp
netwerk/protocol/http/nsHttpPipeline.h
netwerk/protocol/http/nsHttpTransaction.cpp
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -3620,17 +3620,17 @@ Http2Session::IsPersistent()
 nsresult
 Http2Session::TakeTransport(nsISocketTransport **,
                             nsIAsyncInputStream **, nsIAsyncOutputStream **)
 {
   MOZ_ASSERT(false, "TakeTransport of Http2Session");
   return NS_ERROR_UNEXPECTED;
 }
 
-nsHttpConnection *
+already_AddRefed<nsHttpConnection>
 Http2Session::TakeHttpConnection()
 {
   MOZ_ASSERT(false, "TakeHttpConnection of Http2Session");
   return nullptr;
 }
 
 uint32_t
 Http2Session::CancelPipeline(nsresult reason)
--- a/netwerk/protocol/http/SpdySession31.cpp
+++ b/netwerk/protocol/http/SpdySession31.cpp
@@ -2829,17 +2829,17 @@ nsresult
 SpdySession31::TakeTransport(nsISocketTransport **,
                              nsIAsyncInputStream **,
                              nsIAsyncOutputStream **)
 {
   MOZ_ASSERT(false, "TakeTransport of SpdySession31");
   return NS_ERROR_UNEXPECTED;
 }
 
-nsHttpConnection *
+already_AddRefed<nsHttpConnection>
 SpdySession31::TakeHttpConnection()
 {
   MOZ_ASSERT(false, "TakeHttpConnection of SpdySession31");
   return nullptr;
 }
 
 uint32_t
 SpdySession31::CancelPipeline(nsresult reason)
--- a/netwerk/protocol/http/nsAHttpConnection.h
+++ b/netwerk/protocol/http/nsAHttpConnection.h
@@ -122,17 +122,17 @@ public:
 
     // Used by a transaction to manage the state of previous response bodies on
     // the same connection and work around buggy servers.
     virtual bool LastTransactionExpectedNoContent() = 0;
     virtual void   SetLastTransactionExpectedNoContent(bool) = 0;
 
     // Transfer the base http connection object along with a
     // reference to it to the caller.
-    virtual nsHttpConnection *TakeHttpConnection() = 0;
+    virtual already_AddRefed<nsHttpConnection> TakeHttpConnection() = 0;
 
     // Get the nsISocketTransport used by the connection without changing
     //  references or ownership.
     virtual nsISocketTransport *Transport() = 0;
 
     // Cancel and reschedule transactions deeper than the current response.
     // Returns the number of canceled transactions.
     virtual uint32_t CancelPipeline(nsresult originalReason) = 0;
@@ -160,17 +160,17 @@ NS_DEFINE_STATIC_IID_ACCESSOR(nsAHttpCon
     void CloseTransaction(nsAHttpTransaction *, nsresult) override; \
     nsresult TakeTransport(nsISocketTransport **,    \
                            nsIAsyncInputStream **,   \
                            nsIAsyncOutputStream **) override; \
     bool IsPersistent() override;                         \
     bool IsReused() override;                             \
     void DontReuse() override;                            \
     nsresult PushBack(const char *, uint32_t) override;   \
-    nsHttpConnection *TakeHttpConnection() override;      \
+    already_AddRefed<nsHttpConnection> TakeHttpConnection() override; \
     uint32_t CancelPipeline(nsresult originalReason) override; \
     nsAHttpTransaction::Classifier Classification() override; \
     /*                                                    \
        Thes methods below have automatic definitions that just forward the \
        function to a lower level connection object        \
     */                                                    \
     void GetConnectionInfo(nsHttpConnectionInfo **result) \
       override                                            \
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -41,26 +41,26 @@
 namespace mozilla {
 namespace net {
 
 //-----------------------------------------------------------------------------
 
 NS_IMPL_ISUPPORTS(nsHttpConnectionMgr, nsIObserver)
 
 static void
-InsertTransactionSorted(nsTArray<nsHttpTransaction*> &pendingQ, nsHttpTransaction *trans)
+InsertTransactionSorted(nsTArray<RefPtr<nsHttpTransaction> > &pendingQ, nsHttpTransaction *trans)
 {
     // insert into queue with smallest valued number first.  search in reverse
     // order under the assumption that many of the existing transactions will
     // have the same priority (usually 0).
 
-    for (int32_t i=pendingQ.Length()-1; i>=0; --i) {
+    for (int32_t i = pendingQ.Length() - 1; i >= 0; --i) {
         nsHttpTransaction *t = pendingQ[i];
         if (trans->Priority() >= t->Priority()) {
-	  if (ChaosMode::isActive(ChaosFeature::NetworkScheduling)) {
+            if (ChaosMode::isActive(ChaosFeature::NetworkScheduling)) {
                 int32_t samePriorityCount;
                 for (samePriorityCount = 0; i - samePriorityCount >= 0; ++samePriorityCount) {
                     if (pendingQ[i - samePriorityCount]->Priority() != trans->Priority()) {
                         break;
                     }
                 }
                 // skip over 0...all of the elements with the same priority.
                 i -= ChaosMode::randomUint32LessThan(samePriorityCount + 1);
@@ -454,17 +454,18 @@ nsHttpConnectionMgr::SpeculativeConnect(
 }
 
 nsresult
 nsHttpConnectionMgr::GetSocketThreadTarget(nsIEventTarget **target)
 {
     EnsureSocketThreadTarget();
 
     ReentrantMonitorAutoEnter mon(mReentrantMonitor);
-    NS_IF_ADDREF(*target = mSocketThreadTarget);
+    nsCOMPtr<nsIEventTarget> temp(mSocketThreadTarget);
+    temp.forget(target);
     return NS_OK;
 }
 
 nsresult
 nsHttpConnectionMgr::ReclaimConnection(nsHttpConnection *conn)
 {
     LOG(("nsHttpConnectionMgr::ReclaimConnection [conn=%p]\n", conn));
     return PostEvent(&nsHttpConnectionMgr::OnMsgReclaimConnection, 0, conn);
@@ -645,21 +646,21 @@ nsHttpConnectionMgr::CloseIdleConnection
          this, conn));
 
     if (!conn->ConnectionInfo())
         return NS_ERROR_UNEXPECTED;
 
     nsConnectionEntry *ent = LookupConnectionEntry(conn->ConnectionInfo(),
                                                    conn, nullptr);
 
+    RefPtr<nsHttpConnection> deleteProtector(conn);
     if (!ent || !ent->mIdleConns.RemoveElement(conn))
         return NS_ERROR_UNEXPECTED;
 
     conn->Close(NS_ERROR_ABORT);
-    NS_RELEASE(conn);
     mNumIdleConns--;
     ConditionallyStopPruneDeadConnectionsTimer();
     return NS_OK;
 }
 
 // This function lets a connection, after completing the NPN phase,
 // report whether or not it is using spdy through the usingSpdy
 // argument. It would not be necessary if NPN were driven out of
@@ -875,18 +876,18 @@ nsHttpConnectionMgr::ProcessPendingQForE
         if (NS_SUCCEEDED(rv) || (rv != NS_ERROR_NOT_AVAILABLE)) {
             if (NS_SUCCEEDED(rv))
                 LOG(("  dispatching pending transaction...\n"));
             else
                 LOG(("  removing pending transaction based on "
                      "TryDispatchTransaction returning hard error %x\n", rv));
 
             if (ent->mPendingQ.RemoveElement(trans)) {
+                // trans is now potentially destroyed
                 dispatchedSuccessfully = true;
-                NS_RELEASE(trans);
                 continue; // dont ++i as we just made the array shorter
             }
 
             LOG(("  transaction not found in pending queue\n"));
         }
 
         if (dispatchedSuccessfully && !considerAll)
             break;
@@ -1067,21 +1068,20 @@ nsHttpConnectionMgr::AtActiveConnectionL
 }
 
 void
 nsHttpConnectionMgr::ClosePersistentConnections(nsConnectionEntry *ent)
 {
     LOG(("nsHttpConnectionMgr::ClosePersistentConnections [ci=%s]\n",
          ent->mConnInfo->HashKey().get()));
     while (ent->mIdleConns.Length()) {
-        nsHttpConnection *conn = ent->mIdleConns[0];
+        RefPtr<nsHttpConnection> conn(ent->mIdleConns[0]);
         ent->mIdleConns.RemoveElementAt(0);
         mNumIdleConns--;
         conn->Close(NS_ERROR_ABORT);
-        NS_RELEASE(conn);
     }
 
     int32_t activeCount = ent->mActiveConns.Length();
     for (int32_t i=0; i < activeCount; i++)
         ent->mActiveConns[i]->DontReuse();
 }
 
 bool
@@ -1217,20 +1217,19 @@ nsHttpConnectionMgr::MakeNewConnection(n
         auto iter = mCT.Iter();
         while (mNumIdleConns + mNumActiveConns + 1 >= mMaxConns &&
                !iter.Done()) {
             nsAutoPtr<nsConnectionEntry> &ent = iter.Data();
             if (!ent->mIdleConns.Length()) {
               iter.Next();
               continue;
             }
-            nsHttpConnection *conn = ent->mIdleConns[0];
+            RefPtr<nsHttpConnection> conn(ent->mIdleConns[0]);
             ent->mIdleConns.RemoveElementAt(0);
             conn->Close(NS_ERROR_ABORT);
-            NS_RELEASE(conn);
             mNumIdleConns--;
             ConditionallyStopPruneDeadConnectionsTimer();
         }
     }
 
     if ((mNumIdleConns + mNumActiveConns + 1 >= mMaxConns) &&
         mNumActiveConns && gHttpHandler->IsSpdyEnabled())
     {
@@ -1538,18 +1537,16 @@ nsHttpConnectionMgr::TryDispatchTransact
     // step 2
     // consider an idle persistent connection
     if (caps & NS_HTTP_ALLOW_KEEPALIVE) {
         RefPtr<nsHttpConnection> conn;
         while (!conn && (ent->mIdleConns.Length() > 0)) {
             conn = ent->mIdleConns[0];
             ent->mIdleConns.RemoveElementAt(0);
             mNumIdleConns--;
-            nsHttpConnection *temp = conn;
-            NS_RELEASE(temp);
 
             // we check if the connection can be reused before even checking if
             // it is a "matching" connection.
             if (!conn->CanReuse()) {
                 LOG(("   dropping stale connection: [conn=%p]\n", conn.get()));
                 conn->Close(NS_ERROR_ABORT);
                 conn = nullptr;
             }
@@ -1696,34 +1693,33 @@ nsHttpConnectionMgr::DispatchTransaction
 // connection manager.
 //
 class ConnectionHandle : public nsAHttpConnection
 {
 public:
     NS_DECL_THREADSAFE_ISUPPORTS
     NS_DECL_NSAHTTPCONNECTION(mConn)
 
-    explicit ConnectionHandle(nsHttpConnection *conn) { NS_ADDREF(mConn = conn); }
-    nsHttpConnection *mConn;
-
+    explicit ConnectionHandle(nsHttpConnection *conn) : mConn(conn) { }
+    void Reset() { mConn = nullptr; }
 private:
     virtual ~ConnectionHandle();
+    RefPtr<nsHttpConnection> mConn;
 };
 
 nsAHttpConnection *
 nsHttpConnectionMgr::MakeConnectionHandle(nsHttpConnection *aWrapped)
 {
     return new ConnectionHandle(aWrapped);
 }
 
 ConnectionHandle::~ConnectionHandle()
 {
     if (mConn) {
         gHttpHandler->ReclaimConnection(mConn);
-        NS_RELEASE(mConn);
     }
 }
 
 NS_IMPL_ISUPPORTS0(ConnectionHandle)
 
 // Use this method for dispatching nsAHttpTransction's. It can only safely be
 // used upon first use of a connection when NPN has not negotiated SPDY vs
 // HTTP/1 yet as multiplexing onto an existing SPDY session requires a
@@ -1774,19 +1770,17 @@ nsHttpConnectionMgr::DispatchAbstractTra
         if (conn == ent->mYellowConnection)
             ent->OnYellowComplete();
         DecrementActiveConnCount(conn);
         ConditionallyStopTimeoutTick();
 
         // sever back references to connection, and do so without triggering
         // a call to ReclaimConnection ;-)
         transaction->SetConnection(nullptr);
-        NS_RELEASE(handle->mConn);
-        // destroy the connection
-        NS_RELEASE(conn);
+        handle->Reset(); // destroy the connection
     }
 
     // As transaction goes out of scope it will drop the last refernece to the
     // pipeline if activation failed, in which case this will destroy
     // the pipeline, which will cause each the transactions owned by the
     // pipeline to be restarted.
 
     return rv;
@@ -1871,17 +1865,17 @@ nsHttpConnectionMgr::ProcessNewTransacti
 
     // Check if the transaction already has a sticky reference to a connection.
     // If so, then we can just use it directly by transferring its reference
     // to the new connection variable instead of searching for a new one
 
     nsAHttpConnection *wrappedConnection = trans->Connection();
     RefPtr<nsHttpConnection> conn;
     if (wrappedConnection)
-        conn = dont_AddRef(wrappedConnection->TakeHttpConnection());
+        conn = wrappedConnection->TakeHttpConnection();
 
     if (conn) {
         MOZ_ASSERT(trans->Caps() & NS_HTTP_STICKY_CONNECTION);
         LOG(("nsHttpConnectionMgr::ProcessNewTransaction trans=%p "
              "sticky connection=%p\n", trans, conn.get()));
 
         if (static_cast<int32_t>(ent->mActiveConns.IndexOf(conn)) == -1) {
             LOG(("nsHttpConnectionMgr::ProcessNewTransaction trans=%p "
@@ -1907,30 +1901,28 @@ nsHttpConnectionMgr::ProcessNewTransacti
     }
 
     if (rv == NS_ERROR_NOT_AVAILABLE) {
         LOG(("  adding transaction to pending queue "
              "[trans=%p pending-count=%u]\n",
              trans, ent->mPendingQ.Length()+1));
         // put this transaction on the pending queue...
         InsertTransactionSorted(ent->mPendingQ, trans);
-        NS_ADDREF(trans);
         return NS_OK;
     }
 
     LOG(("  ProcessNewTransaction Hard Error trans=%p rv=%x\n", trans, rv));
     return rv;
 }
 
 
 void
 nsHttpConnectionMgr::AddActiveConn(nsHttpConnection *conn,
                                    nsConnectionEntry *ent)
 {
-    NS_ADDREF(conn);
     ent->mActiveConns.AppendElement(conn);
     mNumActiveConns++;
     ActivateTimeoutTick();
 }
 
 void
 nsHttpConnectionMgr::DecrementActiveConnCount(nsHttpConnection *conn)
 {
@@ -1996,17 +1988,17 @@ nsHttpConnectionMgr::CreateTransport(nsC
 
 void
 nsHttpConnectionMgr::ProcessSpdyPendingQ(nsConnectionEntry *ent)
 {
     nsHttpConnection *conn = GetSpdyPreferredConn(ent);
     if (!conn || !conn->CanDirectlyActivate())
         return;
 
-    nsTArray<nsHttpTransaction*> leftovers;
+    nsTArray<RefPtr<nsHttpTransaction> > leftovers;
     uint32_t index;
 
     // Dispatch all the transactions we can
     for (index = 0;
          index < ent->mPendingQ.Length() && conn->CanDirectlyActivate();
          ++index) {
         nsHttpTransaction *trans = ent->mPendingQ[index];
 
@@ -2020,17 +2012,16 @@ nsHttpConnectionMgr::ProcessSpdyPendingQ
         if (NS_FAILED(rv)) {
             // this cannot happen, but if due to some bug it does then
             // close the transaction
             MOZ_ASSERT(false, "Dispatch SPDY Transaction");
             LOG(("ProcessSpdyPendingQ Dispatch Transaction failed trans=%p\n",
                     trans));
             trans->Close(rv);
         }
-        NS_RELEASE(trans);
     }
 
     // Slurp up the rest of the pending queue into our leftovers bucket (we
     // might have some left if conn->CanDirectlyActivate returned false)
     for (; index < ent->mPendingQ.Length(); ++index) {
         nsHttpTransaction *trans = ent->mPendingQ[index];
         leftovers.AppendElement(trans);
     }
@@ -2117,48 +2108,41 @@ nsHttpConnectionMgr::OnMsgShutdown(int32
 
     gHttpHandler->StopRequestTokenBucket();
 
     for (auto iter = mCT.Iter(); !iter.Done(); iter.Next()) {
         nsAutoPtr<nsConnectionEntry>& ent = iter.Data();
 
         // Close all active connections.
         while (ent->mActiveConns.Length()) {
-            nsHttpConnection* conn = ent->mActiveConns[0];
-
+            RefPtr<nsHttpConnection> conn(ent->mActiveConns[0]);
             ent->mActiveConns.RemoveElementAt(0);
             DecrementActiveConnCount(conn);
-
             conn->Close(NS_ERROR_ABORT, true);
-            NS_RELEASE(conn);
         }
 
         // Close all idle connections.
         while (ent->mIdleConns.Length()) {
-            nsHttpConnection* conn = ent->mIdleConns[0];
+            RefPtr<nsHttpConnection> conn(ent->mIdleConns[0]);
 
             ent->mIdleConns.RemoveElementAt(0);
             mNumIdleConns--;
 
             conn->Close(NS_ERROR_ABORT);
-            NS_RELEASE(conn);
         }
 
         // If all idle connections are removed we can stop pruning dead
         // connections.
         ConditionallyStopPruneDeadConnectionsTimer();
 
         // Close all pending transactions.
         while (ent->mPendingQ.Length()) {
-            nsHttpTransaction* trans = ent->mPendingQ[0];
-
+            nsHttpTransaction *trans = ent->mPendingQ[0];
+            trans->Close(NS_ERROR_ABORT);
             ent->mPendingQ.RemoveElementAt(0);
-
-            trans->Close(NS_ERROR_ABORT);
-            NS_RELEASE(trans);
         }
 
         // Close all half open tcp connections.
         for (int32_t i = int32_t(ent->mHalfOpens.Length()) - 1; i >= 0; i--) {
             ent->mHalfOpens[i]->Abandon();
         }
 
         iter.Remove();
@@ -2208,17 +2192,17 @@ nsHttpConnectionMgr::OnMsgNewTransaction
 }
 
 void
 nsHttpConnectionMgr::OnMsgReschedTransaction(int32_t priority, ARefBase *param)
 {
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
     LOG(("nsHttpConnectionMgr::OnMsgReschedTransaction [trans=%p]\n", param));
 
-    nsHttpTransaction *trans = static_cast<nsHttpTransaction *>(param);
+    RefPtr<nsHttpTransaction> trans = static_cast<nsHttpTransaction *>(param);
     trans->SetPriority(priority);
 
     nsConnectionEntry *ent = LookupConnectionEntry(trans->ConnectionInfo(),
                                                    nullptr, trans);
 
     if (ent) {
         int32_t index = ent->mPendingQ.IndexOf(trans);
         if (index >= 0) {
@@ -2230,16 +2214,18 @@ nsHttpConnectionMgr::OnMsgReschedTransac
 
 void
 nsHttpConnectionMgr::OnMsgCancelTransaction(int32_t reason, ARefBase *param)
 {
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
     LOG(("nsHttpConnectionMgr::OnMsgCancelTransaction [trans=%p]\n", param));
 
     nsresult closeCode = static_cast<nsresult>(reason);
+
+    // caller holds a ref to param/trans on stack
     nsHttpTransaction *trans = static_cast<nsHttpTransaction *>(param);
 
     //
     // if the transaction owns a connection and the transaction is not done,
     // then ask the connection to close the transaction.  otherwise, close the
     // transaction directly (removing it from the pending queue first).
     //
     RefPtr<nsAHttpConnection> conn(trans->Connection());
@@ -2250,18 +2236,16 @@ nsHttpConnectionMgr::OnMsgCancelTransact
             LookupConnectionEntry(trans->ConnectionInfo(), nullptr, trans);
 
         if (ent) {
             int32_t index = ent->mPendingQ.IndexOf(trans);
             if (index >= 0) {
                 LOG(("nsHttpConnectionMgr::OnMsgCancelTransaction [trans=%p]"
                      " found in pending queue\n", trans));
                 ent->mPendingQ.RemoveElementAt(index);
-                nsHttpTransaction *temp = trans;
-                NS_RELEASE(temp); // b/c NS_RELEASE nulls its argument!
             }
 
             // Abandon all half-open sockets belonging to the given transaction.
             for (uint32_t index = 0;
                  index < ent->mHalfOpens.Length();
                  ++index) {
                 nsHalfOpenSocket *half = ent->mHalfOpens[index];
                 if (trans == half->Transaction()) {
@@ -2342,24 +2326,22 @@ nsHttpConnectionMgr::OnMsgCancelTransact
     nsHttpConnectionInfo *ci = static_cast<nsHttpConnectionInfo *>(param);
     nsConnectionEntry *ent = mCT.Get(ci->HashKey());
     LOG(("nsHttpConnectionMgr::OnMsgCancelTransactions %s %p\n",
          ci->HashKey().get(), ent));
     if (!ent) {
         return;
     }
 
-    RefPtr<nsHttpTransaction> trans;
     for (int32_t i = ent->mPendingQ.Length() - 1; i >= 0; --i) {
-        trans = dont_AddRef(ent->mPendingQ[i]);
+        nsHttpTransaction *trans = ent->mPendingQ[i];
         LOG(("nsHttpConnectionMgr::OnMsgCancelTransactions %s %p %p\n",
-             ci->HashKey().get(), ent, trans.get()));
+             ci->HashKey().get(), ent, trans));
+        trans->Close(reason);
         ent->mPendingQ.RemoveElementAt(i);
-        trans->Close(reason);
-        trans = nullptr;
     }
 }
 
 void
 nsHttpConnectionMgr::OnMsgPruneDeadConnections(int32_t, ARefBase *)
 {
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
     LOG(("nsHttpConnectionMgr::OnMsgPruneDeadConnections\n"));
@@ -2376,21 +2358,20 @@ nsHttpConnectionMgr::OnMsgPruneDeadConne
             LOG(("  pruning [ci=%s]\n", ent->mConnInfo->HashKey().get()));
 
             // Find out how long it will take for next idle connection to not
             // be reusable anymore.
             uint32_t timeToNextExpire = UINT32_MAX;
             int32_t count = ent->mIdleConns.Length();
             if (count > 0) {
                 for (int32_t i = count - 1; i >= 0; --i) {
-                    nsHttpConnection* conn = ent->mIdleConns[i];
+                    RefPtr<nsHttpConnection> conn(ent->mIdleConns[i]);
                     if (!conn->CanReuse()) {
                         ent->mIdleConns.RemoveElementAt(i);
                         conn->Close(NS_ERROR_ABORT);
-                        NS_RELEASE(conn);
                         mNumIdleConns--;
                     } else {
                         timeToNextExpire =
                             std::min(timeToNextExpire, conn->TimeToLive());
                     }
                 }
             }
 
@@ -2466,18 +2447,17 @@ nsHttpConnectionMgr::OnMsgPruneNoTraffic
         LOG(("  pruning no traffic [ci=%s]\n",
              ent->mConnInfo->HashKey().get()));
 
         uint32_t numConns = ent->mActiveConns.Length();
         if (numConns) {
             // Walk the list backwards to allow us to remove entries easily.
             for (int index = numConns - 1; index >= 0; index--) {
                 if (ent->mActiveConns[index]->NoTraffic()) {
-                    RefPtr<nsHttpConnection> conn =
-                        dont_AddRef(ent->mActiveConns[index]);
+                    RefPtr<nsHttpConnection> conn = ent->mActiveConns[index];
                     ent->mActiveConns.RemoveElementAt(index);
                     DecrementActiveConnCount(conn);
                     conn->Close(NS_ERROR_ABORT);
                     LOG(("  closed active connection due to no traffic "
                          "[conn=%p]\n", conn.get()));
                 }
             }
         }
@@ -2595,18 +2575,16 @@ nsHttpConnectionMgr::OnMsgReclaimConnect
     if (conn->Transaction()) {
         conn->DontReuse();
     }
 
     if (ent->mActiveConns.RemoveElement(conn)) {
         if (conn == ent->mYellowConnection) {
             ent->OnYellowComplete();
         }
-        nsHttpConnection *temp = conn;
-        NS_RELEASE(temp);
         DecrementActiveConnCount(conn);
         ConditionallyStopTimeoutTick();
     }
 
     if (conn->CanReuse()) {
         LOG(("  adding connection to idle list\n"));
         // Keep The idle connection list sorted with the connections that
         // have moved the largest data pipelines at the front because these
@@ -2617,17 +2595,16 @@ nsHttpConnectionMgr::OnMsgReclaimConnect
 
         uint32_t idx;
         for (idx = 0; idx < ent->mIdleConns.Length(); idx++) {
             nsHttpConnection *idleConn = ent->mIdleConns[idx];
             if (idleConn->MaxBytesRead() < conn->MaxBytesRead())
                 break;
         }
 
-        NS_ADDREF(conn);
         ent->mIdleConns.InsertElementAt(idx, conn);
         mNumIdleConns++;
         conn->BeginIdleMonitoring();
 
         // If the added connection was first idle connection or has shortest
         // time to live among the watched connections, pruning dead
         // connections needs to be done when it can't be reused anymore.
         uint32_t timeToLive = conn->TimeToLive();
@@ -3360,17 +3337,17 @@ nsHalfOpenSocket::OnOutputStreamReady(ns
     // from counter of actual connections used for checking limits.
     mHasConnected = true;
 
     // if this is still in the pending list, remove it and dispatch it
     index = mEnt->mPendingQ.IndexOf(mTransaction);
     if (index != -1) {
         MOZ_ASSERT(!mSpeculative,
                    "Speculative Half Open found mTransaction");
-        RefPtr<nsHttpTransaction> temp = dont_AddRef(mEnt->mPendingQ[index]);
+        RefPtr<nsHttpTransaction> temp = mEnt->mPendingQ[index];
         mEnt->mPendingQ.RemoveElementAt(index);
         gHttpHandler->ConnMgr()->AddActiveConn(conn, mEnt);
         rv = gHttpHandler->ConnMgr()->DispatchTransaction(mEnt, temp, conn);
     } else {
         // this transaction was dispatched off the pending q before all the
         // sockets established themselves.
 
         // After about 1 second allow for the possibility of restarting a
@@ -3516,26 +3493,23 @@ nsHttpConnectionMgr::nsHalfOpenSocket::G
         mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks));
         if (callbacks)
             return callbacks->GetInterface(iid, result);
     }
     return NS_ERROR_NO_INTERFACE;
 }
 
 
-nsHttpConnection *
+already_AddRefed<nsHttpConnection>
 ConnectionHandle::TakeHttpConnection()
 {
     // return our connection object to the caller and clear it internally
     // do not drop our reference - the caller now owns it.
-
     MOZ_ASSERT(mConn);
-    nsHttpConnection *conn = mConn;
-    mConn = nullptr;
-    return conn;
+    return mConn.forget();
 }
 
 uint32_t
 ConnectionHandle::CancelPipeline(nsresult reason)
 {
     // no pipeline to cancel
     return 0;
 }
@@ -3973,16 +3947,17 @@ nsHttpConnectionMgr::MoveToWildCardConnE
          ent->mHalfOpens.Length(), ent->mPendingQ.Length()));
 
     LOG(("nsHttpConnectionMgr::MakeConnEntryWildCard wc-ent %p "
          "idle=%d active=%d half=%d pending=%d\n", wcEnt,
          wcEnt->mIdleConns.Length(), wcEnt->mActiveConns.Length(),
          wcEnt->mHalfOpens.Length(), wcEnt->mPendingQ.Length()));
 
     int32_t count = ent->mActiveConns.Length();
+    RefPtr<nsHttpConnection> deleteProtector(proxyConn);
     for (int32_t i = 0; i < count; ++i) {
         if (ent->mActiveConns[i] == proxyConn) {
             ent->mActiveConns.RemoveElementAt(i);
             wcEnt->mActiveConns.InsertElementAt(0, proxyConn);
             return;
         }
     }
 
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -288,19 +288,19 @@ private:
     //
     class nsConnectionEntry
     {
     public:
         explicit nsConnectionEntry(nsHttpConnectionInfo *ci);
         ~nsConnectionEntry();
 
         RefPtr<nsHttpConnectionInfo> mConnInfo;
-        nsTArray<nsHttpTransaction*> mPendingQ;    // pending transaction queue
-        nsTArray<nsHttpConnection*>  mActiveConns; // active connections
-        nsTArray<nsHttpConnection*>  mIdleConns;   // idle persistent connections
+        nsTArray<RefPtr<nsHttpTransaction> > mPendingQ;    // pending transaction queue
+        nsTArray<RefPtr<nsHttpConnection> >  mActiveConns; // active connections
+        nsTArray<RefPtr<nsHttpConnection> >  mIdleConns;   // idle persistent connections
         nsTArray<nsHalfOpenSocket*>  mHalfOpens;   // half open connections
 
         bool AvailableForDispatchNow();
 
         // calculate the number of half open sockets that have not had at least 1
         // connection complete
         uint32_t UnconnectedHalfOpens();
 
--- a/netwerk/protocol/http/nsHttpPipeline.cpp
+++ b/netwerk/protocol/http/nsHttpPipeline.cpp
@@ -83,22 +83,23 @@ nsHttpPipeline::~nsHttpPipeline()
 
     if (mPushBackBuf)
         free(mPushBackBuf);
 }
 
 nsresult
 nsHttpPipeline::AddTransaction(nsAHttpTransaction *trans)
 {
-    LOG(("nsHttpPipeline::AddTransaction [this=%p trans=%x]\n", this, trans));
+    LOG(("nsHttpPipeline::AddTransaction [this=%p trans=%p]\n", this, trans));
 
     if (mRequestQ.Length() || mResponseQ.Length())
         mUtilizedPipeline = true;
 
-    NS_ADDREF(trans);
+    // A reference to the actual transaction is held by the pipeline transaction
+    // in either the request or response queue
     mRequestQ.AppendElement(trans);
     uint32_t qlen = PipelineDepth();
 
     if (qlen != 1) {
         trans->SetPipelinePosition(qlen);
     }
     else {
         // do it for this case in case an idempotent cancellation
@@ -196,55 +197,51 @@ nsHttpPipeline::OnHeadersAvailable(nsAHt
         // The received headers have expanded the eligible
         // pipeline depth for this connection
         gHttpHandler->ConnMgr()->ProcessPendingQForEntry(ci);
 
     return rv;
 }
 
 void
-nsHttpPipeline::CloseTransaction(nsAHttpTransaction *trans, nsresult reason)
+nsHttpPipeline::CloseTransaction(nsAHttpTransaction *aTrans, nsresult reason)
 {
-    LOG(("nsHttpPipeline::CloseTransaction [this=%p trans=%x reason=%x]\n",
-        this, trans, reason));
+    LOG(("nsHttpPipeline::CloseTransaction [this=%p trans=%p reason=%x]\n",
+        this, aTrans, reason));
 
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
     MOZ_ASSERT(NS_FAILED(reason), "expecting failure code");
 
     // the specified transaction is to be closed with the given "reason"
-
+    RefPtr<nsAHttpTransaction> trans(aTrans);
     int32_t index;
     bool killPipeline = false;
 
-    index = mRequestQ.IndexOf(trans);
-    if (index >= 0) {
+    if ((index = mRequestQ.IndexOf(trans)) >= 0) {
         if (index == 0 && mRequestIsPartial) {
             // the transaction is in the request queue.  check to see if any of
             // its data has been written out yet.
             killPipeline = true;
         }
         mRequestQ.RemoveElementAt(index);
-    }
-    else {
-        index = mResponseQ.IndexOf(trans);
-        if (index >= 0)
-            mResponseQ.RemoveElementAt(index);
+    } else if ((index = mResponseQ.IndexOf(trans)) >= 0) {
+        mResponseQ.RemoveElementAt(index);
         // while we could avoid killing the pipeline if this transaction is the
         // last transaction in the pipeline, there doesn't seem to be that much
         // value in doing so.  most likely if this transaction is going away,
         // the others will be shortly as well.
         killPipeline = true;
     }
 
     // Marking this connection as non-reusable prevents other items from being
     // added to it and causes it to be torn down soon.
     DontReuse();
 
     trans->Close(reason);
-    NS_RELEASE(trans);
+    trans = nullptr;
 
     if (killPipeline) {
         // reschedule anything from this pipeline onto a different connection
         CancelPipeline(reason);
     }
 
     // If all the transactions have been removed then we can close the connection
     // right away.
@@ -321,17 +318,17 @@ nsHttpPipeline::PushBack(const char *dat
     }
 
     memcpy(mPushBackBuf, data, length);
     mPushBackLen = length;
 
     return NS_OK;
 }
 
-nsHttpConnection *
+already_AddRefed<nsHttpConnection>
 nsHttpPipeline::TakeHttpConnection()
 {
     if (mConnection)
         return mConnection->TakeHttpConnection();
     return nullptr;
 }
 
 nsAHttpTransaction::Classifier
@@ -377,47 +374,46 @@ nsHttpPipeline::TakeSubTransactions(
     LOG(("nsHttpPipeline::TakeSubTransactions [this=%p]\n", this));
 
     if (mResponseQ.Length() || mRequestIsPartial)
         return NS_ERROR_ALREADY_OPENED;
 
     int32_t i, count = mRequestQ.Length();
     for (i = 0; i < count; ++i) {
         nsAHttpTransaction *trans = Request(i);
-        // set the transaction conneciton object back to the underlying
+        // set the transaction connection object back to the underlying
         // nsHttpConnectionHandle
         trans->SetConnection(mConnection);
         outTransactions.AppendElement(trans);
-        NS_RELEASE(trans);
     }
     mRequestQ.Clear();
 
     LOG(("   took %d\n", count));
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpPipeline::nsAHttpTransaction
 //-----------------------------------------------------------------------------
 
 void
 nsHttpPipeline::SetConnection(nsAHttpConnection *conn)
 {
-    LOG(("nsHttpPipeline::SetConnection [this=%p conn=%x]\n", this, conn));
+    LOG(("nsHttpPipeline::SetConnection [this=%p conn=%p]\n", this, conn));
 
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
     MOZ_ASSERT(!conn || !mConnection, "already have a connection");
 
     mConnection = conn;
 }
 
 nsAHttpConnection *
 nsHttpPipeline::Connection()
 {
-    LOG(("nsHttpPipeline::Connection [this=%p conn=%x]\n", this, mConnection.get()));
+    LOG(("nsHttpPipeline::Connection [this=%p conn=%p]\n", this, mConnection.get()));
 
     MOZ_ASSERT(PR_GetCurrentThread() == gSocketThread);
     return mConnection;
 }
 
 void
 nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result)
 {
@@ -676,30 +672,28 @@ nsHttpPipeline::WriteSegments(nsAHttpSeg
         trans = Request(0);
     }
 
     if (!trans) {
         if (mRequestQ.Length() > 0)
             rv = NS_BASE_STREAM_WOULD_BLOCK;
         else
             rv = NS_BASE_STREAM_CLOSED;
-    }
-    else {
+    } else {
         //
         // ask the transaction to consume data from the connection.
         // PushBack may be called recursively.
         //
         rv = trans->WriteSegments(writer, count, countWritten);
 
         if (rv == NS_BASE_STREAM_CLOSED || trans->IsDone()) {
             trans->Close(NS_OK);
 
             // Release the transaction if it is not IsProxyConnectInProgress()
             if (trans == Response(0)) {
-                NS_RELEASE(trans);
                 mResponseQ.RemoveElementAt(0);
                 mResponseIsPartial = false;
                 ++mHttp1xTransactionCount;
             }
 
             // ask the connection manager to add additional transactions
             // to our pipeline.
             RefPtr<nsHttpConnectionInfo> ci;
@@ -753,28 +747,26 @@ nsHttpPipeline::CancelPipeline(nsresult 
     // any pending requests can ignore this error and be restarted
     // unless it is during a CONNECT tunnel request
     for (i = 0; i < reqLen; ++i) {
         trans = Request(i);
         if (mConnection && mConnection->IsProxyConnectInProgress())
             trans->Close(originalReason);
         else
             trans->Close(NS_ERROR_NET_RESET);
-        NS_RELEASE(trans);
     }
     mRequestQ.Clear();
 
     // any pending responses can be restarted except for the first one,
     // that we might want to finish on this pipeline or cancel individually.
     // Higher levels of callers ensure that we don't process non-idempotent
     // tranasction with the NS_HTTP_ALLOW_PIPELINING bit set
     for (i = 1; i < respLen; ++i) {
         trans = Response(i);
         trans->Close(NS_ERROR_NET_RESET);
-        NS_RELEASE(trans);
     }
 
     if (respLen > 1)
         mResponseQ.TruncateLength(1);
 
     DontReuse();
     Classify(nsAHttpTransaction::CLASS_SOLO);
 
@@ -820,17 +812,16 @@ nsHttpPipeline::Close(nsresult reason)
          reason == NS_ERROR_NET_TIMEOUT ||
          reason == NS_BASE_STREAM_CLOSED)) {
         trans->Close(NS_ERROR_NET_RESET);
     }
     else {
         trans->Close(reason);
     }
 
-    NS_RELEASE(trans);
     mResponseQ.Clear();
 }
 
 nsresult
 nsHttpPipeline::OnReadSegment(const char *segment,
                               uint32_t count,
                               uint32_t *countRead)
 {
@@ -852,17 +843,17 @@ nsHttpPipeline::FillSendBuf()
                         nsIOService::gDefaultSegmentSize,  /* segment size */
                         nsIOService::gDefaultSegmentSize,  /* max size */
                         true, true);
         if (NS_FAILED(rv)) return rv;
     }
 
     uint32_t n;
     uint64_t avail;
-    nsAHttpTransaction *trans;
+    RefPtr<nsAHttpTransaction> trans;
     nsITransport *transport = Transport();
 
     while ((trans = Request(0)) != nullptr) {
         avail = trans->Available();
         if (avail) {
             // if there is already a response in the responseq then this
             // new data comprises a pipeline. Update the transaction in the
             // response queue to reflect that if necessary. We are now sending
--- a/netwerk/protocol/http/nsHttpPipeline.h
+++ b/netwerk/protocol/http/nsHttpPipeline.h
@@ -55,18 +55,18 @@ private:
 
         return mResponseQ[i];
     }
 
     // overload of nsAHttpTransaction::QueryPipeline()
     nsHttpPipeline *QueryPipeline() override;
 
     RefPtr<nsAHttpConnection>   mConnection;
-    nsTArray<nsAHttpTransaction*> mRequestQ;  // array of transactions
-    nsTArray<nsAHttpTransaction*> mResponseQ; // array of transactions
+    nsTArray<RefPtr<nsAHttpTransaction> > mRequestQ;
+    nsTArray<RefPtr<nsAHttpTransaction> > mResponseQ;
     nsresult                      mStatus;
 
     // these flags indicate whether or not the first request or response
     // is partial.  a partial request means that Request(0) has been
     // partially written out to the socket.  a partial response means
     // that Response(0) has been partially read in from the socket.
     bool mRequestIsPartial;
     bool mResponseIsPartial;
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -79,17 +79,16 @@ LogHeaders(const char *lineStart)
 
 //-----------------------------------------------------------------------------
 // nsHttpTransaction <public>
 //-----------------------------------------------------------------------------
 
 nsHttpTransaction::nsHttpTransaction()
     : mLock("transaction lock")
     , mRequestSize(0)
-    , mConnection(nullptr)
     , mRequestHead(nullptr)
     , mResponseHead(nullptr)
     , mReader(nullptr)
     , mWriter(nullptr)
     , mContentLength(-1)
     , mContentRead(0)
     , mTransferSize(0)
     , mInvalidResponseBytesRead(0)