bug 1450630 - use DataStorage in main thread only r?mcmanus draft
authorDaniel Stenberg <daniel@haxx.se>
Mon, 30 Apr 2018 09:08:07 +0200
changeset 789697 ce7c28d9552e22bcf410bee2774821805314c985
parent 789560 c552490c8659bf2e7d279ed28d46fb5d5a245a96
push id108310
push userbmo:daniel@haxx.se
push dateMon, 30 Apr 2018 08:08:59 +0000
reviewersmcmanus
bugs1450630
milestone61.0a1
bug 1450630 - use DataStorage in main thread only r?mcmanus Move the TRR blacklist check to the main thread, and since it is now done a little later and for each separate request, make sure to only do the telemetry counting for one of the record types (A) so that we don't count them twice. MozReview-Commit-ID: BgvU4TzrpCq
netwerk/dns/TRR.cpp
netwerk/dns/TRRService.cpp
netwerk/dns/nsHostResolver.cpp
--- a/netwerk/dns/TRR.cpp
+++ b/netwerk/dns/TRR.cpp
@@ -135,16 +135,32 @@ TRR::SendHTTPRequest()
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
 
   if ((mType != TRRTYPE_A) && (mType != TRRTYPE_AAAA) && (mType != TRRTYPE_NS)) {
     // limit the calling interface because nsHostResolver has explicit slots for
     // these types
     return NS_ERROR_FAILURE;
   }
 
+  if ((mType == TRRTYPE_A) || (mType == TRRTYPE_AAAA)) {
+    // let NS resolves skip the blacklist check
+    if (gTRRService->IsTRRBlacklisted(mHost, mPB, true)) {
+      if (mType == TRRTYPE_A) {
+        // count only blacklist for A records to avoid double counts
+        Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED, true);
+      }
+      // not really an error but no TRR is issued
+      return NS_ERROR_UNKNOWN_HOST;
+    } else {
+      if (mType == TRRTYPE_A) {
+        Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED, false);
+      }
+    }
+  }
+
   nsresult rv;
   nsCOMPtr<nsIIOService> ios(do_GetIOService(&rv));
   NS_ENSURE_SUCCESS(rv, rv);
 
   bool useGet = gTRRService->UseGET();
   nsAutoCString body;
   nsCOMPtr<nsIURI> dnsURI;
 
--- a/netwerk/dns/TRRService.cpp
+++ b/netwerk/dns/TRRService.cpp
@@ -379,23 +379,16 @@ TRRService::MaybeBootstrap(const nsACStr
 }
 
 // When running in TRR-only mode, the blacklist is not used and it will also
 // try resolving the localhost / .local names.
 bool
 TRRService::IsTRRBlacklisted(const nsACString &aHost, bool privateBrowsing,
                              bool aParentsToo) // false if domain
 {
-  if (mClearTRRBLStorage) {
-    if (mTRRBLStorage) {
-      mTRRBLStorage->Clear();
-    }
-    mClearTRRBLStorage = false;
-  }
-
   if (mMode == MODE_TRRONLY) {
     return false; // might as well try
   }
 
   // hardcode these so as to not worry about expiration
   if (StringEndsWith(aHost, NS_LITERAL_CSTRING(".local")) ||
       aHost.Equals(NS_LITERAL_CSTRING("localhost"))) {
     return true;
@@ -403,16 +396,25 @@ TRRService::IsTRRBlacklisted(const nsACS
 
   if (!Enabled()) {
     return true;
   }
   if (!mTRRBLStorage) {
     return false;
   }
 
+  // Only use the Storage API in the main thread
+  MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
+
+  if (mClearTRRBLStorage) {
+    mTRRBLStorage->Clear();
+    mClearTRRBLStorage = false;
+    return false; // just cleared!
+  }
+
   int32_t dot = aHost.FindChar('.');
   if ((dot == kNotFound) && aParentsToo) {
     // Only if a full host name. Domains can be dotless to be able to
     // blacklist entire TLDs
     return true;
   } else if(dot != kNotFound) {
     // there was a dot, check the parent first
     dot++;
--- a/netwerk/dns/nsHostResolver.cpp
+++ b/netwerk/dns/nsHostResolver.cpp
@@ -1109,25 +1109,16 @@ nsHostResolver::TrrLookup(nsHostRecord *
 
         rec->remove();
         mEvictionQSize--;
     }
 
     rec->mTRRSuccess = 0; // bump for each successful TRR response
     rec->mTrrAUsed = nsHostRecord::INIT;
     rec->mTrrAAAAUsed = nsHostRecord::INIT;
-
-    if (gTRRService && gTRRService->IsTRRBlacklisted(rec->host, rec->pb, true)) {
-        Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED, true);
-        MOZ_ASSERT(!rec->mTRRUsed);
-        // not really an error but no TRR is issued
-        return NS_ERROR_UNKNOWN_HOST;
-    }
-    Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED, false);
-
     rec->mTrrStart = TimeStamp::Now();
     rec->mTRRUsed = true; // this record gets TRR treatment
 
     // If asking for AF_UNSPEC, issue both A and AAAA.
     // If asking for AF_INET6 or AF_INET, do only that single type
     enum TrrType rectype = (rec->af == AF_INET6)? TRRTYPE_AAAA : TRRTYPE_A;
     if (pushedTRR) {
         rectype = pushedTRR->Type();