bug 1463356 do not count "not started" TRR resolves as failures r?mcmanus
... when comparing against the native resolver. DNS_TRR_COMPARE is meant
to compare how the actually performed name resolves fare against each
other.
MozReview-Commit-ID: 98NoUGPpHr6
--- a/netwerk/dns/TRR.cpp
+++ b/netwerk/dns/TRR.cpp
@@ -117,17 +117,17 @@ TRR::DohEncode(nsCString &aBody)
return NS_OK;
}
NS_IMETHODIMP
TRR::Run()
{
MOZ_ASSERT(NS_IsMainThread());
if ((gTRRService == nullptr) || NS_FAILED(SendHTTPRequest())) {
- FailData();
+ FailData(NS_ERROR_FAILURE);
// The dtor will now be run
}
return NS_OK;
}
nsresult
TRR::SendHTTPRequest()
{
@@ -845,26 +845,26 @@ TRR::ReturnData()
}
(void)mHostResolver->CompleteLookup(mRec, NS_OK, ai.forget(), mPB);
mHostResolver = nullptr;
mRec = nullptr;
return NS_OK;
}
nsresult
-TRR::FailData()
+TRR::FailData(nsresult error)
{
if (!mHostResolver) {
return NS_ERROR_FAILURE;
}
// create and populate an TRR AddrInfo instance to pass on to signal that
// this comes from TRR
AddrInfo *ai = new AddrInfo(mHost, mType);
- (void)mHostResolver->CompleteLookup(mRec, NS_ERROR_FAILURE, ai, mPB);
+ (void)mHostResolver->CompleteLookup(mRec, error, ai, mPB);
mHostResolver = nullptr;
mRec = nullptr;
return NS_OK;
}
nsresult
TRR::On200Response()
{
@@ -928,17 +928,17 @@ TRR::OnStopRequest(nsIRequest *aRequest,
nsresult rv = NS_OK;
nsAutoCString contentType;
httpChannel->GetContentType(contentType);
if (contentType.Length() &&
!contentType.LowerCaseEqualsLiteral("application/dns-udpwireformat")) {
// try and parse missing content-types, but otherwise require udpwireformat
LOG(("TRR:OnStopRequest %p %s %d should fail due to content type %s\n",
this, mHost.get(), mType, contentType.get()));
- FailData();
+ FailData(NS_ERROR_UNEXPECTED);
return NS_OK;
}
uint32_t httpStatus;
rv = httpChannel->GetResponseStatus(&httpStatus);
if (NS_SUCCEEDED(rv) && httpStatus == 200) {
rv = On200Response();
if (NS_SUCCEEDED(rv)) {
@@ -947,17 +947,17 @@ TRR::OnStopRequest(nsIRequest *aRequest,
} else {
LOG(("TRR:OnStopRequest:%d %p rv %x httpStatus %d\n", __LINE__,
this, (int)rv, httpStatus));
}
}
LOG(("TRR:OnStopRequest %p status %x mFailed %d\n",
this, (int)aStatusCode, mFailed));
- FailData();
+ FailData(NS_ERROR_UNKNOWN_HOST);
return NS_OK;
}
NS_IMETHODIMP
TRR::OnDataAvailable(nsIRequest *aRequest,
nsISupports *aContext,
nsIInputStream *aInputStream,
uint64_t aOffset,
--- a/netwerk/dns/TRR.h
+++ b/netwerk/dns/TRR.h
@@ -142,17 +142,24 @@ public:
private:
~TRR() = default;
nsresult SendHTTPRequest();
nsresult DohEncode(nsCString &target);
nsresult PassQName(unsigned int &index);
nsresult GetQname(nsAutoCString &aQname, unsigned int &aIndex);
nsresult DohDecode(nsCString &aHost);
nsresult ReturnData();
- nsresult FailData();
+
+ // FailData() must be called to signal that the asynch TRR resolve is
+ // completed. For failed name resolves ("no such host"), the 'error' it
+ // passses on in its argument must be NS_ERROR_UNKNOWN_HOST. Other errors
+ // (if host was blacklisted, there as a bad content-type received, etc)
+ // other error codes must be used. This distinction is important for the
+ // subsequent logic to separate the error reasons.
+ nsresult FailData(nsresult error);
nsresult DohDecodeQuery(const nsCString &query,
nsCString &host, enum TrrType &type);
nsresult ReceivePush(nsIHttpChannel *pushed, nsHostRecord *pushedRec);
nsresult On200Response();
nsCOMPtr<nsIChannel> mChannel;
enum TrrType mType;
TimeStamp mStartTime;
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -179,16 +179,17 @@ nsHostKey::SizeOfExcludingThis(mozilla::
nsHostRecord::nsHostRecord(const nsHostKey& key)
: nsHostKey(key)
, addr_info_lock("nsHostRecord.addr_info_lock")
, addr_info_gencnt(0)
, addr_info(nullptr)
, addr(nullptr)
, negative(false)
, mResolverMode(MODE_NATIVEONLY)
+ , mFirstTRRresult(NS_OK)
, mResolving(0)
, mTRRSuccess(0)
, mNativeSuccess(0)
, mNative(false)
, mTRRUsed(false)
, mNativeUsed(false)
, onQueue(false)
, usingAnyThread(false)
@@ -1511,16 +1512,17 @@ nsHostResolver::CompleteLookup(nsHostRec
} else {
MOZ_ASSERT(0);
}
if (NS_SUCCEEDED(status)) {
rec->mTRRSuccess++;
}
if (TRROutstanding()) {
+ rec->mFirstTRRresult = status;
if (NS_FAILED(status)) {
return LOOKUP_OK; // wait for outstanding
}
// There's another TRR complete pending. Wait for it and keep
// this RRset around until then.
MOZ_ASSERT(!rec->mFirstTRR && newRRSet);
rec->mFirstTRR = newRRSet; // autoPtr.swap()
@@ -1550,28 +1552,39 @@ nsHostResolver::CompleteLookup(nsHostRec
merge_rrset(newRRSet, rec->mFirstTRR);
}
else {
newRRSet = rec->mFirstTRR; // transfers
}
rec->mFirstTRR = nullptr;
}
+ if (NS_FAILED(rec->mFirstTRRresult) &&
+ NS_FAILED(status) &&
+ (rec->mFirstTRRresult != NS_ERROR_UNKNOWN_HOST) &&
+ (status != NS_ERROR_UNKNOWN_HOST)) {
+ // the errors are not failed resolves, that means
+ // something else failed, consider this as *TRR not used*
+ // for actually trying to resolve the host
+ rec->mTRRUsed = false;
+ }
+
if (!rec->mTRRSuccess) {
// no TRR success
newRRSet = nullptr;
status = NS_ERROR_UNKNOWN_HOST;
}
if (!rec->mTRRSuccess && rec->mResolverMode == MODE_TRRFIRST) {
MOZ_ASSERT(!rec->mResolving);
NativeLookup(rec);
MOZ_ASSERT(rec->mResolving);
return LOOKUP_OK;
}
+
// continue
}
if (NS_SUCCEEDED(status) && (rec->mTRRSuccess == 1)) {
// store the duration on first (used) TRR response
rec->mTrrDuration = TimeStamp::Now() - rec->mTrrStart;
}
@@ -1795,17 +1808,17 @@ nsHostResolver::ThreadFunc(void *arg)
}
// GetHostToLookup() returns an owning reference
MOZ_ASSERT(tmpRec);
rec.swap(tmpRec);
}
LOG(("DNS lookup thread - Calling getaddrinfo for host [%s].\n",
rec->host.get()));
-
+
TimeStamp startTime = TimeStamp::Now();
bool getTtl = rec->mGetTtl;
nsresult status = GetAddrInfo(rec->host, rec->af,
rec->flags, &ai,
getTtl);
#if defined(RES_RETRY_ON_FAILURE)
if (NS_FAILED(status) && rs.Reset()) {
status = GetAddrInfo(rec->host, rec->af,
--- a/netwerk/dns/nsHostResolver.h
+++ b/netwerk/dns/nsHostResolver.h
@@ -169,16 +169,17 @@ public:
mozilla::net::ResolverMode mResolverMode;
private:
friend class nsHostResolver;
explicit nsHostRecord(const nsHostKey& key);
mozilla::LinkedList<RefPtr<nsResolveHostCallback>> mCallbacks;
nsAutoPtr<mozilla::net::AddrInfo> mFirstTRR; // partial TRR storage
+ nsresult mFirstTRRresult;
uint16_t mResolving; // counter of outstanding resolving calls
uint8_t mTRRSuccess; // number of successful TRR responses
uint8_t mNativeSuccess; // number of native lookup responses
uint16_t mNative : 1; // true if this record is being resolved "natively",
// which means that it is either on the pending queue
// or owned by one of the worker threads. */