Bug 1448034 - Part 2: Lazily create ProxyResolution thread. r=bagder

This delays the creation of the PAC thread until we need to dispatch a
runnable to it.

--HG--
extra : rebase_source : 8897d3d8724f082ad33027635795512ccb4a17eb
extra : source : 068bb4e7b8494d8ae82dfd1b1f22680234bf038c

netwerk/base/ProxyAutoConfig.cpp

@@ -412,7 +412,7 @@ bool PACResolve(const nsCString &aHostName, NetAddr *aNetAddr,
 ProxyAutoConfig::ProxyAutoConfig()
   : mJSContext(nullptr)
   , mJSNeedsSetup(false)
-  , mShutdown(false)
+  , mShutdown(true)
   , mIncludePath(false)
   , mExtraHeapSize(0)
 {
@@ -704,6 +704,8 @@ ProxyAutoConfig::Init(const nsCString &aPACURI,
                       uint32_t aExtraHeapSize,
                       nsIEventTarget *aEventTarget)
 {
+  mShutdown = false; // Shutdown needs to be called prior to destruction
+
   mPACURI = aPACURI;
   mPACScript = sPacUtils;
   mPACScript.Append(aPACScript);

netwerk/base/nsPACMan.cpp

@@ -19,6 +19,8 @@
 #include "nsISystemProxySettings.h"
 #include "nsNetUtil.h"
 #include "nsThreadUtils.h"
+#include "mozilla/Result.h"
+#include "mozilla/ResultExtensions.h"
 
 //-----------------------------------------------------------------------------
 
@@ -266,12 +268,14 @@ public:
     , mSetupPAC(false)
     , mExtraHeapSize(0)
     , mConfigureWPAD(false)
+    , mShutdown(false)
   { }
 
-  void CancelQueue (nsresult status)
+  void CancelQueue (nsresult status, bool aShutdown)
   {
     mCancel = true;
     mCancelStatus = status;
+    mShutdown = aShutdown;
   }
 
   void SetupPAC (const char *text,
@@ -294,7 +298,7 @@ public:
   {
     MOZ_ASSERT(!NS_IsMainThread(), "wrong thread");
     if (mCancel) {
-      mPACMan->CancelPendingQ(mCancelStatus);
+      mPACMan->CancelPendingQ(mCancelStatus, mShutdown);
       mCancel = false;
       return NS_OK;
     }
@@ -338,6 +342,7 @@ private:
   nsCString            mSetupPACData;
   nsCString            mSetupPACURI;
   bool                 mConfigureWPAD;
+  bool                 mShutdown;
 };
 
 //-----------------------------------------------------------------------------
@@ -446,14 +451,41 @@ nsPACMan::Shutdown()
   if (mShutdown) {
     return;
   }
-  mShutdown = true;
+
   CancelExistingLoad();
 
-  MOZ_ASSERT(mPACThread, "mPAC requires mPACThread to shutdown");
-  PostCancelPendingQ(NS_ERROR_ABORT);
+  if (mPACThread) {
+    PostCancelPendingQ(NS_ERROR_ABORT, /*aShutdown =*/ true);
 
-  RefPtr<WaitForThreadShutdown> runnable = new WaitForThreadShutdown(this);
-  Dispatch(runnable.forget());
+    // Shutdown is initiated from an observer. We don't want to block the
+    // observer service on thread shutdown so we post a shutdown runnable that
+    // will run after we return instead.
+    RefPtr<WaitForThreadShutdown> runnable = new WaitForThreadShutdown(this);
+    Dispatch(runnable.forget());
+  }
+
+  mShutdown = true;
+}
+
+nsresult
+nsPACMan::DispatchToPAC(already_AddRefed<nsIRunnable> aEvent, bool aSync)
+{
+  MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
+
+  nsCOMPtr<nsIRunnable> e(aEvent);
+
+  if (mShutdown) {
+    return NS_ERROR_NOT_AVAILABLE;
+  }
+
+  // Lazily create the PAC thread. This method is main-thread only so we don't
+  // have to worry about threading issues here.
+  if (!mPACThread) {
+    MOZ_TRY(NS_NewNamedThread("ProxyResolution", getter_AddRefs(mPACThread)));
+  }
+
+  return mPACThread->Dispatch(e.forget(), aSync ? nsIEventTarget::DISPATCH_SYNC :
+                                                  nsIEventTarget::DISPATCH_NORMAL);
 }
 
 nsresult
@@ -482,7 +514,7 @@ nsPACMan::AsyncGetProxyForURI(nsIURI *uri,
     return NS_OK;
   }
 
-  return mPACThread->Dispatch(query, nsIEventTarget::DISPATCH_NORMAL);
+  return DispatchToPAC(query.forget());
 }
 
 nsresult
@@ -610,9 +642,7 @@ nsPACMan::StartLoading()
     RefPtr<ExecutePACThreadAction> wpadConfigurer =
       new ExecutePACThreadAction(this);
     wpadConfigurer->ConfigureWPAD();
-    if (mPACThread) {
-      mPACThread->Dispatch(wpadConfigurer, nsIEventTarget::DISPATCH_NORMAL);
-    }
+    DispatchToPAC(wpadConfigurer.forget());
   } else {
     ContinueLoadingAfterPACUriKnown();
   }
@@ -716,23 +746,21 @@ nsPACMan::PostProcessPendingQ()
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   RefPtr<ExecutePACThreadAction> pending =
     new ExecutePACThreadAction(this);
-  if (mPACThread)
-    mPACThread->Dispatch(pending, nsIEventTarget::DISPATCH_NORMAL);
+  DispatchToPAC(pending.forget());
 }
 
 void
