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 291040 2204c405d07c203a32bad351200e81b43ac062c4
parent 291039 afb08f0036485e6840155eb3104327345670cd63
child 291041 2f39deb1b3e2865ced9cead27a03e97d729fbcfb
push id74434
push userkwierso@gmail.com
push dateThu, 31 Mar 2016 21:07:54 +0000
treeherdermozilla-inbound@c91015c43d55 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdragana, kwierso
bugs905460
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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)