Bug 1489229 - crash on MOZ_RELEASE_ASSERT which checks that WPAD is not being r=valentin

called if not explicitly requested by the user prefs

The author did not isolate and fix the cause of the assertion failure,
but put in further diagnostics.The author did not isolate and fix the cause of the assertion failure,
but put in further diagnostics.
* an additional assertion was put in on the main thread (which if
triggered would reveal the stack trace)
* in one place where a previously a failure to read the network.proxy.type pref
was ignored, execution of WPAD is now halted with a warning.

Besides these improved diagnostics, in nsPACMan::LoadPACFromURI where an
asynchronous call was made to nsPACMan::StartLoading *before* the preconditions
for this call are set up was changed to be the other way around. The author
suspects that the previous code may have led to race conditions when
LoadPACFromURI was not called from the main thread, although it is not obvious
that this would have caused such a crash.

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

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

netwerk/base/nsPACMan.cpp

@@ -544,6 +544,29 @@ nsPACMan::LoadPACFromURI(const nsACString &aSpec)
   NS_ENSURE_STATE(loader);
 
   LOG(("nsPACMan::LoadPACFromURI aSpec: %s\n", aSpec.BeginReading()));
+
+  CancelExistingLoad();
+
+  mLoader = loader;
+  mPACURIRedirectSpec.Truncate();
+  mNormalPACURISpec.Truncate(); // set at load time
+  mLoadFailureCount = 0;  // reset
+  mAutoDetect = aSpec.IsEmpty();
+  mPACURISpec.Assign(aSpec);
+
+  // reset to Null
+  mScheduledReload = TimeStamp();
+
+  // if we're on the main thread here so we can get hold of prefs,
+  // we check that we have WPAD preffed on if we're auto-detecting
+  if (mAutoDetect && NS_IsMainThread()) {
+    nsresult rv = GetNetworkProxyTypeFromPref(&mProxyConfigType);
+    if (NS_FAILED(rv)) {
+      return rv;
+    }
+    MOZ_RELEASE_ASSERT(mProxyConfigType == nsIProtocolProxyService::PROXYCONFIG_WPAD,
+            "WPAD is being executed when not selected by user");
+  }
   // Since we might get called from nsProtocolProxyService::Init, we need to
   // post an event back to the main thread before we try to use the IO service.
   //
@@ -561,17 +584,6 @@ nsPACMan::LoadPACFromURI(const nsACString &aSpec)
     mLoadPending = true;
   }
 
-  CancelExistingLoad();
-
-  mLoader = loader;
-  mPACURIRedirectSpec.Truncate();
-  mNormalPACURISpec.Truncate(); // set at load time
-  mLoadFailureCount = 0;  // reset
-  mAutoDetect = aSpec.IsEmpty();
-  mPACURISpec.Assign(aSpec);
-
-  // reset to Null
-  mScheduledReload = TimeStamp();
   return NS_OK;
 }
 
@@ -638,7 +650,11 @@ nsPACMan::StartLoading()
   }
 
   if (mAutoDetect) {
-    GetNetworkProxyTypeFromPref(&mProxyConfigType);
+    nsresult rv = GetNetworkProxyTypeFromPref(&mProxyConfigType);
+    if (!NS_SUCCEEDED(rv)) {
+      NS_WARNING("Could not retrieve Network Proxy Type pref when auto-detecting proxy. Halting.");
+      return;
+    }
     RefPtr<ExecutePACThreadAction> wpadConfigurer =
       new ExecutePACThreadAction(this);
     wpadConfigurer->ConfigureWPAD();