Backed out 2 changesets (bug 1565004) for xpc failures on netwerk/test/unit/test_trr.js.

Backed out changeset 3d6d67621349 (bug 1565004)
Backed out changeset ef016d00ec9b (bug 1565004)

modules/libpref/init/StaticPrefList.yaml

@@ -6827,14 +6827,6 @@
   value: false
   mirror: always
 
-# This pref controls if TRR will still be enabled when a VPN is detected on
-# the system. Detection is not foolproof, and some VPNs may not be detected.
-# See bug 1565004 comment 3 for details.
-- name: network.trr.enable_when_vpn_detected
-  type: RelaxedAtomicBool
-  value: false
-  mirror: always
-
 # Allow the network changed event to get sent when a network topology or setup
 # change is noticed while running.
 - name: network.notify.changed

netwerk/base/nsINetworkLinkService.idl

@@ -56,11 +56,6 @@ interface nsINetworkLinkService : nsISupports
    * The list of DNS suffixes for the currently active network interfaces.
    */
   readonly attribute Array<ACString> dnsSuffixList;
-
-  /**
-   * Whether a VPN was detected on at least one active network interface.
-   */
-  readonly attribute boolean vpnDetected;
 };
 
 %{C++

netwerk/dns/TRRService.cpp

@@ -53,8 +53,7 @@ TRRService::TRRService()
       mConfirmationState(CONFIRM_INIT),
       mRetryConfirmInterval(1000),
       mTRRFailures(0),
-      mParentalControlEnabled(false),
-      mVPNDetected(false) {
+      mParentalControlEnabled(false) {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
 }
 
@@ -452,39 +451,24 @@ TRRService::Observe(nsISupports* aSubject, const char* aTopic,
       mTRRBLStorage->Clear();
     }
   } else if (!strcmp(aTopic, NS_NETWORK_LINK_TOPIC)) {
-    LOG(("TRRService link-event"));
-    nsCOMPtr<nsINetworkLinkService> link = do_QueryInterface(aSubject);
-    RebuildSuffixList(link);
-    CheckVPNStatus(link);
+    mDNSSuffixDomains.Clear();
+    nsAutoCString data = NS_ConvertUTF16toUTF8(aData);
+    if (data.EqualsLiteral(NS_NETWORK_LINK_DATA_CHANGED)) {
+      nsCOMPtr<nsINetworkLinkService> link = do_QueryInterface(aSubject);
+      // The network link service notification normally passes itself as the
+      // subject, but some unit tests will sometimes pass a null subject.
+      if (link) {
+        nsTArray<nsCString> suffixList;
+        link->GetDnsSuffixList(suffixList);
+        for (const auto& item : suffixList) {
+          mDNSSuffixDomains.PutEntry(item);
+        }
+      }
+    }
   }
   return NS_OK;
 }
 
