From 5faa109e12b651042a428c6580048a9486424e58 Mon Sep 17 00:00:00 2001 From: Josh Matthews Date: Wed, 14 Nov 2012 16:00:44 +0000 Subject: [PATCH] Bug 804655 - Part 2: Wrap up interface aggregator callbacks with a target thread on which they should be released. r=mayhemer --- browser/components/search/content/search.xml | 4 +- mobile/android/chrome/content/browser.js | 2 +- netwerk/base/public/nsISpeculativeConnect.idl | 8 +--- netwerk/base/public/nsNetUtil.h | 11 ++++- netwerk/base/src/nsIOService.cpp | 11 ++--- netwerk/protocol/http/NullHttpTransaction.cpp | 17 +------- netwerk/protocol/http/NullHttpTransaction.h | 3 -- netwerk/protocol/http/SpdySession2.cpp | 3 +- netwerk/protocol/http/SpdySession3.cpp | 3 +- netwerk/protocol/http/nsAHttpConnection.h | 8 ++-- netwerk/protocol/http/nsAHttpTransaction.h | 6 +-- netwerk/protocol/http/nsHttpChannel.cpp | 5 ++- netwerk/protocol/http/nsHttpConnection.cpp | 33 +++----------- netwerk/protocol/http/nsHttpConnection.h | 7 +-- netwerk/protocol/http/nsHttpConnectionMgr.cpp | 22 +++++----- netwerk/protocol/http/nsHttpConnectionMgr.h | 3 +- netwerk/protocol/http/nsHttpHandler.cpp | 5 +-- netwerk/protocol/http/nsHttpHandler.h | 5 +-- netwerk/protocol/http/nsHttpPipeline.cpp | 7 +-- netwerk/protocol/http/nsHttpTransaction.cpp | 14 +++--- netwerk/protocol/http/nsHttpTransaction.h | 3 +- netwerk/test/unit/test_speculative_connect.js | 2 +- xpcom/base/nsInterfaceRequestorAgg.cpp | 43 ++++++++++++++++++- xpcom/base/nsInterfaceRequestorAgg.h | 15 ++++++- 24 files changed, 119 insertions(+), 121 deletions(-) diff --git a/browser/components/search/content/search.xml b/browser/components/search/content/search.xml index e354572ce0d..0da1d1fec5f 100644 --- a/browser/components/search/content/search.xml +++ b/browser/components/search/content/search.xml @@ -498,12 +498,12 @@ var connector = Services.io.QueryInterface(Components.interfaces.nsISpeculativeConnect); var searchURI = engine.getSubmission("dummy").uri; - connector.speculativeConnect(searchURI, null, null); + connector.speculativeConnect(searchURI, null); if (engine.supportsResponseType(SUGGEST_TYPE)) { var suggestURI = engine.getSubmission("dummy", SUGGEST_TYPE).uri; if (suggestURI.prePath != searchURI.prePath) - connector.speculativeConnect(suggestURI, null, null); + connector.speculativeConnect(suggestURI, null); } ]]> diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index ebf88bf03be..e93a6fcc7e2 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -3802,7 +3802,7 @@ var BrowserEventHandler = { if (closest) { let uri = this._getLinkURI(closest); if (uri) { - Services.io.QueryInterface(Ci.nsISpeculativeConnect).speculativeConnect(uri, null, null); + Services.io.QueryInterface(Ci.nsISpeculativeConnect).speculativeConnect(uri, null); } this._doTapHighlight(closest); } diff --git a/netwerk/base/public/nsISpeculativeConnect.idl b/netwerk/base/public/nsISpeculativeConnect.idl index 63d785d791f..f77628f45cb 100644 --- a/netwerk/base/public/nsISpeculativeConnect.idl +++ b/netwerk/base/public/nsISpeculativeConnect.idl @@ -7,9 +7,8 @@ interface nsIURI; interface nsIInterfaceRequestor; -interface nsIEventTarget; -[scriptable, uuid(b3c53863-1313-480a-90a2-5b0da651ee5e)] +[scriptable, uuid(fa580a8d-f4a4-47c5-8ade-4f9786e8d1de)] interface nsISpeculativeConnect : nsISupports { /** @@ -24,13 +23,10 @@ interface nsISpeculativeConnect : nsISupports * @param aURI the URI of the hinted transaction * @param aCallbacks any security callbacks for use with SSL for interfaces * such as nsIBadCertListener. May be null. - * @param aTarget the thread on which the release of the callbacks will - * occur. May be null for "any thread". * */ void speculativeConnect(in nsIURI aURI, - in nsIInterfaceRequestor aCallbacks, - in nsIEventTarget aTarget); + in nsIInterfaceRequestor aCallbacks); }; diff --git a/netwerk/base/public/nsNetUtil.h b/netwerk/base/public/nsNetUtil.h index 8df13000864..58d33380900 100644 --- a/netwerk/base/public/nsNetUtil.h +++ b/netwerk/base/public/nsNetUtil.h @@ -1504,12 +1504,21 @@ NS_QueryNotificationCallbacks(const nsCOMPtr &aChannel, inline nsresult NS_NewNotificationCallbacksAggregation(nsIInterfaceRequestor *callbacks, nsILoadGroup *loadGroup, + nsIEventTarget *target, nsIInterfaceRequestor **result) { nsCOMPtr cbs; if (loadGroup) loadGroup->GetNotificationCallbacks(getter_AddRefs(cbs)); - return NS_NewInterfaceRequestorAggregation(callbacks, cbs, result); + return NS_NewInterfaceRequestorAggregation(callbacks, cbs, target, result); +} + +inline nsresult +NS_NewNotificationCallbacksAggregation(nsIInterfaceRequestor *callbacks, + nsILoadGroup *loadGroup, + nsIInterfaceRequestor **result) +{ + return NS_NewNotificationCallbacksAggregation(callbacks, loadGroup, nullptr, result); } /** diff --git a/netwerk/base/src/nsIOService.cpp b/netwerk/base/src/nsIOService.cpp index 1793a08e52e..c008963ddc4 100644 --- a/netwerk/base/src/nsIOService.cpp +++ b/netwerk/base/src/nsIOService.cpp @@ -1178,16 +1178,13 @@ public: NS_DECL_NSIPROTOCOLPROXYCALLBACK IOServiceProxyCallback(nsIInterfaceRequestor *aCallbacks, - nsIEventTarget *aTarget, nsIOService *aIOService) : mCallbacks(aCallbacks) - , mTarget(aTarget) , mIOService(aIOService) { } private: nsRefPtr mCallbacks; - nsRefPtr mTarget; nsRefPtr mIOService; }; @@ -1223,15 +1220,13 @@ IOServiceProxyCallback::OnProxyAvailable(nsICancelable *request, nsIURI *aURI, return NS_OK; speculativeHandler->SpeculativeConnect(aURI, - mCallbacks, - mTarget); + mCallbacks); return NS_OK; } NS_IMETHODIMP nsIOService::SpeculativeConnect(nsIURI *aURI, - nsIInterfaceRequestor *aCallbacks, - nsIEventTarget *aTarget) + nsIInterfaceRequestor *aCallbacks) { // Check for proxy information. If there is a proxy configured then a // speculative connect should not be performed because the potential @@ -1244,6 +1239,6 @@ nsIOService::SpeculativeConnect(nsIURI *aURI, nsCOMPtr cancelable; nsRefPtr callback = - new IOServiceProxyCallback(aCallbacks, aTarget, this); + new IOServiceProxyCallback(aCallbacks, this); return pps->AsyncResolve(aURI, 0, callback, getter_AddRefs(cancelable)); } diff --git a/netwerk/protocol/http/NullHttpTransaction.cpp b/netwerk/protocol/http/NullHttpTransaction.cpp index 30cd1c0b135..33ae1caeabd 100644 --- a/netwerk/protocol/http/NullHttpTransaction.cpp +++ b/netwerk/protocol/http/NullHttpTransaction.cpp @@ -16,12 +16,10 @@ NS_IMPL_THREADSAFE_ISUPPORTS0(NullHttpTransaction) NullHttpTransaction::NullHttpTransaction(nsHttpConnectionInfo *ci, nsIInterfaceRequestor *callbacks, - nsIEventTarget *target, uint8_t caps) : mStatus(NS_OK) , mCaps(caps | NS_HTTP_ALLOW_KEEPALIVE) , mCallbacks(callbacks) - , mEventTarget(target) , mConnectionInfo(ci) , mRequestHead(nullptr) , mIsDone(false) @@ -30,11 +28,7 @@ NullHttpTransaction::NullHttpTransaction(nsHttpConnectionInfo *ci, NullHttpTransaction::~NullHttpTransaction() { - if (mCallbacks) { - nsIInterfaceRequestor *cbs = nullptr; - mCallbacks.swap(cbs); - NS_ProxyRelease(mEventTarget, cbs); - } + mCallbacks = nullptr; delete mRequestHead; } @@ -51,18 +45,11 @@ NullHttpTransaction::Connection() } void -NullHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **outCB, - nsIEventTarget **outTarget) +NullHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **outCB) { nsCOMPtr copyCB(mCallbacks); *outCB = copyCB; copyCB.forget(); - - if (outTarget) { - nsCOMPtr copyET(mEventTarget); - *outTarget = copyET; - copyET.forget(); - } } void diff --git a/netwerk/protocol/http/NullHttpTransaction.h b/netwerk/protocol/http/NullHttpTransaction.h index 5e2fa1eeeaa..f4608faa1c5 100644 --- a/netwerk/protocol/http/NullHttpTransaction.h +++ b/netwerk/protocol/http/NullHttpTransaction.h @@ -10,7 +10,6 @@ #include "nsAHttpTransaction.h" #include "nsAHttpConnection.h" #include "nsIInterfaceRequestor.h" -#include "nsIEventTarget.h" #include "nsHttpConnectionInfo.h" #include "nsHttpRequestHead.h" #include "mozilla/Attributes.h" @@ -30,7 +29,6 @@ public: NullHttpTransaction(nsHttpConnectionInfo *ci, nsIInterfaceRequestor *callbacks, - nsIEventTarget *target, uint8_t caps); ~NullHttpTransaction(); @@ -45,7 +43,6 @@ private: uint8_t mCaps; nsRefPtr mConnection; nsCOMPtr mCallbacks; - nsCOMPtr mEventTarget; nsRefPtr mConnectionInfo; nsHttpRequestHead *mRequestHead; bool mIsDone; diff --git a/netwerk/protocol/http/SpdySession2.cpp b/netwerk/protocol/http/SpdySession2.cpp index 976d54d4bc5..d0489656b15 100644 --- a/netwerk/protocol/http/SpdySession2.cpp +++ b/netwerk/protocol/http/SpdySession2.cpp @@ -2119,8 +2119,7 @@ SpdySession2::SetConnection(nsAHttpConnection *) } void -SpdySession2::GetSecurityCallbacks(nsIInterfaceRequestor **, - nsIEventTarget **) +SpdySession2::GetSecurityCallbacks(nsIInterfaceRequestor **) { // This is unexpected NS_ABORT_IF_FALSE(false, "SpdySession2::GetSecurityCallbacks()"); diff --git a/netwerk/protocol/http/SpdySession3.cpp b/netwerk/protocol/http/SpdySession3.cpp index 5a1cd6aabb3..edab93ba7fd 100644 --- a/netwerk/protocol/http/SpdySession3.cpp +++ b/netwerk/protocol/http/SpdySession3.cpp @@ -2178,8 +2178,7 @@ SpdySession3::SetConnection(nsAHttpConnection *) } void -SpdySession3::GetSecurityCallbacks(nsIInterfaceRequestor **, - nsIEventTarget **) +SpdySession3::GetSecurityCallbacks(nsIInterfaceRequestor **) { // This is unexpected NS_ABORT_IF_FALSE(false, "SpdySession3::GetSecurityCallbacks()"); diff --git a/netwerk/protocol/http/nsAHttpConnection.h b/netwerk/protocol/http/nsAHttpConnection.h index 4920b31b55f..bde2d43eb1e 100644 --- a/netwerk/protocol/http/nsAHttpConnection.h +++ b/netwerk/protocol/http/nsAHttpConnection.h @@ -127,8 +127,7 @@ public: // Update the callbacks used to provide security info. May be called on // any thread. - virtual void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks, - nsIEventTarget* aCallbackTarget) = 0; + virtual void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks) = 0; }; #define NS_DECL_NSAHTTPCONNECTION(fwdObject) \ @@ -201,10 +200,9 @@ public: } \ int64_t BytesWritten() \ { return fwdObject ? (fwdObject)->BytesWritten() : 0; } \ - void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks, \ - nsIEventTarget* aCallbackTarget) \ + void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks) \ { \ - (fwdObject)->SetSecurityCallbacks(aCallbacks, aCallbackTarget); \ + (fwdObject)->SetSecurityCallbacks(aCallbacks); \ } #endif // nsAHttpConnection_h__ diff --git a/netwerk/protocol/http/nsAHttpTransaction.h b/netwerk/protocol/http/nsAHttpTransaction.h index 803b9942957..2b4b8fcaa3b 100644 --- a/netwerk/protocol/http/nsAHttpTransaction.h +++ b/netwerk/protocol/http/nsAHttpTransaction.h @@ -37,8 +37,7 @@ public: // called by the connection to get security callbacks to set on the // socket transport. - virtual void GetSecurityCallbacks(nsIInterfaceRequestor **, - nsIEventTarget **) = 0; + virtual void GetSecurityCallbacks(nsIInterfaceRequestor **) = 0; // called to report socket status (see nsITransportEventSink) virtual void OnTransportStatus(nsITransport* transport, @@ -138,8 +137,7 @@ public: #define NS_DECL_NSAHTTPTRANSACTION \ void SetConnection(nsAHttpConnection *); \ nsAHttpConnection *Connection(); \ - void GetSecurityCallbacks(nsIInterfaceRequestor **, \ - nsIEventTarget **); \ + void GetSecurityCallbacks(nsIInterfaceRequestor **); \ void OnTransportStatus(nsITransport* transport, \ nsresult status, uint64_t progress); \ bool IsDone(); \ diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 4cdfad35fd5..03d059228dc 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -537,7 +537,7 @@ nsHttpChannel::SpeculativeConnect() mConnectionInfo->SetAnonymous((mLoadFlags & LOAD_ANONYMOUS) != 0); mConnectionInfo->SetPrivate(mPrivateBrowsing); gHttpHandler->SpeculativeConnect(mConnectionInfo, - callbacks, NS_GetCurrentThread()); + callbacks); } void @@ -5933,8 +5933,9 @@ nsHttpChannel::UpdateAggregateCallbacks() } nsCOMPtr callbacks; NS_NewNotificationCallbacksAggregation(mCallbacks, mLoadGroup, + NS_GetCurrentThread(), getter_AddRefs(callbacks)); - mTransaction->SetSecurityCallbacks(callbacks, NS_GetCurrentThread()); + mTransaction->SetSecurityCallbacks(callbacks); } NS_IMETHODIMP diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp index fa2b04e4dd2..e7dae9ab5a0 100644 --- a/netwerk/protocol/http/nsHttpConnection.cpp +++ b/netwerk/protocol/http/nsHttpConnection.cpp @@ -78,12 +78,6 @@ nsHttpConnection::~nsHttpConnection() { LOG(("Destroying nsHttpConnection @%x\n", this)); - if (mCallbacks) { - nsIInterfaceRequestor *cbs = nullptr; - mCallbacks.swap(cbs); - NS_ProxyRelease(mCallbackTarget, cbs); - } - // release our reference to the handler nsHttpHandler *handler = gHttpHandler; NS_RELEASE(handler); @@ -113,7 +107,6 @@ nsHttpConnection::Init(nsHttpConnectionInfo *info, nsIAsyncInputStream *instream, nsIAsyncOutputStream *outstream, nsIInterfaceRequestor *callbacks, - nsIEventTarget *callbackTarget, PRIntervalTime rtt) { NS_ABORT_IF_FALSE(transport && instream && outstream, @@ -140,7 +133,6 @@ nsHttpConnection::Init(nsHttpConnectionInfo *info, NS_ENSURE_SUCCESS(rv, rv); mCallbacks = callbacks; - mCallbackTarget = callbackTarget; rv = mSocketTransport->SetSecurityCallbacks(this); NS_ENSURE_SUCCESS(rv, rv); @@ -325,10 +317,8 @@ nsHttpConnection::Activate(nsAHttpTransaction *trans, uint8_t caps, int32_t pri) // Update security callbacks nsCOMPtr callbacks; - nsCOMPtr callbackTarget; - trans->GetSecurityCallbacks(getter_AddRefs(callbacks), - getter_AddRefs(callbackTarget)); - SetSecurityCallbacks(callbacks, callbackTarget); + trans->GetSecurityCallbacks(getter_AddRefs(callbacks)); + SetSecurityCallbacks(callbacks); SetupNPN(caps); // only for spdy @@ -1038,19 +1028,10 @@ nsHttpConnection::GetSecurityInfo(nsISupports **secinfo) } void -nsHttpConnection::SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks, - nsIEventTarget* aCallbackTarget) +nsHttpConnection::SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks) { - nsCOMPtr callbacks = aCallbacks; - MutexAutoLock lock(mCallbacksLock); - if (aCallbacks == mCallbacks) - return; - - mCallbacks.swap(callbacks); - if (callbacks) - NS_ProxyRelease(mCallbackTarget, callbacks); - mCallbackTarget = aCallbackTarget; + mCallbacks = aCallbacks; } nsresult @@ -1168,11 +1149,7 @@ nsHttpConnection::CloseTransaction(nsAHttpTransaction *trans, nsresult reason) { MutexAutoLock lock(mCallbacksLock); - if (mCallbacks) { - nsIInterfaceRequestor *cbs = nullptr; - mCallbacks.swap(cbs); - NS_ProxyRelease(mCallbackTarget, cbs); - } + mCallbacks = nullptr; } if (NS_FAILED(reason)) diff --git a/netwerk/protocol/http/nsHttpConnection.h b/netwerk/protocol/http/nsHttpConnection.h index c862b0f7a74..1f3b41e3aa9 100644 --- a/netwerk/protocol/http/nsHttpConnection.h +++ b/netwerk/protocol/http/nsHttpConnection.h @@ -21,7 +21,6 @@ #include "nsIAsyncInputStream.h" #include "nsIAsyncOutputStream.h" #include "nsIInterfaceRequestor.h" -#include "nsIEventTarget.h" class nsHttpRequestHead; class nsHttpResponseHead; @@ -60,7 +59,7 @@ public: nsresult Init(nsHttpConnectionInfo *info, uint16_t maxHangTime, nsISocketTransport *, nsIAsyncInputStream *, nsIAsyncOutputStream *, nsIInterfaceRequestor *, - nsIEventTarget *, PRIntervalTime); + PRIntervalTime); // Activate causes the given transaction to be processed on this // connection. It fails if there is already an existing transaction unless @@ -151,8 +150,7 @@ public: int64_t BytesWritten() { return mTotalBytesWritten; } - void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks, - nsIEventTarget* aCallbackTarget); + void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks); void PrintDiagnostics(nsCString &log); private: @@ -201,7 +199,6 @@ private: mozilla::Mutex mCallbacksLock; nsCOMPtr mCallbacks; - nsCOMPtr mCallbackTarget; nsRefPtr mConnInfo; diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.cpp b/netwerk/protocol/http/nsHttpConnectionMgr.cpp index 9a8ed61a3e0..7f4848e5349 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp +++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp @@ -313,15 +313,19 @@ nsHttpConnectionMgr::ClosePersistentConnections() nsresult nsHttpConnectionMgr::SpeculativeConnect(nsHttpConnectionInfo *ci, - nsIInterfaceRequestor *callbacks, - nsIEventTarget *target) + nsIInterfaceRequestor *callbacks) { LOG(("nsHttpConnectionMgr::SpeculativeConnect [ci=%s]\n", ci->HashKey().get())); + // Wrap up the callbacks and the target to ensure they're released on the target + // thread properly. + nsCOMPtr wrappedCallbacks; + NS_NewInterfaceRequestorAggregation(callbacks, nullptr, getter_AddRefs(wrappedCallbacks)); + uint8_t caps = ci->GetAnonymous() ? NS_HTTP_LOAD_ANONYMOUS : 0; nsRefPtr trans = - new NullHttpTransaction(ci, callbacks, target, caps); + new NullHttpTransaction(ci, wrappedCallbacks, caps); nsresult rv = PostEvent(&nsHttpConnectionMgr::OnMsgSpeculativeConnect, 0, trans); @@ -2652,15 +2656,13 @@ nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream *out) "Created new nshttpconnection %p\n", conn.get())); nsCOMPtr callbacks; - nsCOMPtr callbackTarget; - mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks), - getter_AddRefs(callbackTarget)); + mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks)); if (out == mStreamOut) { TimeDuration rtt = TimeStamp::Now() - mPrimarySynStarted; rv = conn->Init(mEnt->mConnInfo, gHttpHandler->ConnMgr()->mMaxRequestDelay, mSocketTransport, mStreamIn, mStreamOut, - callbacks, callbackTarget, + callbacks, PR_MillisecondsToInterval(rtt.ToMilliseconds())); // The nsHttpConnection object now owns these streams and sockets @@ -2673,7 +2675,7 @@ nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream *out) rv = conn->Init(mEnt->mConnInfo, gHttpHandler->ConnMgr()->mMaxRequestDelay, mBackupTransport, mBackupStreamIn, mBackupStreamOut, - callbacks, callbackTarget, + callbacks, PR_MillisecondsToInterval(rtt.ToMilliseconds())); // The nsHttpConnection object now owns these streams and sockets @@ -2730,7 +2732,7 @@ nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream *out) "be used to finish SSL handshake on conn %p\n", conn.get())); nsRefPtr trans = new NullHttpTransaction(mEnt->mConnInfo, - callbacks, callbackTarget, + callbacks, mCaps & ~NS_HTTP_ALLOW_PIPELINING); gHttpHandler->ConnMgr()->AddActiveConn(conn, mEnt); @@ -2836,7 +2838,7 @@ nsHttpConnectionMgr::nsHalfOpenSocket::GetInterface(const nsIID &iid, { if (mTransaction) { nsCOMPtr callbacks; - mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks), nullptr); + mTransaction->GetSecurityCallbacks(getter_AddRefs(callbacks)); if (callbacks) return callbacks->GetInterface(iid, result); } diff --git a/netwerk/protocol/http/nsHttpConnectionMgr.h b/netwerk/protocol/http/nsHttpConnectionMgr.h index 3d1ad22c133..9ff854af58e 100644 --- a/netwerk/protocol/http/nsHttpConnectionMgr.h +++ b/netwerk/protocol/http/nsHttpConnectionMgr.h @@ -105,8 +105,7 @@ public: // connection manager, nor is the submitter obligated to actually submit a // real transaction for this connectionInfo. nsresult SpeculativeConnect(nsHttpConnectionInfo *, - nsIInterfaceRequestor *, - nsIEventTarget *); + nsIInterfaceRequestor *); // called when a connection is done processing a transaction. if the // connection can be reused then it will be added to the idle list, else diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 2757ae9744c..babff35f94b 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -1613,8 +1613,7 @@ nsHttpHandler::Observe(nsISupports *subject, NS_IMETHODIMP nsHttpHandler::SpeculativeConnect(nsIURI *aURI, - nsIInterfaceRequestor *aCallbacks, - nsIEventTarget *aTarget) + nsIInterfaceRequestor *aCallbacks) { nsIStrictTransportSecurityService* stss = gHttpHandler->GetSTSService(); bool isStsHost = false; @@ -1665,7 +1664,7 @@ nsHttpHandler::SpeculativeConnect(nsIURI *aURI, nsHttpConnectionInfo *ci = new nsHttpConnectionInfo(host, port, nullptr, usingSSL); - return SpeculativeConnect(ci, aCallbacks, aTarget); + return SpeculativeConnect(ci, aCallbacks); } //----------------------------------------------------------------------------- diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index ed8be6702b4..d34ad3a2fe9 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -160,10 +160,9 @@ public: } nsresult SpeculativeConnect(nsHttpConnectionInfo *ci, - nsIInterfaceRequestor *callbacks, - nsIEventTarget *target) + nsIInterfaceRequestor *callbacks) { - return mConnMgr->SpeculativeConnect(ci, callbacks, target); + return mConnMgr->SpeculativeConnect(ci, callbacks); } // diff --git a/netwerk/protocol/http/nsHttpPipeline.cpp b/netwerk/protocol/http/nsHttpPipeline.cpp index 7ea108e7885..8a5a4b73bdf 100644 --- a/netwerk/protocol/http/nsHttpPipeline.cpp +++ b/netwerk/protocol/http/nsHttpPipeline.cpp @@ -422,8 +422,7 @@ nsHttpPipeline::Connection() } void -nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result, - nsIEventTarget **target) +nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result) { NS_ASSERTION(PR_GetCurrentThread() == gSocketThread, "wrong thread"); @@ -435,11 +434,9 @@ nsHttpPipeline::GetSecurityCallbacks(nsIInterfaceRequestor **result, if (!trans) trans = Response(0); if (trans) - trans->GetSecurityCallbacks(result, target); + trans->GetSecurityCallbacks(result); else { *result = nullptr; - if (target) - *target = nullptr; } } diff --git a/netwerk/protocol/http/nsHttpTransaction.cpp b/netwerk/protocol/http/nsHttpTransaction.cpp index e63e904da21..af67d1f3c55 100644 --- a/netwerk/protocol/http/nsHttpTransaction.cpp +++ b/netwerk/protocol/http/nsHttpTransaction.cpp @@ -120,6 +120,9 @@ nsHttpTransaction::~nsHttpTransaction() { LOG(("Destroying nsHttpTransaction @%x\n", this)); + // Force the callbacks to be released right now + mCallbacks = nullptr; + NS_IF_RELEASE(mConnection); NS_IF_RELEASE(mConnInfo); @@ -389,26 +392,21 @@ nsHttpTransaction::SetConnection(nsAHttpConnection *conn) } void -nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb, - nsIEventTarget **target) +nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb) { MutexAutoLock lock(mCallbacksLock); NS_IF_ADDREF(*cb = mCallbacks); - if (target) - NS_IF_ADDREF(*target = mConsumerTarget); } void -nsHttpTransaction::SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks, - nsIEventTarget* aCallbackTarget) +nsHttpTransaction::SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks) { { MutexAutoLock lock(mCallbacksLock); mCallbacks = aCallbacks; - mConsumerTarget = aCallbackTarget; } if (mConnection) { - mConnection->SetSecurityCallbacks(aCallbacks, mConsumerTarget); + mConnection->SetSecurityCallbacks(aCallbacks); } } diff --git a/netwerk/protocol/http/nsHttpTransaction.h b/netwerk/protocol/http/nsHttpTransaction.h index e68147ed002..09fd8811efb 100644 --- a/netwerk/protocol/http/nsHttpTransaction.h +++ b/netwerk/protocol/http/nsHttpTransaction.h @@ -86,8 +86,7 @@ public: nsIEventTarget *ConsumerTarget() { return mConsumerTarget; } - void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks, - nsIEventTarget* aCallbackTarget); + void SetSecurityCallbacks(nsIInterfaceRequestor* aCallbacks); // Called to take ownership of the response headers; the transaction // will drop any reference to the response headers after this call. diff --git a/netwerk/test/unit/test_speculative_connect.js b/netwerk/test/unit/test_speculative_connect.js index c7fda7b1ad5..364333fa719 100644 --- a/netwerk/test/unit/test_speculative_connect.js +++ b/netwerk/test/unit/test_speculative_connect.js @@ -36,7 +36,7 @@ function run_test() { serv = new TestServer(); URI = ios.newURI("http://localhost:4444/just/a/test", null, null); ios.QueryInterface(Components.interfaces.nsISpeculativeConnect) - .speculativeConnect(URI, null, null); + .speculativeConnect(URI, null); do_test_pending(); } diff --git a/xpcom/base/nsInterfaceRequestorAgg.cpp b/xpcom/base/nsInterfaceRequestorAgg.cpp index 56364d66870..a9fe0177384 100644 --- a/xpcom/base/nsInterfaceRequestorAgg.cpp +++ b/xpcom/base/nsInterfaceRequestorAgg.cpp @@ -5,6 +5,8 @@ #include "nsInterfaceRequestorAgg.h" #include "nsCOMPtr.h" #include "mozilla/Attributes.h" +#include "nsThreadUtils.h" +#include "nsProxyRelease.h" class nsInterfaceRequestorAgg MOZ_FINAL : public nsIInterfaceRequestor { @@ -13,11 +15,21 @@ public: NS_DECL_NSIINTERFACEREQUESTOR nsInterfaceRequestorAgg(nsIInterfaceRequestor *aFirst, - nsIInterfaceRequestor *aSecond) + nsIInterfaceRequestor *aSecond, + nsIEventTarget *aConsumerTarget = nullptr) : mFirst(aFirst) - , mSecond(aSecond) {} + , mSecond(aSecond) + , mConsumerTarget(aConsumerTarget) + { + if (!mConsumerTarget) { + mConsumerTarget = NS_GetCurrentThread(); + } + } + ~nsInterfaceRequestorAgg(); +private: nsCOMPtr mFirst, mSecond; + nsCOMPtr mConsumerTarget; }; // XXX This needs to support threadsafe refcounting until we fix bug 243591. @@ -34,6 +46,20 @@ nsInterfaceRequestorAgg::GetInterface(const nsIID &aIID, void **aResult) return rv; } +nsInterfaceRequestorAgg::~nsInterfaceRequestorAgg() +{ + nsIInterfaceRequestor* iir = nullptr; + mFirst.swap(iir); + if (iir) { + NS_ProxyRelease(mConsumerTarget, iir); + } + iir = nullptr; + mSecond.swap(iir); + if (iir) { + NS_ProxyRelease(mConsumerTarget, iir); + } +} + nsresult NS_NewInterfaceRequestorAggregation(nsIInterfaceRequestor *aFirst, nsIInterfaceRequestor *aSecond, @@ -45,3 +71,16 @@ NS_NewInterfaceRequestorAggregation(nsIInterfaceRequestor *aFirst, NS_ADDREF(*aResult); return NS_OK; } + +nsresult +NS_NewInterfaceRequestorAggregation(nsIInterfaceRequestor *aFirst, + nsIInterfaceRequestor *aSecond, + nsIEventTarget* aTarget, + nsIInterfaceRequestor **aResult) +{ + *aResult = new nsInterfaceRequestorAgg(aFirst, aSecond, aTarget); + if (!*aResult) + return NS_ERROR_OUT_OF_MEMORY; + NS_ADDREF(*aResult); + return NS_OK; +} diff --git a/xpcom/base/nsInterfaceRequestorAgg.h b/xpcom/base/nsInterfaceRequestorAgg.h index 11382c5f71d..2dc0e30a037 100644 --- a/xpcom/base/nsInterfaceRequestorAgg.h +++ b/xpcom/base/nsInterfaceRequestorAgg.h @@ -7,16 +7,29 @@ #include "nsIInterfaceRequestor.h" +class nsIEventTarget; + /** * This function returns an instance of nsIInterfaceRequestor that aggregates * two nsIInterfaceRequestor instances. Its GetInterface method queries * aFirst for the requested interface and will query aSecond only if aFirst * failed to supply the requested interface. Both aFirst and aSecond may - * be null. + * be null, and will be released on the main thread when the aggregator is + * destroyed. */ extern nsresult NS_NewInterfaceRequestorAggregation(nsIInterfaceRequestor *aFirst, nsIInterfaceRequestor *aSecond, nsIInterfaceRequestor **aResult); +/** + * Like the previous method, but aFirst and aSecond will be released on the + * provided target thread. + */ +extern nsresult +NS_NewInterfaceRequestorAggregation(nsIInterfaceRequestor *aFirst, + nsIInterfaceRequestor *aSecond, + nsIEventTarget *aTarget, + nsIInterfaceRequestor **aResult); + #endif // !defined( nsInterfaceRequestorAgg_h__ )