Bug 1536744 - Make resource protocol handler thread safe r=baku

This is achieved by adding a RWLock to SubstitutingProtocolHandler

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

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

netwerk/base/nsNetUtil.cpp

@@ -99,6 +99,8 @@
 #include "nsSocketTransportService2.h"
 #include "nsViewSourceHandler.h"
 
+#include "nsResProtocolHandler.h"
+#include "mozilla/net/ExtensionProtocolHandler.h"
 #include <limits>
 
 using namespace mozilla;
@@ -1795,13 +1797,7 @@ nsresult NS_NewURIOnAnyThread(nsIURI** aURI, const nsACString& aSpec,
   }
 
   if (scheme.EqualsLiteral("resource")) {
-    if (!NS_IsMainThread()) {
-      return NS_ERROR_NOT_AVAILABLE;
-    }
-    // TODO: must be thread safe
-    MOZ_ASSERT(NS_IsMainThread());
-    nsCOMPtr<nsIProtocolHandler> handler =
-        do_GetService("@mozilla.org/network/protocol;1?name=resource");
+    RefPtr<nsResProtocolHandler> handler = nsResProtocolHandler::GetSingleton();
     if (!handler) {
       return NS_ERROR_NOT_AVAILABLE;
     }
@@ -1814,8 +1810,8 @@ nsresult NS_NewURIOnAnyThread(nsIURI** aURI, const nsACString& aSpec,
     }
     // TODO: must be thread safe
     MOZ_ASSERT(NS_IsMainThread());
-    nsCOMPtr<nsIProtocolHandler> handler =
-        do_GetService("@mozilla.org/network/protocol;1?name=moz-extension");
+    RefPtr<mozilla::net::ExtensionProtocolHandler> handler =
+        mozilla::net::ExtensionProtocolHandler::GetSingleton();
     if (!handler) {
       return NS_ERROR_NOT_AVAILABLE;
     }

netwerk/protocol/res/SubstitutingProtocolHandler.cpp

@@ -200,6 +200,7 @@ SubstitutingProtocolHandler::SubstitutingProtocolHandler(const char* aScheme,
                                                          uint32_t aFlags,
                                                          bool aEnforceFileOrJar)
     : mScheme(aScheme),
+      mSubstitutionsLock("SubstitutingProtocolHandler::mSubstitutions"),
       mSubstitutions(16),
       mEnforceFileOrJar(aEnforceFileOrJar) {
   mFlags.emplace(aFlags);
@@ -207,7 +208,10 @@ SubstitutingProtocolHandler::SubstitutingProtocolHandler(const char* aScheme,
 }
 
 SubstitutingProtocolHandler::SubstitutingProtocolHandler(const char* aScheme)
-    : mScheme(aScheme), mSubstitutions(16), mEnforceFileOrJar(true) {
+    : mScheme(aScheme),
+      mSubstitutionsLock("SubstitutingProtocolHandler::mSubstitutions"),
+      mSubstitutions(16),
+      mEnforceFileOrJar(true) {
   ConstructInternal();
 }
 
@@ -223,6 +227,7 @@ void SubstitutingProtocolHandler::ConstructInternal() {
 
 nsresult SubstitutingProtocolHandler::CollectSubstitutions(
     InfallibleTArray<SubstitutionMapping>& aMappings) {
+  AutoReadLock lock(mSubstitutionsLock);
   for (auto iter = mSubstitutions.ConstIter(); !iter.Done(); iter.Next()) {
     SubstitutionEntry& entry = iter.Data();
     nsCOMPtr<nsIURI> uri = entry.baseURI;
@@ -448,7 +453,10 @@ nsresult SubstitutingProtocolHandler::SetSubstitutionWithFlags(
   ToLowerCase(origRoot, root);
 
   if (!baseURI) {
-    mSubstitutions.Remove(root);
+    {
+      AutoWriteLock lock(mSubstitutionsLock);
+      mSubstitutions.Remove(root);
+    }
     NotifyObservers(root, baseURI);
     return SendSubstitution(root, baseURI, flags);
   }
@@ -466,9 +474,12 @@ nsresult SubstitutingProtocolHandler::SetSubstitutionWithFlags(
       return NS_ERROR_INVALID_ARG;
     }
 
-    SubstitutionEntry& entry = mSubstitutions.GetOrInsert(root);
-    entry.baseURI = baseURI;
-    entry.flags = flags;
+    {
+      AutoWriteLock lock(mSubstitutionsLock);
+      SubstitutionEntry& entry = mSubstitutions.GetOrInsert(root);
+      entry.baseURI = baseURI;
+      entry.flags = flags;
+    }
     NotifyObservers(root, baseURI);
     return SendSubstitution(root, baseURI, flags);
   }
@@ -483,9 +494,12 @@ nsresult SubstitutingProtocolHandler::SetSubstitutionWithFlags(
       mIOService->NewURI(newBase, nullptr, nullptr, getter_AddRefs(newBaseURI));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  SubstitutionEntry& entry = mSubstitutions.GetOrInsert(root);
-  entry.baseURI = newBaseURI;
-  entry.flags = flags;
+  {
+    AutoWriteLock lock(mSubstitutionsLock);
+    SubstitutionEntry& entry = mSubstitutions.GetOrInsert(root);
+    entry.baseURI = newBaseURI;
+    entry.flags = flags;
+  }
   NotifyObservers(root, baseURI);
   return SendSubstitution(root, newBaseURI, flags);
 }
@@ -497,11 +511,14 @@ nsresult SubstitutingProtocolHandler::GetSubstitution(
   nsAutoCString root;
   ToLowerCase(origRoot, root);
 
-  SubstitutionEntry entry;
-  if (mSubstitutions.Get(root, &entry)) {
-    nsCOMPtr<nsIURI> baseURI = entry.baseURI;
-    baseURI.forget(result);
-    return NS_OK;
+  {
+    AutoReadLock lock(mSubstitutionsLock);
+    SubstitutionEntry entry;
+    if (mSubstitutions.Get(root, &entry)) {
+      nsCOMPtr<nsIURI> baseURI = entry.baseURI;
+      baseURI.forget(result);
+      return NS_OK;
+    }
   }
 
   uint32_t flags;
@@ -518,10 +535,15 @@ nsresult SubstitutingProtocolHandler::GetSubstitutionFlags(
 #endif
 
   *flags = 0;
-  SubstitutionEntry entry;
-  if (mSubstitutions.Get(root, &entry)) {
-    *flags = entry.flags;
-    return NS_OK;
+
+  {
+    AutoReadLock lock(mSubstitutionsLock);
+
+    SubstitutionEntry entry;
+    if (mSubstitutions.Get(root, &entry)) {
+      *flags = entry.flags;
+      return NS_OK;
+    }
   }
 
   nsCOMPtr<nsIURI> baseURI;

netwerk/protocol/res/SubstitutingProtocolHandler.h

@@ -15,6 +15,7 @@
 #include "nsJARURI.h"
 #include "mozilla/chrome/RegistryMessageUtils.h"
 #include "mozilla/Maybe.h"
+#include "mozilla/RWLock.h"
 
 class nsIIOService;
 
@@ -32,11 +33,12 @@ class SubstitutingProtocolHandler {
                               bool aEnforceFileOrJar = true);
   explicit SubstitutingProtocolHandler(const char* aScheme);
 
-  NS_INLINE_DECL_REFCOUNTING(SubstitutingProtocolHandler);
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(SubstitutingProtocolHandler);
   NS_DECL_NON_VIRTUAL_NSIPROTOCOLHANDLER;
   NS_DECL_NON_VIRTUAL_NSISUBSTITUTINGPROTOCOLHANDLER;
 
   bool HasSubstitution(const nsACString& aRoot) const {
+    AutoReadLock lock(const_cast<RWLock&>(mSubstitutionsLock));
     return mSubstitutions.Get(aRoot, nullptr);
   }
 
@@ -100,6 +102,8 @@ class SubstitutingProtocolHandler {
 
   nsCString mScheme;
   Maybe<uint32_t> mFlags;
+
+  RWLock mSubstitutionsLock;
   nsDataHashtable<nsCStringHashKey, SubstitutionEntry> mSubstitutions;
   nsCOMPtr<nsIIOService> mIOService;