Bug 1525208 - Part 3: Cache the result of calling PR_StringToNetAddr() since it may be expensive; r=baku

Depends on D18644

Differential Revision: https://phabricator.services.mozilla.com/D18645

--HG--
extra : moz-landing-system : lando

netwerk/dns/nsEffectiveTLDService.cpp

@@ -189,17 +189,16 @@ nsresult nsEffectiveTLDService::GetBaseDomainInternal(nsCString &aHostname,
   if (aHostname.IsEmpty() || aHostname.Last() == '.')
     return NS_ERROR_INVALID_ARG;
 
-  // Check if we're dealing with an IPv4/IPv6 hostname, and return
-  PRNetAddr addr;
-  PRStatus result = PR_StringToNetAddr(aHostname.get(), &addr);
-  if (result == PR_SUCCESS) return NS_ERROR_HOST_IS_IP_ADDRESS;
-
   // Lookup in the cache if this is a normal query. This is restricted to
   // main thread-only as the cache is not thread-safe.
   Maybe<TldCache::Entry> entry;
   if (aAdditionalParts == 1 && NS_IsMainThread()) {
     auto p = mMruTable.Lookup(aHostname);
     if (p) {
+      if (NS_FAILED(p.Data().mResult)) {
+        return p.Data().mResult;
+      }
+
       // There was a match, just return the cached value.
       aBaseDomain = p.Data().mBaseDomain;
       if (trailingDot) {
@@ -212,6 +211,19 @@ nsresult nsEffectiveTLDService::GetBaseDomainInternal(nsCString &aHostname,
     entry = Some(p);
   }
 
+  // Check if we're dealing with an IPv4/IPv6 hostname, and return
+  PRNetAddr addr;
+  PRStatus result = PR_StringToNetAddr(aHostname.get(), &addr);
+  if (result == PR_SUCCESS) {
+    // Update the MRU table if in use.
+    if (entry) {
+      entry->Set(TLDCacheEntry{aHostname, EmptyCString(),
+                               NS_ERROR_HOST_IS_IP_ADDRESS});
+    }
+
+    return NS_ERROR_HOST_IS_IP_ADDRESS;
+  }
+
   // Walk up the domain tree, most specific to least specific,
   // looking for matches at each level.  Note that a given level may
   // have multiple attributes (e.g. IsWild() and IsNormal()).
@@ -225,7 +237,15 @@ nsresult nsEffectiveTLDService::GetBaseDomainInternal(nsCString &aHostname,
     // sanity check the string we're about to look up: it should not begin with
     // a '.'; this would mean the hostname began with a '.' or had an
     // embedded '..' sequence.
-    if (*currDomain == '.') return NS_ERROR_INVALID_ARG;
+    if (*currDomain == '.') {
+      // Update the MRU table if in use.
+      if (entry) {
+        entry->Set(
+            TLDCacheEntry{aHostname, EmptyCString(), NS_ERROR_INVALID_ARG});
+      }
+
+      return NS_ERROR_INVALID_ARG;
+    }
 
     // Perform the lookup.
     const int result = mGraph.Lookup(Substring(currDomain, end));
@@ -287,13 +307,21 @@ nsresult nsEffectiveTLDService::GetBaseDomainInternal(nsCString &aHostname,
     }
   }
 
-  if (aAdditionalParts != 0) return NS_ERROR_INSUFFICIENT_DOMAIN_LEVELS;
+  if (aAdditionalParts != 0) {
+    // Update the MRU table if in use.
+    if (entry) {
+      entry->Set(TLDCacheEntry{aHostname, EmptyCString(),
+                               NS_ERROR_INSUFFICIENT_DOMAIN_LEVELS});
+    }
+
+    return NS_ERROR_INSUFFICIENT_DOMAIN_LEVELS;
+  }
 
   aBaseDomain = Substring(iter, end);
 
   // Update the MRU table if in use.
   if (entry) {
-    entry->Set(TLDCacheEntry{aHostname, nsCString(aBaseDomain)});
+    entry->Set(TLDCacheEntry{aHostname, nsCString(aBaseDomain), NS_OK});
   }
 
   // add on the trailing dot, if applicable

netwerk/dns/nsEffectiveTLDService.h

@@ -42,9 +42,14 @@ class nsEffectiveTLDService final : public nsIEffectiveTLDService,
   // The DAFSA provides a compact encoding of the rather large eTLD list.
   mozilla::Dafsa mGraph;
 
+  // Note that the cache entries here can record entries that were cached
+  // successfully or unsuccessfully.  mResult must be checked before using an
+  // entry.  If it's a success error code, the cache entry is valid and can be
+  // used.
   struct TLDCacheEntry {
     nsCString mHost;
     nsCString mBaseDomain;
+    nsresult mResult;
   };
 
   // We use a small most recently used cache to compensate for DAFSA lookups