-nsPACMan::PostCancelPendingQ(nsresult status)
+nsPACMan::PostCancelPendingQ(nsresult status, bool aShutdown)
 {
   MOZ_ASSERT(NS_IsMainThread(), "wrong thread");
   RefPtr<ExecutePACThreadAction> pending =
     new ExecutePACThreadAction(this);
-  pending->CancelQueue(status);
-  if (mPACThread)
-    mPACThread->Dispatch(pending, nsIEventTarget::DISPATCH_NORMAL);
+  pending->CancelQueue(status, aShutdown);
+  DispatchToPAC(pending.forget());
 }
 
 void
-nsPACMan::CancelPendingQ(nsresult status)
+nsPACMan::CancelPendingQ(nsresult status, bool aShutdown)
 {
   MOZ_ASSERT(!NS_IsMainThread(), "wrong thread");
   RefPtr<PendingPACQuery> query;
@@ -742,7 +770,7 @@ nsPACMan::CancelPendingQ(nsresult status)
     query->Complete(status, EmptyCString());
   }
 
-  if (mShutdown)
+  if (aShutdown)
     mPAC.Shutdown();
 }
 
@@ -869,8 +897,7 @@ nsPACMan::OnStreamComplete(nsIStreamLoader *loader,
     RefPtr<ExecutePACThreadAction> pending =
       new ExecutePACThreadAction(this);
     pending->SetupPAC(text, dataLen, pacURI, GetExtraJSContextHeapSize());
-    if (mPACThread)
-      mPACThread->Dispatch(pending, nsIEventTarget::DISPATCH_NORMAL);
+    DispatchToPAC(pending.forget());
 
     LOG(("OnStreamComplete: process the PAC contents\n"));
 
@@ -946,12 +973,8 @@ nsPACMan::Init(nsISystemProxySettings *systemProxySettings)
 {
   mSystemProxySettings = systemProxySettings;
   mDHCPClient = do_GetService(NS_DHCPCLIENT_CONTRACTID);
-
-  nsresult rv =
-    NS_NewNamedThread("ProxyResolution", getter_AddRefs(mPACThread));
-
-  return rv;
+  return NS_OK;
 }
 
 } // namespace net
-} // namespace mozilla
+} // namespace mozilla

netwerk/base/nsPACMan.h

@@ -176,7 +176,7 @@ public:
 
   // PAC thread operations only
   void ProcessPendingQ();
-  void CancelPendingQ(nsresult);
+  void CancelPendingQ(nsresult, bool aShutdown);
 
   void SetWPADOverDHCPEnabled(bool aValue) { mWPADOverDHCPEnabled = aValue; }
 
@@ -232,12 +232,21 @@ private:
 
   // PAC thread operations only
   void PostProcessPendingQ();
-  void PostCancelPendingQ(nsresult);
+  void PostCancelPendingQ(nsresult, bool aShutdown = false);
   bool ProcessPending();
   nsresult GetPACFromDHCP(nsACString &aSpec);
   nsresult ConfigureWPAD(nsACString &aSpec);
 
 private:
+  /**
+   * Dispatches a runnable to the PAC processing thread. Handles lazy
+   * instantiation of the thread.
+   *
+   * @param aEvent The event to disptach.
+   * @param aSync Whether or not this should be synchronous dispatch.
+   */
+  nsresult DispatchToPAC(already_AddRefed<nsIRunnable> aEvent, bool aSync = false);
+
   ProxyAutoConfig mPAC;
   nsCOMPtr<nsIThread>           mPACThread;
   nsCOMPtr<nsISystemProxySettings> mSystemProxySettings;
@@ -270,4 +279,4 @@ extern LazyLogModule gProxyLog;
 } // namespace net
 } // namespace mozilla
 
-#endif  // nsPACMan_h__
+#endif  // nsPACMan_h__

netwerk/test/gtest/TestPACMan.cpp

@@ -201,7 +201,7 @@ class TestPACMan : public ::testing::Test {
       RefPtr<ProcessPendingEventsAction> action =
           new ProcessPendingEventsAction();
 
-      mPACMan->mPACThread->Dispatch(action, nsIEventTarget::DISPATCH_SYNC);
+      mPACMan->DispatchToPAC(action.forget(), /*aSync =*/ true);
     }
 };
 
@@ -279,4 +279,4 @@ TEST_F(TestPACMan, WhenPACUriIsSetDirectlyItIsUsedRatherThanWPAD) {
 }
 
 } // namespace net
-} // namespace mozilla
+} // namespace mozilla