Bug 1453633 - Make nsHostObjectURI hold its principal wrapped in a nsMainThreadPtrHandle r=baku
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 12 Apr 2018 22:37:41 +0200
changeset 413065 165ff300ac9589a505104f4507cd91ae78bf6e7e
parent 413064 6ea3c1db006049ff57f392c7042db0a91ec9b10b
child 413066 5159724ccfc68cca3ed717a5fd9765cb483a079d
push id33833
push useraiakab@mozilla.com
push dateFri, 13 Apr 2018 09:41:15 +0000
treeherdermozilla-central@260e4c83c8a9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku
bugs1453633, 1443925
milestone61.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 1453633 - Make nsHostObjectURI hold its principal wrapped in a nsMainThreadPtrHandle r=baku nsIPrincipal is not yet threadsafe (bug 1443925). This is a problem in the context of threadsafe nsIURI, where cloning an nsIURI would also AddRef the principal. To get around this problem, we use nsMainThreadPtrHandle<nsIPrincipal>. This means cloning the URI would not AddRef the principal (it addrefs the nsMainThreadPtrHolder that holds the principal). When the last ref is dropped, the principal is released on the main thread. We should get rid of this once principals become thread-safe MozReview-Commit-ID: AbJEhTNXVv6
dom/file/nsHostObjectURI.cpp
dom/file/nsHostObjectURI.h
--- a/dom/file/nsHostObjectURI.cpp
+++ b/dom/file/nsHostObjectURI.cpp
@@ -35,17 +35,19 @@ NS_INTERFACE_MAP_BEGIN(nsHostObjectURI)
   else
 NS_INTERFACE_MAP_END_INHERITING(mozilla::net::nsSimpleURI)
 
 // nsIURIWithPrincipal methods:
 
 NS_IMETHODIMP
 nsHostObjectURI::GetPrincipal(nsIPrincipal** aPrincipal)
 {
-  NS_IF_ADDREF(*aPrincipal = mPrincipal);
+  MOZ_ASSERT(NS_IsMainThread());
+  nsCOMPtr<nsIPrincipal> principal = mPrincipal.get();
+  principal.forget(aPrincipal);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsHostObjectURI::GetPrincipalUri(nsIURI** aUri)
 {
   if (mPrincipal) {
@@ -72,46 +74,49 @@ nsHostObjectURI::ReadPrivate(nsIObjectIn
 {
   nsresult rv = mozilla::net::nsSimpleURI::ReadPrivate(aStream);
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsCOMPtr<nsISupports> supports;
   rv = NS_ReadOptionalObject(aStream, true, getter_AddRefs(supports));
   NS_ENSURE_SUCCESS(rv, rv);
 
-  mPrincipal = do_QueryInterface(supports, &rv);
+  nsCOMPtr<nsIPrincipal> principal = do_QueryInterface(supports, &rv);
+  mPrincipal = new nsMainThreadPtrHolder<nsIPrincipal>("nsIPrincipal", principal, false);
   return rv;
 }
 
 NS_IMETHODIMP
 nsHostObjectURI::Write(nsIObjectOutputStream* aStream)
 {
   nsresult rv = mozilla::net::nsSimpleURI::Write(aStream);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  return NS_WriteOptionalCompoundObject(aStream, mPrincipal,
+  nsCOMPtr<nsIPrincipal> principal = mPrincipal.get();
+  return NS_WriteOptionalCompoundObject(aStream, principal,
                                         NS_GET_IID(nsIPrincipal),
                                         true);
 }
 
 // nsIIPCSerializableURI methods:
 void
 nsHostObjectURI::Serialize(mozilla::ipc::URIParams& aParams)
 {
   using namespace mozilla::ipc;
 
   HostObjectURIParams hostParams;
   URIParams simpleParams;
 
   mozilla::net::nsSimpleURI::Serialize(simpleParams);
   hostParams.simpleParams() = simpleParams;
 
-  if (mPrincipal) {
+  nsCOMPtr<nsIPrincipal> principal = mPrincipal.get();
+  if (principal) {
     PrincipalInfo info;
-    nsresult rv = PrincipalToPrincipalInfo(mPrincipal, &info);
+    nsresult rv = PrincipalToPrincipalInfo(principal, &info);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return;
     }
 
     hostParams.principal() = info;
   } else {
     hostParams.principal() = mozilla::void_t();
   }
@@ -134,20 +139,21 @@ nsHostObjectURI::Deserialize(const mozil
   if (!mozilla::net::nsSimpleURI::Deserialize(hostParams.simpleParams())) {
     return false;
   }
 
   if (hostParams.principal().type() == OptionalPrincipalInfo::Tvoid_t) {
     return true;
   }
 
-  mPrincipal = PrincipalInfoToPrincipal(hostParams.principal().get_PrincipalInfo());
-  if (!mPrincipal) {
+  nsCOMPtr<nsIPrincipal> principal = PrincipalInfoToPrincipal(hostParams.principal().get_PrincipalInfo());
+  if (!principal) {
     return false;
   }
+  mPrincipal = new nsMainThreadPtrHolder<nsIPrincipal>("nsIPrincipal", principal, false);
 
   return true;
 }
 
 nsresult
 nsHostObjectURI::SetScheme(const nsACString& aScheme)
 {
   // Disallow setting the scheme, since that could cause us to be associated
--- a/dom/file/nsHostObjectURI.h
+++ b/dom/file/nsHostObjectURI.h
@@ -11,32 +11,33 @@
 #include "mozilla/dom/File.h"
 #include "nsCOMPtr.h"
 #include "nsIClassInfo.h"
 #include "nsIPrincipal.h"
 #include "nsISerializable.h"
 #include "nsIURIWithPrincipal.h"
 #include "nsSimpleURI.h"
 #include "nsIIPCSerializableURI.h"
-
+#include "nsProxyRelease.h"
 
 /**
  * These URIs refer to host objects: Blobs, with scheme "blob",
  * MediaStreams, with scheme "mediastream", and MediaSources, with scheme
  * "mediasource".
  */
 class nsHostObjectURI final
   : public mozilla::net::nsSimpleURI
   , public nsIURIWithPrincipal
 {
 private:
   explicit nsHostObjectURI(nsIPrincipal* aPrincipal)
     : mozilla::net::nsSimpleURI()
-    , mPrincipal(aPrincipal)
-  {}
+  {
+    mPrincipal = new nsMainThreadPtrHolder<nsIPrincipal>("nsIPrincipal", aPrincipal, false);
+  }
 
   // For use only from deserialization
   explicit nsHostObjectURI()
     : mozilla::net::nsSimpleURI()
   {}
 
 public:
   NS_DECL_ISUPPORTS_INHERITED
@@ -59,17 +60,17 @@ public:
   {
     nsHostObjectURI* url = new nsHostObjectURI();
     SetRefOnClone(url, refHandlingMode, newRef);
     return url;
   }
 
   NS_IMETHOD Mutate(nsIURIMutator * *_retval) override;
 
-  nsCOMPtr<nsIPrincipal> mPrincipal;
+  nsMainThreadPtrHandle<nsIPrincipal> mPrincipal;
 
 protected:
   virtual ~nsHostObjectURI() {}
 
   nsresult SetScheme(const nsACString &aProtocol) override;
   bool Deserialize(const mozilla::ipc::URIParams&);
   nsresult ReadPrivate(nsIObjectInputStream *stream);
 
@@ -97,17 +98,18 @@ public:
     }
 
     MOZ_MUST_USE NS_IMETHOD
     SetPrincipal(nsIPrincipal *aPrincipal) override
     {
         if (!mURI) {
             return NS_ERROR_NULL_POINTER;
         }
-        mURI->mPrincipal = aPrincipal;
+        MOZ_ASSERT(NS_IsMainThread());
+        mURI->mPrincipal = new nsMainThreadPtrHolder<nsIPrincipal>("nsIPrincipal", aPrincipal, false);
         return NS_OK;
     }
 
     explicit Mutator() { }
   private:
     virtual ~Mutator() { }
 
     friend class nsHostObjectURI;