bug 1463356 do not count "not started" TRR resolves as failures r=mcmanus
authorDaniel Stenberg <daniel@haxx.se>
Wed, 20 Jun 2018 11:00:19 +0200
changeset 479857 08b8d3da8635e10564150b04742b787d74d372e6
parent 479856 ddde6ad6d9e6c4069632e195613bcd53a39883e4
child 479858 88a05771dd2bfbd22e31a543c7403414886e1523
push id1757
push userffxbld-merge
push dateFri, 24 Aug 2018 17:02:43 +0000
treeherdermozilla-release@736023aebdb1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs1463356
milestone62.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 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
netwerk/dns/TRR.cpp
netwerk/dns/TRR.h
netwerk/dns/nsHostResolver.cpp
netwerk/dns/nsHostResolver.h
--- 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)
@@ -1514,16 +1515,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()
@@ -1553,28 +1555,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;
         }
 
--- 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. */