Bug 1562388 - Remove legacy ICS handling from nsNotifyAddrListener r=mayhemer
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 12 Nov 2019 18:21:48 +0000
changeset 501651 8171a4e575e2d06a6240ad67b72574ced6e57ca8
parent 501650 3b98d7dcb7066c220a0c378d83a103535d98d93b
child 501652 5350d0eedcf62664919c62fe2f6ffda1e00292c5
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmayhemer
bugs1562388, 465158
milestone72.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 1562388 - Remove legacy ICS handling from nsNotifyAddrListener r=mayhemer This code was added in bug 465158 to deal with on-demand dial-up connections. That scenario is hopefully not common anymore. One benefit of the code was that it excludes network adapters that share the computers network from being part of network change notifications, network ID, etc. However, the code assumed that only adapters with the address 192.168.0.1 would be a ICS adapter, but trying to set up ICS on Windows 10 actually set the LAN interface with 192.168.137.1 - so that assumption isn't valid anymore. So the code is likely not even running for adapters that are ICS. It may run for adapters which have the IP 192.168.0.1, but probably that's quite rare as that's more often a gateway IP than a client IP. In any case, it's better to just get rid of it and if needed, however unlikely, we'll replace it with a more modern API. Depends on D52573 Differential Revision: https://phabricator.services.mozilla.com/D52574
netwerk/system/win32/nsNotifyAddrListener.cpp
netwerk/system/win32/nsNotifyAddrListener.h
--- a/netwerk/system/win32/nsNotifyAddrListener.cpp
+++ b/netwerk/system/win32/nsNotifyAddrListener.cpp
@@ -46,58 +46,35 @@
 #include <iphlpapi.h>
 
 using namespace mozilla;
 
 static LazyLogModule gNotifyAddrLog("nsNotifyAddr");
 #define LOG(args) MOZ_LOG(gNotifyAddrLog, mozilla::LogLevel::Debug, args)
 #define LOG_ENABLED() MOZ_LOG_TEST(gNotifyAddrLog, mozilla::LogLevel::Debug)
 
-static HMODULE sNetshell;
-static decltype(NcFreeNetconProperties)* sNcFreeNetconProperties;
-
 // period during which to absorb subsequent network change events, in
 // milliseconds
 static const unsigned int kNetworkChangeCoalescingPeriod = 1000;
 
-static void InitNetshellLibrary(void) {
-  if (!sNetshell) {
-    sNetshell = LoadLibraryW(L"Netshell.dll");
-    if (sNetshell) {
-      sNcFreeNetconProperties =
-          (decltype(NcFreeNetconProperties)*)GetProcAddress(
-              sNetshell, "NcFreeNetconProperties");
-    }
-  }
-}
-
-static void FreeDynamicLibraries(void) {
-  if (sNetshell) {
-    sNcFreeNetconProperties = nullptr;
-    FreeLibrary(sNetshell);
-    sNetshell = nullptr;
-  }
-}
-
 NS_IMPL_ISUPPORTS(nsNotifyAddrListener, nsINetworkLinkService, nsIRunnable,
                   nsIObserver)
 
 nsNotifyAddrListener::nsNotifyAddrListener()
     : mLinkUp(true),  // assume true by default
       mStatusKnown(false),
       mCheckAttempted(false),
       mMutex("nsNotifyAddrListener::mMutex"),
       mCheckEvent(nullptr),
       mShutdown(false),
       mIPInterfaceChecksum(0),
       mCoalescingActive(false) {}
 
 nsNotifyAddrListener::~nsNotifyAddrListener() {
   NS_ASSERTION(!mThread, "nsNotifyAddrListener thread shutdown failed");
-  FreeDynamicLibraries();
 }
 
 NS_IMETHODIMP
 nsNotifyAddrListener::GetIsLinkUp(bool* aIsUp) {
   if (!mCheckAttempted && !mStatusKnown) {
     mCheckAttempted = true;
     CheckLinkStatus();
   }
@@ -413,103 +390,16 @@ nsNotifyAddrListener::ChangeEvent::Run()
   nsCOMPtr<nsIObserverService> observerService =
       mozilla::services::GetObserverService();
   if (observerService)
     observerService->NotifyObservers(mService, NS_NETWORK_LINK_TOPIC,
                                      NS_ConvertASCIItoUTF16(mEventID).get());
   return NS_OK;
 }
 
