Bug 1495024 - Firefox does not use exponential back-off after failing to load a PAC file. r=bagder
authorPolly Shaw <polly.shaw@gmail.com>
Fri, 19 Oct 2018 09:13:16 +0000
changeset 500615 ab0725fa9dc15ce3e3d8c0c15f63df516cdd12cd
parent 500614 929617c0f367c6fe1b8751dbcca043235490494e
child 500616 5ba65c3ad1ffc76c31dcfda165b87afc1269b415
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbagder
bugs1495024, 356831
milestone64.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 1495024 - Firefox does not use exponential back-off after failing to load a PAC file. r=bagder This patch addresses a bug introduced in the solution to Bug 356831, in which the back-off time for reloading PAC files was set to the shortest interval every time a failure happened, thus auto-detecting PAC every 5 seconds on a network on which WPAD did not resolve when the proxy was set to auto-detect. The changes in this patch are: * nsPACMan.h - declares a private overload to LoadPACFromURI, with an additional parameter called aResetLoadFailureCount. * nsPACMan.cpp - moves the implementation of the old LoadPACFromURI to the new private overload, with the modification that the mLoadFailureCount field is only reset to 0 if aResetLoadFailureCount is true. Replaces the implementation of the public LoadPACFromURI with a call to the private overload with aResetLoadFailureCount = true. Also replaces the call made from within nsPACMan when triggering an internal reload with a call with aResetLoadFailureCount = false, thus ensuring that internally triggered reloads do not reset the back-off time. Differential Revision: https://phabricator.services.mozilla.com/D9035
netwerk/base/nsPACMan.cpp
netwerk/base/nsPACMan.h
--- a/netwerk/base/nsPACMan.cpp
+++ b/netwerk/base/nsPACMan.cpp
@@ -497,17 +497,17 @@ nsPACMan::AsyncGetProxyForURI(nsIURI *ur
   if (mShutdown)
     return NS_ERROR_NOT_AVAILABLE;
 
   // Maybe Reload PAC
   if (!mPACURISpec.IsEmpty() && !mScheduledReload.IsNull() &&
       TimeStamp::Now() > mScheduledReload) {
     LOG(("nsPACMan::AsyncGetProxyForURI reload as scheduled\n"));
 
-    LoadPACFromURI(mAutoDetect? EmptyCString(): mPACURISpec);
+    LoadPACFromURI(mAutoDetect? EmptyCString(): mPACURISpec, false);
   }
 
   RefPtr<PendingPACQuery> query =
     new PendingPACQuery(this, uri, callback, mainThreadResponse);
 
   if (IsPACURI(uri)) {
     // deal with this directly instead of queueing it
     query->Complete(NS_OK, EmptyCString());
@@ -532,30 +532,38 @@ nsPACMan::PostQuery(PendingPACQuery *que
   mPendingQ.insertBack(addref.forget().take());
   ProcessPendingQ();
   return NS_OK;
 }
 
 nsresult
 nsPACMan::LoadPACFromURI(const nsACString &aSpec)
 {
+  return LoadPACFromURI(aSpec, true);
+}
+
+nsresult
+nsPACMan::LoadPACFromURI(const nsACString &aSpec, bool aResetLoadFailureCount)
+{
   NS_ENSURE_STATE(!mShutdown);
 
   nsCOMPtr<nsIStreamLoader> loader =
       do_CreateInstance(NS_STREAMLOADER_CONTRACTID);
   NS_ENSURE_STATE(loader);
 
-  LOG(("nsPACMan::LoadPACFromURI aSpec: %s\n", aSpec.BeginReading()));
+  LOG(("nsPACMan::LoadPACFromURI aSpec: %s, aResetLoadFailureCount: %s\n", aSpec.BeginReading(), aResetLoadFailureCount? "true": "false"));
 
   CancelExistingLoad();
 
   mLoader = loader;
   mPACURIRedirectSpec.Truncate();
   mNormalPACURISpec.Truncate(); // set at load time
-  mLoadFailureCount = 0;  // reset
+  if (aResetLoadFailureCount) {
+    mLoadFailureCount = 0;
+  }
   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
--- a/netwerk/base/nsPACMan.h
+++ b/netwerk/base/nsPACMan.h
@@ -204,16 +204,30 @@ private:
    */
   void StartLoading();
 
   /**
    * Continue loading the PAC file.
    */
   void ContinueLoadingAfterPACUriKnown();
 
+/**
+   * This method may be called to reload the PAC file.  While we are loading
+   * the PAC file, any asynchronous PAC queries will be queued up to be
+   * processed once the PAC file finishes loading.
+   *
+   * @param aSpec
+   *        The non normalized uri spec of this URI used for comparison with
+   *        system proxy settings to determine if the PAC uri has changed.
+   * @param aResetLoadFailureCount
+   *        A flag saying whether the exponential back-off for attempting to reload the
+   *        PAC should be reset.
+   */
+  nsresult LoadPACFromURI(const nsACString &aSpec, bool aResetLoadFailureCount);
+
   /**
    * Reload the PAC file if there is reason to.
    */
   void MaybeReloadPAC();
 
   /**
    * Called when we fail to load the PAC file.
    */