-void TRRService::RebuildSuffixList(nsINetworkLinkService* aLinkService) {
-  mDNSSuffixDomains.Clear();
-  // The network link service notification normally passes itself as the
-  // subject, but some unit tests will sometimes pass a null subject.
-  if (!aLinkService) {
-    return;
-  }
-
-  nsTArray<nsCString> suffixList;
-  aLinkService->GetDnsSuffixList(suffixList);
-  for (const auto& item : suffixList) {
-    LOG(("TRRService adding %s to suffix list", item.get()));
-    mDNSSuffixDomains.PutEntry(item);
-  }
-}
-
-void TRRService::CheckVPNStatus(nsINetworkLinkService* aLinkService) {
-  if (!aLinkService) {
-    return;
-  }
-
-  aLinkService->GetVpnDetected(&mVPNDetected);
-  LOG(("TRRService vpnDetected=%d", mVPNDetected));
-}
-
 void TRRService::MaybeConfirm() {
   MutexAutoLock lock(mLock);
   MaybeConfirm_locked();
@@ -640,11 +624,6 @@ bool TRRService::IsTRRBlacklisted(const nsACString& aHost,
 }
 
 bool TRRService::IsExcludedFromTRR(const nsACString& aHost) {
-  if (mVPNDetected && !StaticPrefs::network_trr_enable_when_vpn_detected()) {
-    LOG(("%s is excluded from TRR because of VPN", aHost.BeginReading()));
-    return true;
-  }
-
   int32_t dot = 0;
   // iteratively check the sub-domain of |aHost|
   while (dot < static_cast<int32_t>(aHost.Length())) {

netwerk/dns/TRRService.h

@@ -14,7 +14,6 @@
 
 class nsDNSService;
 class nsIPrefBranch;
-class nsINetworkLinkService;
 
 namespace mozilla {
 namespace net {
@@ -76,9 +75,6 @@ class TRRService : public nsIObserver,
                            const nsACString& aOriginSuffix,
                            bool aPrivateBrowsing);
 
-  void RebuildSuffixList(nsINetworkLinkService* aLinkService);
-  void CheckVPNStatus(nsINetworkLinkService* aLinkService);
-
   bool mInitialized;
   Atomic<uint32_t, Relaxed> mMode;
   Atomic<uint32_t, Relaxed> mTRRBlacklistExpireTime;
@@ -128,7 +124,6 @@ class TRRService : public nsIObserver,
   uint32_t mRetryConfirmInterval;  // milliseconds until retry
   Atomic<uint32_t, Relaxed> mTRRFailures;
   bool mParentalControlEnabled;
-  bool mVPNDetected;
 };
 
 extern TRRService* gTRRService;

netwerk/system/android/nsAndroidNetworkLinkService.cpp

@@ -152,11 +152,6 @@ nsAndroidNetworkLinkService::GetDnsSuffixList(
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsAndroidNetworkLinkService::GetVpnDetected(bool* aHasVPN) {
-  return NS_ERROR_NOT_IMPLEMENTED;
-}
-
 void nsAndroidNetworkLinkService::OnNetworkChanged() {
   if (mozilla::StaticPrefs::network_notify_changed()) {
     if (!mNetworkChangeTime.IsNull()) {

netwerk/system/linux/nsNetworkLinkService.cpp

@@ -68,11 +68,6 @@ nsNetworkLinkService::GetDnsSuffixList(nsTArray<nsCString>& aDnsSuffixList) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNetworkLinkService::GetVpnDetected(bool* aHasVPN) {
-  return NS_ERROR_NOT_IMPLEMENTED;
-}
-
 NS_IMETHODIMP
 nsNetworkLinkService::Observe(nsISupports* subject, const char* topic,
                               const char16_t* data) {

netwerk/system/mac/nsNetworkLinkService.mm

@@ -118,9 +118,6 @@ nsNetworkLinkService::GetNetworkID(nsACString& aNetworkID) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNetworkLinkService::GetVpnDetected(bool* aHasVPN) { return NS_ERROR_NOT_IMPLEMENTED; }
-
 // Note that this function is copied from xpcom/io/nsLocalFileUnix.cpp.
 static nsresult CFStringReftoUTF8(CFStringRef aInStrRef, nsACString& aOutStr) {
   // first see if the conversion would succeed and find the length of the result

netwerk/system/win32/nsNotifyAddrListener.cpp

@@ -66,7 +66,6 @@ nsNotifyAddrListener::nsNotifyAddrListener()
       mMutex("nsNotifyAddrListener::mMutex"),
       mCheckEvent(nullptr),
       mShutdown(false),
-      mFoundVPN(false),
       mIPInterfaceChecksum(0),
       mCoalescingActive(false) {}
 
@@ -115,12 +114,6 @@ nsNotifyAddrListener::GetDnsSuffixList(nsTArray<nsCString>& aDnsSuffixList) {
   return NS_OK;
 }
 
-NS_IMETHODIMP
-nsNotifyAddrListener::GetVpnDetected(bool* aHasVPN) {
-  *aHasVPN = mFoundVPN;
-  return NS_OK;
-}
-
 //
 // Hash the sorted network ids
 //
@@ -445,7 +438,7 @@ nsNotifyAddrListener::CheckAdaptersAddresses(void) {
   ULONG sumAll = 0;
 
   nsTArray<nsCString> dnsSuffixList;
-  mFoundVPN = false;
+
   if (ret == ERROR_SUCCESS) {
     bool linkUp = false;
     ULONG sum = 0;
@@ -458,11 +451,6 @@ nsNotifyAddrListener::CheckAdaptersAddresses(void) {
         continue;
       }
 
-      if (adapter->IfType == IF_TYPE_PPP) {
-        LOG(("VPN connection found"));
-        mFoundVPN = true;
-      }
-
       sum <<= 2;
       // Add chars from AdapterName to the checksum.
       for (int i = 0; adapter->AdapterName[i]; ++i) {

netwerk/system/win32/nsNotifyAddrListener.h

@@ -15,7 +15,6 @@
 #include "nsThreadUtils.h"
 #include "nsThreadPool.h"
 #include "nsCOMPtr.h"
-#include "mozilla/Atomics.h"
 #include "mozilla/TimeStamp.h"
 #include "mozilla/Mutex.h"
 #include "mozilla/SHA1.h"
@@ -72,9 +71,6 @@ class nsNotifyAddrListener : public nsINetworkLinkService,
   // set true when mCheckEvent means shutdown
   bool mShutdown;
 
-  // If a VPN was detected on at least one active network interface.
-  mozilla::Atomic<bool, mozilla::MemoryOrdering::Relaxed> mFoundVPN;
-
   // This is a checksum of various meta data for all network interfaces
   // considered UP at last check.
   ULONG mIPInterfaceChecksum;

netwerk/test/unit/test_trr.js

@@ -25,7 +25,7 @@ async function SetParentalControlEnabled(aEnabled) {
   MockRegistrar.unregister(cid);
 }
 
-function setup() {
+add_task(function setup() {
   dump("start!\n");
 
   let env = Cc["@mozilla.org/process/environment;1"].getService(
@@ -70,9 +70,8 @@ function setup() {
   addCertFromFile(certdb, "http2-ca.pem", "CTu,u,u");
 
   SetParentalControlEnabled(false);
-}
+});
 
-setup();
 registerCleanupFunction(() => {
   Services.prefs.clearUserPref("network.trr.mode");
   Services.prefs.clearUserPref("network.trr.uri");
@@ -1128,48 +1127,3 @@ add_task(async function test_dnsSuffix() {
   await checkDnsSuffixInMode(3);
   Services.prefs.clearUserPref("network.trr.bootstrapAddress");
 });
-
-add_task(async function test_vpnDetection() {
-  Services.prefs.setIntPref("network.trr.mode", 2);
-  Services.prefs.setCharPref(
-    "network.trr.uri",
-    `https://foo.example.com:${h2Port}/doh?responseIP=1.2.3.4&push=true`
-  );
-  dns.clearCache(true);
-  await new DNSListener("example.org", "1.2.3.4");
-  await new DNSListener("push.example.org", "2018::2018");
-
-  let networkLinkService = {
-    vpnDetected: true,
-    QueryInterface: ChromeUtils.generateQI([Ci.nsINetworkLinkService]),
-  };
-
-  Services.obs.notifyObservers(
-    networkLinkService,
-    "network:link-status-changed",
-    "changed"
-  );
-  await new DNSListener("example.org", "127.0.0.1");
-  await new DNSListener("test.com", "127.0.0.1");
-  // Also test that we don't use the pushed entry.
-  await new DNSListener("push.example.org", "127.0.0.1");
-
-  Services.prefs.setCharPref("network.trr.bootstrapAddress", "127.0.0.1");
-  Services.prefs.setIntPref("network.trr.mode", 3);
-  dns.clearCache(true);
-
-  await new DNSListener("example.org", "127.0.0.1");
-  await new DNSListener("test.com", "127.0.0.1");
-  // Also test that we don't use the pushed entry.
-  await new DNSListener("push.example.org", "127.0.0.1");
-
-  Services.prefs.clearUserPref("network.trr.bootstrapAddress");
-
-  // Attempt to clean up, just in case
-  networkLinkService.vpnDetected = false;
-  Services.obs.notifyObservers(
-    networkLinkService,
-    "network:link-status-changed",
-    "changed"
-  );
-});