Bug 1367861 - enable throttling for http/2. r?mayhemer
This also fixes the bug that prevented throttled http/2 streams from
ever re-starting by calling TransactionHasDataToRecv.
MozReview-Commit-ID: 5dFotZGhQk9
--- a/netwerk/protocol/http/Http2Session.cpp
+++ b/netwerk/protocol/http/Http2Session.cpp
@@ -440,17 +440,17 @@ Http2Session::AddStream(nsAHttpTransacti
"transaction (%08x).\n", this, aHttpTransaction, trans,
static_cast<uint32_t>(rv)));
}
return true;
}
}
aHttpTransaction->SetConnection(this);
- aHttpTransaction->OnActivated(true);
+ aHttpTransaction->OnActivated();
if (aUseTunnel) {
LOG3(("Http2Session::AddStream session=%p trans=%p OnTunnel",
this, aHttpTransaction));
DispatchOnTunnel(aHttpTransaction, aCallbacks);
return true;
}
--- a/netwerk/protocol/http/nsAHttpTransaction.h
+++ b/netwerk/protocol/http/nsAHttpTransaction.h
@@ -42,17 +42,17 @@ class nsAHttpTransaction : public nsSupp
public:
NS_DECLARE_STATIC_IID_ACCESSOR(NS_AHTTPTRANSACTION_IID)
// called by the connection when it takes ownership of the transaction.
virtual void SetConnection(nsAHttpConnection *) = 0;
// called by the connection after a successfull activation of this transaction
// in other words, tells the transaction it transitioned to the "active" state.
- virtual void OnActivated(bool h2) {}
+ virtual void OnActivated() {}
// used to obtain the connection associated with this transaction
virtual nsAHttpConnection *Connection() = 0;
// called by the connection to get security callbacks to set on the
// socket transport.
virtual void GetSecurityCallbacks(nsIInterfaceRequestor **) = 0;
--- a/netwerk/protocol/http/nsHttpConnection.cpp
+++ b/netwerk/protocol/http/nsHttpConnection.cpp
@@ -725,17 +725,17 @@ nsHttpConnection::Activate(nsAHttpTransa
}
if (mTLSFilter) {
rv = mTLSFilter->SetProxiedTransaction(trans);
NS_ENSURE_SUCCESS(rv, rv);
mTransaction = mTLSFilter;
}
- trans->OnActivated(false);
+ trans->OnActivated();
rv = OnOutputStreamReady(mSocketOut);
failed_activation:
if (NS_FAILED(rv)) {
mTransaction = nullptr;
}
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -106,17 +106,16 @@ nsHttpTransaction::nsHttpTransaction()
, mCurrentHttpResponseHeaderSize(0)
, mThrottlingReadAllowance(THROTTLE_NO_LIMIT)
, mCapsToClear(0)
, mResponseIsComplete(false)
, mReadingStopped(false)
, mClosed(false)
, mConnected(false)
, mActivated(false)
- , mActivatedAsH2(false)
, mHaveStatusLine(false)
, mHaveAllHeaders(false)
, mTransactionDone(false)
, mDidContentStart(false)
, mNoContent(false)
, mSentData(false)
, mReceivedData(false)
, mStatusEventPending(false)
@@ -170,16 +169,17 @@ void nsHttpTransaction::ResumeReading()
mReadingStopped = false;
// This with either reengage the limit when still throttled in WriteSegments or
// simply reset to allow unlimeted reading again.
mThrottlingReadAllowance = THROTTLE_NO_LIMIT;
if (mConnection) {
+ mConnection->TransactionHasDataToRecv(this);
nsresult rv = mConnection->ResumeRecv();
if (NS_FAILED(rv)) {
LOG((" resume failed with rv=%" PRIx32, static_cast<uint32_t>(rv)));
}
}
}
bool nsHttpTransaction::EligibleForThrottling() const
@@ -534,21 +534,20 @@ nsHttpTransaction::SetConnection(nsAHttp
{
{
MutexAutoLock lock(mLock);
mConnection = conn;
}
}
void
-nsHttpTransaction::OnActivated(bool h2)
+nsHttpTransaction::OnActivated()
{
MOZ_ASSERT(OnSocketThread());
- mActivatedAsH2 = h2;
if (mActivated) {
return;
}
mActivated = true;
gHttpHandler->ConnMgr()->AddActiveTransaction(this);
}
@@ -870,27 +869,16 @@ nsHttpTransaction::WritePipeSegment(nsIO
if (NS_FAILED(rv))
trans->Close(rv);
return rv; // failure code only stops WriteSegments; it is not propagated.
}
bool nsHttpTransaction::ShouldThrottle()
{
- if (mActivatedAsH2) {
- // Throttling feature is now disabled for http/2 transactions
- // because of bug 1367861. The logic around mActivatedAsH2
- // will be removed when that is fixed.
- //
- // Calling ShouldThrottle on the manager just to make sure
- // the throttling time window is correctly updated by this transaction.
- Unused << gHttpHandler->ConnMgr()->ShouldThrottle(this);
- return false;
- }
-
if (mClassOfService & nsIClassOfService::DontThrottle) {
// We deliberately don't touch the throttling window here since
// DontThrottle requests are expected to be long-standing media
// streams and would just unnecessarily block running downloads.
// If we want to ballance bandwidth for media responses against
// running downloads, we need to find something smarter like
// changing the suspend/resume throttling intervals at-runtime.
return false;
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -85,17 +85,17 @@ public:
uint64_t reqContentLength,
bool reqBodyIncludesHeaders,
nsIEventTarget *consumerTarget,
nsIInterfaceRequestor *callbacks,
nsITransportEventSink *eventsink,
uint64_t topLevelOuterContentWindowId,
nsIAsyncInputStream **responseBody);
- void OnActivated(bool h2) override;
+ void OnActivated() override;
// attributes
nsHttpResponseHead *ResponseHead() { return mHaveAllHeaders ? mResponseHead : nullptr; }
nsISupports *SecurityInfo() { return mSecurityInfo; }
nsIEventTarget *ConsumerTarget() { return mConsumerTarget; }
nsISupports *HttpChannel() { return mChannel; }
@@ -344,17 +344,16 @@ private:
// to resume reading.
bool mReadingStopped;
// state flags, all logically boolean, but not packed together into a
// bitfield so as to avoid bitfield-induced races. See bug 560579.
bool mClosed;
bool mConnected;
bool mActivated;
- bool mActivatedAsH2;
bool mHaveStatusLine;
bool mHaveAllHeaders;
bool mTransactionDone;
bool mDidContentStart;
bool mNoContent; // expecting an empty entity body
bool mSentData;
bool mReceivedData;
bool mStatusEventPending;