Bug 1387090 - Properly handle DontThrottle flag in all places in nsHttpTransaction. r=mcmanus
--- a/netwerk/protocol/http/nsHttpConnectionMgr.cpp
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ -2956,85 +2956,88 @@ void nsHttpConnectionMgr::LogActiveTrans
// - unthrottled transaction for the active tab
// - throttled transaction for the active tab
// - unthrottled transaction for background tabs
// - throttled transaction for background tabs
LOG(("Active transactions %c[%u,%u,%u,%u]", operation, au, at, bu, bt));
}
void
-nsHttpConnectionMgr::AddActiveTransaction(nsHttpTransaction * aTrans, bool aThrottled)
+nsHttpConnectionMgr::AddActiveTransaction(nsHttpTransaction * aTrans)
{
MOZ_ASSERT(OnSocketThread(), "not on socket thread");
uint64_t tabId = aTrans->TopLevelOuterContentWindowId();
+ bool throttled = aTrans->EligibleForThrottling();
nsTArray<RefPtr<nsHttpTransaction>> *transactions =
- mActiveTransactions[aThrottled].LookupOrAdd(tabId);
+ mActiveTransactions[throttled].LookupOrAdd(tabId);
MOZ_ASSERT(!transactions->Contains(aTrans));
transactions->AppendElement(aTrans);
LOG(("nsHttpConnectionMgr::AddActiveTransaction t=%p tabid=%" PRIx64 "(%d) thr=%d",
- aTrans, tabId, tabId == mCurrentTopLevelOuterContentWindowId, aThrottled));
+ aTrans, tabId, tabId == mCurrentTopLevelOuterContentWindowId, throttled));
LogActiveTransactions('+');
if (tabId == mCurrentTopLevelOuterContentWindowId) {
mActiveTabTransactionsExist = true;
- if (!aThrottled) {
+ if (!throttled) {
mActiveTabUnthrottledTransactionsExist = true;
}
}
if (!mThrottleEnabled) {
return;
}
EnsureThrottleTickerIfNeeded();
}
void
-nsHttpConnectionMgr::RemoveActiveTransaction(nsHttpTransaction * aTrans, bool aThrottled)
+nsHttpConnectionMgr::RemoveActiveTransaction(nsHttpTransaction * aTrans,
+ Maybe<bool> const& aOverride)
{
MOZ_ASSERT(OnSocketThread(), "not on socket thread");
uint64_t tabId = aTrans->TopLevelOuterContentWindowId();
bool forActiveTab = tabId == mCurrentTopLevelOuterContentWindowId;
+ bool throttled = aOverride.valueOr(aTrans->EligibleForThrottling());
nsTArray<RefPtr<nsHttpTransaction>> *transactions =
- mActiveTransactions[aThrottled].Get(tabId);
+ mActiveTransactions[throttled].Get(tabId);
if (!transactions || !transactions->RemoveElement(aTrans)) {
// Was not tracked as active, probably just ignore.
return;
}
LOG(("nsHttpConnectionMgr::RemoveActiveTransaction t=%p tabid=%" PRIx64 "(%d) thr=%d",
- aTrans, tabId, forActiveTab, aThrottled));
+ aTrans, tabId, forActiveTab, throttled));
if (!transactions->IsEmpty()) {
// There are still transactions of the type, hence nothing in the throttling conditions
// has changed and we don't need to update "Exists" caches nor we need to wake any now
// throttled transactions.
LogActiveTransactions('-');
return;
}
// To optimize the following logic, always remove the entry when the array is empty.
- mActiveTransactions[aThrottled].Remove(tabId);
+ mActiveTransactions[throttled].Remove(tabId);
LogActiveTransactions('-');
if (forActiveTab) {
// Update caches of the active tab transaction existence, since it's now affected
- if (!aThrottled) {
+ if (!throttled) {
mActiveTabUnthrottledTransactionsExist = false;
}
if (mActiveTabTransactionsExist) {
- mActiveTabTransactionsExist = mActiveTransactions[!aThrottled].Contains(tabId);
+ mActiveTabTransactionsExist = mActiveTransactions[!throttled].Contains(tabId);
}
}
if (!mThrottleEnabled) {
return;
}
bool unthrottledExist = !mActiveTransactions[false].IsEmpty();
@@ -3062,17 +3065,17 @@ nsHttpConnectionMgr::RemoveActiveTransac
LOG((" there are unthrottled for the active tab"));
return;
}
if (mActiveTabTransactionsExist) {
// There are only trottled transactions for the active tab.
// If the last transaction we just removed was a non-throttled for the active tab
// we can wake the throttled transactions for the active tab.
- if (forActiveTab && !aThrottled) {
+ if (forActiveTab && !throttled) {
LOG((" resuming throttled for active tab"));
ResumeReadOf(mActiveTransactions[true].Get(mCurrentTopLevelOuterContentWindowId));
}
return;
}
if (!unthrottledExist) {
// There are no unthrottled transactions for any tab. Resume all throttled,
@@ -3089,49 +3092,55 @@ nsHttpConnectionMgr::RemoveActiveTransac
DelayedResumeBackgroundThrottledTransactions();
return;
}
LOG((" not resuming anything"));
}
void
-nsHttpConnectionMgr::MoveActiveTransaction(nsHttpTransaction * aTrans, bool aThrottled)
+nsHttpConnectionMgr::UpdateActiveTransaction(nsHttpTransaction * aTrans)
{
- LOG(("nsHttpConnectionMgr::MoveActiveTransaction ENTER t=%p", aTrans));
- AddActiveTransaction(aTrans, aThrottled);
- RemoveActiveTransaction(aTrans, !aThrottled);
- LOG(("nsHttpConnectionMgr::MoveActiveTransaction EXIT t=%p", aTrans));
+ LOG(("nsHttpConnectionMgr::UpdateActiveTransaction ENTER t=%p", aTrans));
+
+ AddActiveTransaction(aTrans);
+
+ Maybe<bool> reversed;
+ reversed.emplace(!aTrans->EligibleForThrottling());
+ RemoveActiveTransaction(aTrans, reversed);
+
+ LOG(("nsHttpConnectionMgr::UpdateActiveTransaction EXIT t=%p", aTrans));
}
bool
-nsHttpConnectionMgr::ShouldStopReading(nsHttpTransaction * aTrans, bool aThrottled)
+nsHttpConnectionMgr::ShouldStopReading(nsHttpTransaction * aTrans)
{
MOZ_ASSERT(OnSocketThread(), "not on socket thread");
if (!mThrottlingInhibitsReading || !mThrottleEnabled) {
return false;
}
uint64_t tabId = aTrans->TopLevelOuterContentWindowId();
bool forActiveTab = tabId == mCurrentTopLevelOuterContentWindowId;
+ bool throttled = aTrans->EligibleForThrottling();
if (mActiveTabTransactionsExist) {
if (!tabId) {
// Chrome initiated and unidentified transactions just respect
// their throttle flag, when something for the active tab is happening.
- return aThrottled;
+ return throttled;
}
if (!forActiveTab) {
// This is a background tab request, we want them to always throttle.
return true;
}
if (mActiveTabUnthrottledTransactionsExist) {
// Unthrottled transactions for the active tab take precedence
- return aThrottled;
+ return throttled;
}
// This is a throttled transaction for the active tab and there are no
// unthrottled for the active tab, just let go on full fuel.
return false;
}
MOZ_ASSERT(!forActiveTab);
@@ -3140,17 +3149,17 @@ nsHttpConnectionMgr::ShouldStopReading(n
// after a delay.
return true;
}
if (!mActiveTransactions[false].IsEmpty()) {
// This means there are unthrottled active transactions for background tabs.
// If we are here, there can't be any transactions for the active tab.
// (If there is no transaction for a tab id, there is no entry for it in the hashtable.)
- return aThrottled;
+ return throttled;
}
// There are only unthrottled transactions for background tabs: don't throttle.
return false;
}
bool nsHttpConnectionMgr::IsConnEntryUnderPressure(nsHttpConnectionInfo *connInfo)
{
--- a/netwerk/protocol/http/nsHttpConnectionMgr.h
+++ b/netwerk/protocol/http/nsHttpConnectionMgr.h
@@ -212,25 +212,26 @@ public:
uint16_t MaxRequestDelay() { return mMaxRequestDelay; }
// public, so that the SPDY/http2 seesions can activate
void ActivateTimeoutTick();
nsresult UpdateCurrentTopLevelOuterContentWindowId(uint64_t aWindowId);
// tracks and untracks active transactions according their throttle status
- void AddActiveTransaction(nsHttpTransaction* aTrans, bool aThrottled);
- void RemoveActiveTransaction(nsHttpTransaction* aTrans, bool aThrottled);
- void MoveActiveTransaction(nsHttpTransaction* aTrans, bool aThrottled);
+ void AddActiveTransaction(nsHttpTransaction* aTrans);
+ void RemoveActiveTransaction(nsHttpTransaction* aTrans,
+ Maybe<bool> const& aOverride = Nothing());
+ void UpdateActiveTransaction(nsHttpTransaction* aTrans);
// called by nsHttpTransaction::WriteSegments. decides whether the transaction
// should stop reading data based on: the throttling ticker status, overall
// status of all active transactions regarding active tab and respective
// throttling state.
- bool ShouldStopReading(nsHttpTransaction* aTrans, bool aThrottled);
+ bool ShouldStopReading(nsHttpTransaction* aTrans);
// return true iff the connection has pending transactions for the active tab.
// it's mainly used to disallow throttling (stop reading) of a response
// belonging to the same conn info to free up a connection ASAP.
// NOTE: relatively expensive to call, there are two hashtable lookups.
bool IsConnEntryUnderPressure(nsHttpConnectionInfo*);
private:
--- a/netwerk/protocol/http/nsHttpTransaction.cpp
+++ b/netwerk/protocol/http/nsHttpTransaction.cpp
@@ -168,36 +168,36 @@ void nsHttpTransaction::ResumeReading()
if (mConnection) {
nsresult rv = mConnection->ResumeRecv();
if (NS_FAILED(rv)) {
LOG((" resume failed with rv=%" PRIx32, static_cast<uint32_t>(rv)));
}
}
}
-bool nsHttpTransaction::EligibleForThrottling()
+bool nsHttpTransaction::EligibleForThrottling() const
{
- return (mClassOfService & (nsIClassOfService::Throttleable |
- nsIClassOfService::DontThrottle |
- nsIClassOfService::Leader |
- nsIClassOfService::Unblocked)) ==
- nsIClassOfService::Throttleable;
+ return (mClassOfService & (nsIClassOfService::Throttleable |
+ nsIClassOfService::DontThrottle |
+ nsIClassOfService::Leader |
+ nsIClassOfService::Unblocked)) ==
+ nsIClassOfService::Throttleable;
}
void nsHttpTransaction::SetClassOfService(uint32_t cos)
{
bool wasThrottling = EligibleForThrottling();
mClassOfService = cos;
bool isThrottling = EligibleForThrottling();
if (mConnection && wasThrottling != isThrottling) {
// Do nothing until we are actually activated. For now
- // only remember the throttle flag. Call to MoveActiveTransaction
+ // only remember the throttle flag. Call to UpdateActiveTransaction
// would add this transaction to the list too early.
- gHttpHandler->ConnMgr()->MoveActiveTransaction(this, isThrottling);
+ gHttpHandler->ConnMgr()->UpdateActiveTransaction(this);
if (mReadingStopped && !isThrottling) {
ResumeReading();
}
}
}
nsHttpTransaction::~nsHttpTransaction()
@@ -521,18 +521,17 @@ nsHttpTransaction::OnActivated(bool h2)
MOZ_ASSERT(OnSocketThread());
mActivatedAsH2 = h2;
if (mActivated) {
return;
}
mActivated = true;
- gHttpHandler->ConnMgr()->AddActiveTransaction(
- this, mClassOfService & nsIClassOfService::Throttleable);
+ gHttpHandler->ConnMgr()->AddActiveTransaction(this);
}
void
nsHttpTransaction::GetSecurityCallbacks(nsIInterfaceRequestor **cb)
{
MutexAutoLock lock(mLock);
nsCOMPtr<nsIInterfaceRequestor> tmp(mCallbacks);
tmp.forget(cb);
@@ -844,17 +843,17 @@ bool nsHttpTransaction::ShouldStopReadin
// will be removed when that is fixed
return false;
}
if (mClassOfService & nsIClassOfService::DontThrottle) {
return false;
}
- if (!gHttpHandler->ConnMgr()->ShouldStopReading(this, EligibleForThrottling())) {
+ if (!gHttpHandler->ConnMgr()->ShouldStopReading(this)) {
// We are not obligated to throttle
return false;
}
if (mContentRead < 16000) {
// Let the first bytes go, it may also well be all the content we get
LOG(("nsHttpTransaction::ShouldStopReading too few content (%" PRIi64 ") this=%p", mContentRead, this));
return false;
@@ -938,18 +937,17 @@ nsHttpTransaction::WriteSegments(nsAHttp
void
nsHttpTransaction::Close(nsresult reason)
{
LOG(("nsHttpTransaction::Close [this=%p reason=%" PRIx32 "]\n",
this, static_cast<uint32_t>(reason)));
if (!mClosed) {
- gHttpHandler->ConnMgr()->RemoveActiveTransaction(
- this, mClassOfService & nsIClassOfService::Throttleable);
+ gHttpHandler->ConnMgr()->RemoveActiveTransaction(this);
mActivated = false;
}
MOZ_ASSERT(OnSocketThread(), "not on socket thread");
if (reason == NS_BINDING_RETARGETED) {
LOG((" close %p skipped due to ERETARGETED\n", this));
return;
}
--- a/netwerk/protocol/http/nsHttpTransaction.h
+++ b/netwerk/protocol/http/nsHttpTransaction.h
@@ -389,18 +389,18 @@ public:
// but later can be dispatched via spdy (not subject to rate pacing).
void CancelPacing(nsresult reason);
// Called by the connetion manager on the socket thread when reading for this
// previously throttled transaction has to be resumed.
void ResumeReading();
// This examins classification of this transaction whether the Throttleable class
- // has been set while Leader or Unblocked are not.
- bool EligibleForThrottling();
+ // has been set while Leader, Unblocked, DontThrottle has not.
+ bool EligibleForThrottling() const;
private:
bool mSubmittedRatePacing;
bool mPassedRatePacing;
bool mSynchronousRatePaceRequest;
nsCOMPtr<nsICancelable> mTokenBucketCancel;
public:
void SetClassOfService(uint32_t cos);