-// Bug 465158 features an explanation for this check. ICS being "Internet
-// Connection Sharing). The description says it is always IP address
-// 192.168.0.1 for this case.
-bool nsNotifyAddrListener::CheckICSGateway(PIP_ADAPTER_ADDRESSES aAdapter) {
-  if (!aAdapter->FirstUnicastAddress) return false;
-
-  LPSOCKADDR aAddress = aAdapter->FirstUnicastAddress->Address.lpSockaddr;
-  if (!aAddress) return false;
-
-  PSOCKADDR_IN in_addr = (PSOCKADDR_IN)aAddress;
-  bool isGateway = (aAddress->sa_family == AF_INET &&
-                    in_addr->sin_addr.S_un.S_un_b.s_b1 == 192 &&
-                    in_addr->sin_addr.S_un.S_un_b.s_b2 == 168 &&
-                    in_addr->sin_addr.S_un.S_un_b.s_b3 == 0 &&
-                    in_addr->sin_addr.S_un.S_un_b.s_b4 == 1);
-
-  if (isGateway) isGateway = CheckICSStatus(aAdapter->FriendlyName);
-
-  return isGateway;
-}
-
-bool nsNotifyAddrListener::CheckICSStatus(PWCHAR aAdapterName) {
-  InitNetshellLibrary();
-  // This method enumerates all privately shared connections and checks if some
-  // of them has the same name as the one provided in aAdapterName. If such
-  // connection is found in the collection the adapter is used as ICS gateway
-  bool isICSGatewayAdapter = false;
-
-  HRESULT hr;
-  RefPtr<INetSharingManager> netSharingManager;
-  hr = CoCreateInstance(CLSID_NetSharingManager, nullptr, CLSCTX_INPROC_SERVER,
-                        IID_INetSharingManager,
-                        getter_AddRefs(netSharingManager));
-
-  RefPtr<INetSharingPrivateConnectionCollection> privateCollection;
-  if (SUCCEEDED(hr)) {
-    hr = netSharingManager->get_EnumPrivateConnections(
-        ICSSC_DEFAULT, getter_AddRefs(privateCollection));
-  }
-
-  RefPtr<IEnumNetSharingPrivateConnection> privateEnum;
-  if (SUCCEEDED(hr)) {
-    RefPtr<IUnknown> privateEnumUnknown;
-    hr = privateCollection->get__NewEnum(getter_AddRefs(privateEnumUnknown));
-    if (SUCCEEDED(hr)) {
-      hr = privateEnumUnknown->QueryInterface(
-          IID_IEnumNetSharingPrivateConnection, getter_AddRefs(privateEnum));
-    }
-  }
-
-  if (SUCCEEDED(hr)) {
-    ULONG fetched;
-    VARIANT connectionVariant;
-    while (!isICSGatewayAdapter &&
-           SUCCEEDED(hr = privateEnum->Next(1, &connectionVariant, &fetched)) &&
-           fetched) {
-      if (connectionVariant.vt != VT_UNKNOWN) {
-        // We should call VariantClear here but it needs to link
-        // with oleaut32.lib that produces a Ts incrase about 10ms
-        // that is undesired. As it is quit unlikely the result would
-        // be of a different type anyway, let's pass the variant
-        // unfreed here.
-        NS_ERROR(
-            "Variant of unexpected type, expecting VT_UNKNOWN, we probably "
-            "leak it!");
-        continue;
-      }
-
-      RefPtr<INetConnection> connection;
-      if (SUCCEEDED(connectionVariant.punkVal->QueryInterface(
-              IID_INetConnection, getter_AddRefs(connection)))) {
-        connectionVariant.punkVal->Release();
-
-        NETCON_PROPERTIES* properties;
-        if (SUCCEEDED(connection->GetProperties(&properties))) {
-          if (!wcscmp(properties->pszwName, aAdapterName))
-            isICSGatewayAdapter = true;
-
-          if (sNcFreeNetconProperties) sNcFreeNetconProperties(properties);
-        }
-      }
-    }
-  }
-
-  return isICSGatewayAdapter;
-}
-
 DWORD
 nsNotifyAddrListener::CheckAdaptersAddresses(void) {
   ULONG len = 16384;
 
   PIP_ADAPTER_ADDRESSES adapterList = (PIP_ADAPTER_ADDRESSES)moz_xmalloc(len);
 
   ULONG flags = GAA_FLAG_SKIP_DNS_SERVER | GAA_FLAG_SKIP_MULTICAST |
                 GAA_FLAG_SKIP_ANYCAST;
@@ -541,18 +431,17 @@ nsNotifyAddrListener::CheckAdaptersAddre
   if (ret == ERROR_SUCCESS) {
     bool linkUp = false;
     ULONG sum = 0;
 
     for (PIP_ADAPTER_ADDRESSES adapter = adapterList; adapter;
          adapter = adapter->Next) {
       if (adapter->OperStatus != IfOperStatusUp ||
           !adapter->FirstUnicastAddress ||
-          adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK ||
-          CheckICSGateway(adapter)) {
+          adapter->IfType == IF_TYPE_SOFTWARE_LOOPBACK) {
         continue;
       }
 
       sum <<= 2;
       // Add chars from AdapterName to the checksum.
       for (int i = 0; adapter->AdapterName[i]; ++i) {
         sum += adapter->AdapterName[i];
       }
--- a/netwerk/system/win32/nsNotifyAddrListener.h
+++ b/netwerk/system/win32/nsNotifyAddrListener.h
@@ -55,20 +55,16 @@ class nsNotifyAddrListener : public nsIN
   bool mStatusKnown;
   bool mCheckAttempted;
 
   nsresult Shutdown(void);
   nsresult SendEvent(const char* aEventID);
 
   DWORD CheckAdaptersAddresses(void);
 
-  // Checks for an Internet Connection Sharing (ICS) gateway.
-  bool CheckICSGateway(PIP_ADAPTER_ADDRESSES aAdapter);
-  bool CheckICSStatus(PWCHAR aAdapterName);
-
   // This threadpool only ever holds 1 thread. It is a threadpool and not a
   // regular thread so that we may call shutdownWithTimeout on it.
   nsCOMPtr<nsIThreadPool> mThread;
 
  private:
   // Returns the new timeout period for coalescing (or INFINITE)
   DWORD nextCoalesceWaitTime();