Bug 1435141 - Check for user certificates on a background thread to avoid main thread IO r=keeler

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

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

netwerk/protocol/http/nsHttpHandler.cpp

@@ -65,6 +65,7 @@
 #include "mozilla/Telemetry.h"
 #include "mozilla/Unused.h"
 #include "mozilla/BasePrincipal.h"
+#include "mozilla/LazyIdleThread.h"
 
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/Navigator.h"
@@ -277,7 +278,6 @@ nsHttpHandler::nsHttpHandler()
       mConnectTimeout(90000),
       mTLSHandshakeTimeout(30000),
       mParallelSpeculativeConnectLimit(6),
-      mSpeculativeConnectEnabled(true),
       mRequestTokenBucketEnabled(true),
       mRequestTokenBucketMinParallelism(6),
       mRequestTokenBucketHz(100),
@@ -292,6 +292,7 @@ nsHttpHandler::nsHttpHandler()
       mDefaultHpackBuffer(4096),
       mMaxHttpResponseHeaderSize(393216),
       mFocusedWindowTransactionRatio(0.9f),
+      mSpeculativeConnectEnabled(false),
       mUseFastOpen(true),
       mFastOpenConsecutiveFailureLimit(5),
       mFastOpenConsecutiveFailureCounter(0),
@@ -466,6 +467,9 @@ nsresult nsHttpHandler::Init() {
   mIOService = new nsMainThreadPtrHolder<nsIIOService>(
       "nsHttpHandler::mIOService", service);
 
+  mBackgroundThread = new mozilla::LazyIdleThread(
+      10000, NS_LITERAL_CSTRING("HTTP Handler Background"));
+
   if (IsNeckoChild()) NeckoChild::InitNeckoChild();
 
   InitUserAgentComponents();
@@ -559,6 +563,7 @@ nsresult nsHttpHandler::Init() {
     obsService->AddObserver(this, "psm:user-certificate-added", true);
     obsService->AddObserver(this, "psm:user-certificate-deleted", true);
     obsService->AddObserver(this, "intl:app-locales-changed", true);
+    obsService->AddObserver(this, "browser-delayed-startup-finished", true);
 
     if (!IsNeckoChild()) {
       obsService->AddObserver(
@@ -2175,8 +2180,6 @@ nsHttpHandler::GetMisc(nsACString &value) {
 // nsHttpHandler::nsIObserver
 //-----------------------------------------------------------------------------
 
-static bool CanEnableSpeculativeConnect();  // forward declaration
-
 NS_IMETHODIMP
 nsHttpHandler::Observe(nsISupports *subject, const char *topic,
                        const char16_t *data) {
@@ -2326,10 +2329,12 @@ nsHttpHandler::Observe(nsISupports *subject, const char *topic,
   } else if (!strcmp(topic, "psm:user-certificate-deleted")) {
     // If a user certificate has been removed, we need to check if there
     // are others installed
-    mSpeculativeConnectEnabled = CanEnableSpeculativeConnect();
+    MaybeEnableSpeculativeConnect();
   } else if (!strcmp(topic, "intl:app-locales-changed")) {
     // If the locale changed, there's a chance the accept language did too
     mAcceptLanguagesIsDirty = true;
+  } else if (!strcmp(topic, "browser-delayed-startup-finished")) {
+    MaybeEnableSpeculativeConnect();
   }
 
   return NS_OK;
@@ -2338,13 +2343,9 @@ nsHttpHandler::Observe(nsISupports *subject, const char *topic,
 // nsISpeculativeConnect
 
 static bool CanEnableSpeculativeConnect() {
-  MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
-
   nsCOMPtr<nsINSSComponent> component(do_GetService(PSM_COMPONENT_CONTRACTID));
-  if (!component) {
-    return false;
-  }
 
+  MOZ_ASSERT(!NS_IsMainThread(), "Must run on the background thread");
   // Check if any 3rd party PKCS#11 module are installed, as they may produce
   // client certificates
   bool activeSmartCards = false;
@@ -2361,9 +2362,34 @@ static bool CanEnableSpeculativeConnect() {
     return false;
   }
 
+  // No smart cards and no client certificates means
+  // we can enable speculative connect.
   return true;
 }
 
+void nsHttpHandler::MaybeEnableSpeculativeConnect() {
+  MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
+
+  // We don't need to and can't check this in the child process.
+  if (IsNeckoChild()) {
+    return;
+  }
+
+  if (!mBackgroundThread) {
+    NS_WARNING(
+        "nsHttpHandler::MaybeEnableSpeculativeConnect() no background thread");
+    return;
+  }
+
+  net_EnsurePSMInit();
+
+  mBackgroundThread->Dispatch(
+      NS_NewRunnableFunction("CanEnableSpeculativeConnect", [] {
+        gHttpHandler->mSpeculativeConnectEnabled =
+            CanEnableSpeculativeConnect();
+      }));
+}
+
 nsresult nsHttpHandler::SpeculativeConnectInternal(
     nsIURI *aURI, nsIPrincipal *aPrincipal, nsIInterfaceRequestor *aCallbacks,
     bool anonymous) {
@@ -2447,12 +2473,6 @@ nsresult nsHttpHandler::SpeculativeConnectInternal(
   rv = aURI->SchemeIs("https", &usingSSL);
   if (NS_FAILED(rv)) return rv;
 
-  static bool sCheckedIfSpeculativeEnabled = false;
-  if (!sCheckedIfSpeculativeEnabled) {
-    sCheckedIfSpeculativeEnabled = true;
-    mSpeculativeConnectEnabled = CanEnableSpeculativeConnect();
-  }
-
   if (usingSSL && !mSpeculativeConnectEnabled) {
     return NS_ERROR_UNEXPECTED;
   }

netwerk/protocol/http/nsHttpHandler.h

@@ -437,6 +437,10 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
 
   void EnsureUAOverridesInit();
 
+  // Checks if there are any user certs or active smart cards on a different
+  // thread. Updates mSpeculativeConnectEnabled when done.
+  void MaybeEnableSpeculativeConnect();
+
  private:
   // cached services
   nsMainThreadPtrHandle<nsIIOService> mIOService;
@@ -451,6 +455,10 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
   // the connection manager
   RefPtr<nsHttpConnectionMgr> mConnMgr;
 
+  // This thread is used for performing operations that should not block
+  // the main thread.
+  nsCOMPtr<nsIThread> mBackgroundThread;
+
   //
   // prefs
   //
@@ -597,10 +605,6 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
   // when starting a new speculative connection.
   uint32_t mParallelSpeculativeConnectLimit;
 
-  // We may disable speculative connect if the browser has user certificates
-  // installed as that might randomly popup the certificate choosing window.
-  bool mSpeculativeConnectEnabled;
-
   // For Rate Pacing of HTTP/1 requests through a netwerk/base/EventTokenBucket
   // Active requests <= *MinParallelism are not subject to the rate pacing
   bool mRequestTokenBucketEnabled;
@@ -641,6 +645,10 @@ class nsHttpHandler final : public nsIHttpProtocolHandler,
   // The ratio for dispatching transactions from the focused window.
   float mFocusedWindowTransactionRatio;
 
+  // We may disable speculative connect if the browser has user certificates
+  // installed as that might randomly popup the certificate choosing window.
+  Atomic<bool, Relaxed> mSpeculativeConnectEnabled;
+
   Atomic<bool, Relaxed> mUseFastOpen;
   Atomic<bool, Relaxed> mFastOpenSupported;
   uint32_t mFastOpenConsecutiveFailureLimit;

security/manager/ssl/nsINSSComponent.idl

@@ -87,13 +87,11 @@ interface nsINSSComponent : nsISupports {
 
   /**
    * Returns true if the user has a PKCS#11 module with removable slots.
-   * Main thread only.
    */
   [noscript] bool hasActiveSmartCards();
 
   /**
    * Returns true if the user has any client authentication certificates.
-   * Main thread only.
    */
   [noscript] bool hasUserCertsInstalled();
 

security/manager/ssl/nsNSSComponent.cpp

@@ -695,14 +695,10 @@ LoadLoadableRootsTask::Run() {
 NS_IMETHODIMP
 nsNSSComponent::HasActiveSmartCards(bool* result) {
   NS_ENSURE_ARG_POINTER(result);
-  MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
-  if (!NS_IsMainThread()) {
-    return NS_ERROR_NOT_SAME_THREAD;
-  }
+
+  BlockUntilLoadableRootsLoaded();
 
 #ifndef MOZ_NO_SMART_CARDS
-  MutexAutoLock nsNSSComponentLock(mMutex);
-
   AutoSECMODListReadLock secmodLock;
   SECMODModuleList* list = SECMOD_GetDefaultModuleList();
   while (list) {
@@ -720,10 +716,8 @@ nsNSSComponent::HasActiveSmartCards(bool* result) {
 NS_IMETHODIMP
 nsNSSComponent::HasUserCertsInstalled(bool* result) {
   NS_ENSURE_ARG_POINTER(result);
-  MOZ_ASSERT(NS_IsMainThread(), "Main thread only");
-  if (!NS_IsMainThread()) {
-    return NS_ERROR_NOT_SAME_THREAD;
-  }
+
+  BlockUntilLoadableRootsLoaded();
 
   *result = false;
   UniqueCERTCertList certList(CERT_FindUserCertsByUsage(