Bug 1722925 - Remove NS_MutatorMethod in favor of templated nsIURIMut…

…ator::Apply r=necko-reviewers,kershaw This basically reverts the changes in 5caa81103c00 (bug 1435671). In that bug we switched from having a templated method to using a templated function that returned a lambda because the templated method caused a binary size regression on windows (MSVC). Since Firefox 67 we no longer support MSVC. Using a lambda also required capturing the arguments by value, so it was slightly inefficient. This patch removes NS_MutatorMethod and makes the Apply method a template. This improves perfomance as we can just pass the arguments to the called function, without worrying about needing to copy them. Since MSVC is not supported anymore, and clang and gcc didn't report a binary size regression, this is a much better solution. Differential Revision: https://phabricator.services.mozilla.com/D122081
iaxat · Aug 12, 2021 · f9d039d · f9d039d
1 parent 0fe1ad3
commit f9d039d
Show file tree

Hide file tree

Showing 16 changed files with 71 additions and 103 deletions.
diff --git a/chrome/nsChromeProtocolHandler.cpp b/chrome/nsChromeProtocolHandler.cpp
@@ -68,11 +68,12 @@ nsChromeProtocolHandler::GetProtocolFlags(uint32_t* result) {
   nsresult rv;
   nsCOMPtr<nsIURI> surl;
   nsCOMPtr<nsIURI> base(aBaseURI);
-  rv = NS_MutateURI(new mozilla::net::nsStandardURL::Mutator())
-           .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                   nsIStandardURL::URLTYPE_STANDARD, -1,
-                                   nsCString(aSpec), aCharset, base, nullptr))
-           .Finalize(surl);
+  rv =
+      NS_MutateURI(new mozilla::net::nsStandardURL::Mutator())
+          .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_STANDARD,
+                 -1, nsCString(aSpec), aCharset, aBaseURI, nullptr)
+
+          .Finalize(surl);
   if (NS_FAILED(rv)) {
     return rv;
   }

diff --git a/dom/base/nsContentAreaDragDrop.cpp b/dom/base/nsContentAreaDragDrop.cpp
@@ -468,8 +468,8 @@ nsresult DragDataProducer::GetImageData(imgIContainer* aImage,
         mimeInfo->GetPrimaryExtension(primaryExtension);
         if (!primaryExtension.IsEmpty()) {
           rv = NS_MutateURI(imgUrl)
-                   .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension,
-                                           primaryExtension, nullptr))
+                   .Apply(&nsIURLMutator::SetFileExtension, primaryExtension,
+                          nullptr)
                    .Finalize(imgUrl);
           NS_ENSURE_SUCCESS(rv, rv);
         }

diff --git a/dom/base/nsCopySupport.cpp b/dom/base/nsCopySupport.cpp
@@ -646,8 +646,8 @@ static nsresult AppendImagePromise(nsITransferable* aTransferable,
     mimeInfo->GetPrimaryExtension(primaryExtension);
     if (!primaryExtension.IsEmpty()) {
       rv = NS_MutateURI(imgUri)
-               .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension,
-                                       primaryExtension, nullptr))
+               .Apply(&nsIURLMutator::SetFileExtension, primaryExtension,
+                      nullptr)
                .Finalize(imgUrl);
       NS_ENSURE_SUCCESS(rv, rv);
     }

diff --git a/dom/file/uri/BlobURLProtocolHandler.cpp b/dom/file/uri/BlobURLProtocolHandler.cpp
@@ -868,7 +868,7 @@ BlobURLProtocolHandler::GetFlagsForURI(nsIURI* aURI, uint32_t* aResult) {
 
   return NS_MutateURI(new BlobURL::Mutator())
       .SetSpec(aSpec)
-      .Apply(NS_MutatorMethod(&nsIBlobURLMutator::SetRevoked, revoked))
+      .Apply(&nsIBlobURLMutator::SetRevoked, revoked)
       .Finalize(aResult);
 }
 

diff --git a/dom/jsurl/nsJSProtocolHandler.cpp b/dom/jsurl/nsJSProtocolHandler.cpp
@@ -1150,7 +1150,7 @@ nsJSProtocolHandler::GetProtocolFlags(uint32_t* result) {
 
   NS_MutateURI mutator(new nsJSURI::Mutator());
   nsCOMPtr<nsIURI> base(aBaseURI);
-  mutator.Apply(NS_MutatorMethod(&nsIJSURIMutator::SetBase, base));
+  mutator.Apply(&nsIJSURIMutator::SetBase, base);
   if (!aCharset || !nsCRT::strcasecmp("UTF-8", aCharset)) {
     mutator.SetSpec(aSpec);
   } else {

diff --git a/dom/webbrowserpersist/nsWebBrowserPersist.cpp b/dom/webbrowserpersist/nsWebBrowserPersist.cpp
@@ -2064,11 +2064,11 @@ nsresult nsWebBrowserPersist::CalculateUniqueFilename(
 
       // Resync the URI with the file after the extension has been appended
       return NS_MutateURI(aURI)
-          .Apply(NS_MutatorMethod(&nsIFileURLMutator::SetFile, localFile))
+          .Apply(&nsIFileURLMutator::SetFile, localFile)
           .Finalize(aOutURI);
     }
     return NS_MutateURI(url)
-        .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileName, filename, nullptr))
+        .Apply(&nsIURLMutator::SetFileName, filename, nullptr)
         .Finalize(aOutURI);
   }
 
@@ -2217,12 +2217,11 @@ nsresult nsWebBrowserPersist::CalculateAndAppendFileExt(
 
           // Resync the URI with the file after the extension has been appended
           return NS_MutateURI(url)
-              .Apply(NS_MutatorMethod(&nsIFileURLMutator::SetFile, localFile))
+              .Apply(&nsIFileURLMutator::SetFile, localFile)
               .Finalize(aOutURI);
         }
         return NS_MutateURI(url)
-            .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileName, newFileName,
-                                    nullptr))
+            .Apply(&nsIURLMutator::SetFileName, newFileName, nullptr)
             .Finalize(aOutURI);
       }
     }

diff --git a/modules/libjar/nsJARURI.cpp b/modules/libjar/nsJARURI.cpp
@@ -84,9 +84,8 @@ nsresult nsJARURI::CreateEntryURL(const nsACString& entryFilename,
   // Flatten the concatenation, just in case.  See bug 128288
   nsAutoCString spec(NS_BOGUS_ENTRY_SCHEME + entryFilename);
   return NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID)
-      .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                              nsIStandardURL::URLTYPE_NO_AUTHORITY, -1, spec,
-                              charset, nullptr, nullptr))
+      .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_NO_AUTHORITY,
+             -1, spec, charset, nullptr, nullptr)
       .Finalize(url);
 }
 
@@ -252,10 +251,9 @@ nsresult nsJARURI::SetSpecWithBase(const nsACString& aSpec, nsIURI* aBaseURL) {
     nsCOMPtr<nsIURI> entry;
 
     rv = NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID)
-             .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                     nsIStandardURL::URLTYPE_NO_AUTHORITY, -1,
-                                     nsCString(aSpec), mCharsetHint.get(),
-                                     otherJAR->mJAREntry, nullptr))
+             .Apply(&nsIStandardURLMutator::Init,
+                    nsIStandardURL::URLTYPE_NO_AUTHORITY, -1, nsCString(aSpec),
+                    mCharsetHint.get(), otherJAR->mJAREntry, nullptr)
              .Finalize(entry);
     if (NS_FAILED(rv)) {
       return rv;
@@ -515,8 +513,7 @@ nsJARURI::GetFileName(nsACString& fileName) {
 
 nsresult nsJARURI::SetFileNameInternal(const nsACString& fileName) {
   return NS_MutateURI(mJAREntry)
-      .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileName, nsCString(fileName),
-                              nullptr))
+      .Apply(&nsIURLMutator::SetFileName, nsCString(fileName), nullptr)
       .Finalize(mJAREntry);
 }
 
@@ -527,8 +524,7 @@ nsJARURI::GetFileBaseName(nsACString& fileBaseName) {
 
 nsresult nsJARURI::SetFileBaseNameInternal(const nsACString& fileBaseName) {
   return NS_MutateURI(mJAREntry)
-      .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileBaseName,
-                              nsCString(fileBaseName), nullptr))
+      .Apply(&nsIURLMutator::SetFileBaseName, nsCString(fileBaseName), nullptr)
       .Finalize(mJAREntry);
 }
 
@@ -539,8 +535,8 @@ nsJARURI::GetFileExtension(nsACString& fileExtension) {
 
 nsresult nsJARURI::SetFileExtensionInternal(const nsACString& fileExtension) {
   return NS_MutateURI(mJAREntry)
-      .Apply(NS_MutatorMethod(&nsIURLMutator::SetFileExtension,
-                              nsCString(fileExtension), nullptr))
+      .Apply(&nsIURLMutator::SetFileExtension, nsCString(fileExtension),
+             nullptr)
       .Finalize(mJAREntry);
 }
 

diff --git a/netwerk/base/nsIURIMutator.idl b/netwerk/base/nsIURIMutator.idl
@@ -11,7 +11,7 @@ interface nsIURIMutator;
 %{C++
 #include "nsString.h"
 #include "nsCOMPtr.h"
-#include <functional>
+#include <utility>
 
 #undef SetPort  // XXX Windows!
 
@@ -301,7 +301,6 @@ interface nsIURIMutator : nsIURISetters
 %{C++
 
 // This templated struct is used to extract the class type of the method
-// passed to NS_MutatorMethod.
 template <typename Method>
 struct nsMethodTypeTraits;
 
@@ -319,25 +318,6 @@ struct nsMethodTypeTraits<R(__stdcall C::*)(As...)>
 };
 #endif
 
-// This helper returns a std::function that will be applied on the
-// nsIURIMutator. The type of `Interface` will be deduced from the method type.
-// aMethod will be called on the target object if it successfully QIs to
-// `Interface`, and the arguments will be passed to the call.
-template <typename Method, typename... Args>
-const std::function<nsresult(nsIURIMutator*)>
-NS_MutatorMethod(Method aMethod, Args ...aArgs)
-{
-  // Capture arguments by value, otherwise we crash.
-  return [=](nsIURIMutator* aMutator) {
-    typedef typename nsMethodTypeTraits<Method>::class_type Interface;
-    nsresult rv;
-    nsCOMPtr<Interface> target = do_QueryInterface(aMutator, &rv);
-    NS_ENSURE_SUCCESS(rv, rv);
-    rv = (target->*aMethod)(aArgs...);
-    if (NS_FAILED(rv)) return rv;
-    return NS_OK;
-  };
-}
 
 // This class provides a useful helper that allows chaining of setter operations
 class MOZ_STACK_CLASS NS_MutateURI
@@ -477,20 +457,23 @@ public:
    * nsCOMPtr<nsIURI> uri;
    * nsresult rv = NS_MutateURI(new URIClass::Mutator())
    *                 .SetSpec(aSpec)
-   *                 .Apply(NS_MutatorMethod(&SomeInterface::Method, arg1, arg2))
+   *                 .Apply(&SomeInterface::Method, arg1, arg2)
    *                 .Finalize(uri);
    *
    * If mMutator does not implement SomeInterface, do_QueryInterface will fail
    * and the method will not be called.
    * If aMethod does not exist, or if there is a mismatch between argument
    * types, or the number of arguments, then there will be a compile error.
    */
-  NS_MutateURI& Apply(const std::function<nsresult(nsIURIMutator*)>& aFunction)
+  template <typename Method, typename... Args>
+  NS_MutateURI& Apply(Method aMethod, Args&&... aArgs)
   {
-    if (NS_FAILED(mStatus)) {
-      return *this;
-    }
-    mStatus = aFunction(mMutator);
+    typedef typename nsMethodTypeTraits<Method>::class_type Interface;
+    NS_ENSURE_SUCCESS(mStatus, *this);
+    nsCOMPtr<Interface> target = do_QueryInterface(mMutator, &mStatus);
+    MOZ_ASSERT(NS_SUCCEEDED(mStatus), "URL object must implement interface");
+    NS_ENSURE_SUCCESS(mStatus, *this);
+    mStatus = (target->*aMethod)(std::forward<Args>(aArgs)...);
     return *this;
   }
 

diff --git a/netwerk/base/nsNetUtil.cpp b/netwerk/base/nsNetUtil.cpp
@@ -1732,9 +1732,8 @@ static nsresult NewStandardURI(const nsACString& aSpec, const char* aCharset,
                                nsIURI** aURI) {
   nsCOMPtr<nsIURI> base(aBaseURI);
   return NS_MutateURI(new nsStandardURL::Mutator())
-      .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                              nsIStandardURL::URLTYPE_AUTHORITY, aDefaultPort,
-                              nsCString(aSpec), aCharset, base, nullptr))
+      .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_AUTHORITY,
+             aDefaultPort, nsCString(aSpec), aCharset, base, nullptr)
       .Finalize(aURI);
 }
 
@@ -1819,10 +1818,10 @@ nsresult NS_NewURI(nsIURI** aURI, const nsACString& aSpec,
 
     nsCOMPtr<nsIURI> base(aBaseURI);
     return NS_MutateURI(new nsStandardURL::Mutator())
-        .Apply(NS_MutatorMethod(&nsIFileURLMutator::MarkFileURL))
-        .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                nsIStandardURL::URLTYPE_NO_AUTHORITY, -1, buf,
-                                aCharset, base, nullptr))
+        .Apply(&nsIFileURLMutator::MarkFileURL)
+        .Apply(&nsIStandardURLMutator::Init,
+               nsIStandardURL::URLTYPE_NO_AUTHORITY, -1, buf, aCharset, base,
+               nullptr)
         .Finalize(aURI);
   }
 
@@ -1868,9 +1867,8 @@ nsresult NS_NewURI(nsIURI** aURI, const nsACString& aSpec,
   if (scheme.EqualsLiteral("indexeddb")) {
     nsCOMPtr<nsIURI> base(aBaseURI);
     return NS_MutateURI(new nsStandardURL::Mutator())
-        .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                nsIStandardURL::URLTYPE_AUTHORITY, 0,
-                                nsCString(aSpec), aCharset, base, nullptr))
+        .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_AUTHORITY,
+               0, nsCString(aSpec), aCharset, base, nullptr)
         .Finalize(aURI);
   }
 
@@ -1907,8 +1905,8 @@ nsresult NS_NewURI(nsIURI** aURI, const nsACString& aSpec,
   if (scheme.EqualsLiteral("jar")) {
     nsCOMPtr<nsIURI> base(aBaseURI);
     return NS_MutateURI(new nsJARURI::Mutator())
-        .Apply(NS_MutatorMethod(&nsIJARURIMutator::SetSpecBaseCharset,
-                                nsCString(aSpec), base, aCharset))
+        .Apply(&nsIJARURIMutator::SetSpecBaseCharset, nsCString(aSpec), base,
+               aCharset)
         .Finalize(aURI);
   }
 
@@ -1922,19 +1920,17 @@ nsresult NS_NewURI(nsIURI** aURI, const nsACString& aSpec,
   if (scheme.EqualsLiteral("smb") || scheme.EqualsLiteral("sftp")) {
     nsCOMPtr<nsIURI> base(aBaseURI);
     return NS_MutateURI(new nsStandardURL::Mutator())
-        .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                nsIStandardURL::URLTYPE_STANDARD, -1,
-                                nsCString(aSpec), aCharset, base, nullptr))
+        .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_STANDARD,
+               -1, nsCString(aSpec), aCharset, base, nullptr)
         .Finalize(aURI);
   }
 #endif
 
   if (scheme.EqualsLiteral("android")) {
     nsCOMPtr<nsIURI> base(aBaseURI);
     return NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID)
-        .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                nsIStandardURL::URLTYPE_STANDARD, -1,
-                                nsCString(aSpec), aCharset, base, nullptr))
+        .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_STANDARD,
+               -1, nsCString(aSpec), aCharset, base, nullptr)
         .Finalize(aURI);
   }
 
@@ -3039,8 +3035,7 @@ nsresult NS_GetSecureUpgradedURI(nsIURI* aURI, nsIURI** aUpgradedURI) {
   // Change the default port to 443:
   nsCOMPtr<nsIStandardURL> stdURL = do_QueryInterface(aURI);
   if (stdURL) {
-    mutator.Apply(
-        NS_MutatorMethod(&nsIStandardURLMutator::SetDefaultPort, 443, nullptr));
+    mutator.Apply(&nsIStandardURLMutator::SetDefaultPort, 443, nullptr);
   } else {
     // If we don't have a nsStandardURL, fall back to using GetPort/SetPort.
     // XXXdholbert Is this function even called with a non-nsStandardURL arg,

diff --git a/netwerk/dns/TRRService.cpp b/netwerk/dns/TRRService.cpp
@@ -311,12 +311,11 @@ bool TRRService::MaybeSetPrivateURI(const nsACString& aURI) {
     }
 
     nsCOMPtr<nsIURI> url;
-    nsresult rv =
-        NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID)
-            .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                    nsIStandardURL::URLTYPE_STANDARD, 443,
-                                    newURI, nullptr, nullptr, nullptr))
-            .Finalize(url);
+    nsresult rv = NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID)
+                      .Apply(&nsIStandardURLMutator::Init,
+                             nsIStandardURL::URLTYPE_STANDARD, 443, newURI,
+                             nullptr, nullptr, nullptr)
+                      .Finalize(url);
     if (NS_FAILED(rv)) {
       LOG(("TRRService::MaybeSetPrivateURI failed to create URI!\n"));
       return false;
@@ -865,9 +864,8 @@ bool TRRService::MaybeBootstrap(const nsACString& aPossible,
   nsCOMPtr<nsIURI> url;
   nsresult rv =
       NS_MutateURI(NS_STANDARDURLMUTATOR_CONTRACTID)
-          .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                  nsIStandardURL::URLTYPE_STANDARD, 443,
-                                  mPrivateURI, nullptr, nullptr, nullptr))
+          .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_STANDARD,
+                 443, mPrivateURI, nullptr, nullptr, nullptr)
           .Finalize(url);
   if (NS_FAILED(rv)) {
     LOG(("TRRService::MaybeBootstrap failed to create URI!\n"));

diff --git a/netwerk/protocol/about/nsAboutProtocolHandler.cpp b/netwerk/protocol/about/nsAboutProtocolHandler.cpp
@@ -149,10 +149,8 @@ nsresult nsAboutProtocolHandler::CreateNewURI(const nsACString& aSpec,
     rv = NS_NewURI(getter_AddRefs(inner), spec);
     NS_ENSURE_SUCCESS(rv, rv);
 
-    nsCOMPtr<nsIURI> base(aBaseURI);
     rv = NS_MutateURI(new nsNestedAboutURI::Mutator())
-             .Apply(NS_MutatorMethod(&nsINestedAboutURIMutator::InitWithBase,
-                                     inner, base))
+             .Apply(&nsINestedAboutURIMutator::InitWithBase, inner, aBaseURI)
              .SetSpec(aSpec)
              .Finalize(url);
     NS_ENSURE_SUCCESS(rv, rv);

diff --git a/netwerk/protocol/data/nsDataHandler.cpp b/netwerk/protocol/data/nsDataHandler.cpp
@@ -74,9 +74,8 @@ nsDataHandler::GetProtocolFlags(uint32_t* result) {
                    contentType.Find("xml") == kNotFound)) {
       // it's ascii encoded binary, don't let any spaces in
       rv = NS_MutateURI(new mozilla::net::nsSimpleURI::Mutator())
-               .Apply(NS_MutatorMethod(
-                   &nsISimpleURIMutator::SetSpecAndFilterWhitespace, spec,
-                   nullptr))
+               .Apply(&nsISimpleURIMutator::SetSpecAndFilterWhitespace, spec,
+                      nullptr)
                .Finalize(uri);
     } else {
       rv = NS_MutateURI(new mozilla::net::nsSimpleURI::Mutator())

diff --git a/netwerk/protocol/file/nsFileProtocolHandler.cpp b/netwerk/protocol/file/nsFileProtocolHandler.cpp
@@ -225,7 +225,7 @@ nsFileProtocolHandler::NewFileURI(nsIFile* aFile, nsIURI** aResult) {
   // NOTE: the origin charset is assigned the value of the platform
   // charset by the SetFile method.
   return NS_MutateURI(new nsStandardURL::Mutator())
-      .Apply(NS_MutatorMethod(&nsIFileURLMutator::SetFile, file))
+      .Apply(&nsIFileURLMutator::SetFile, file)
       .Finalize(aResult);
 }
 

diff --git a/netwerk/protocol/http/Http2Stream.cpp b/netwerk/protocol/http/Http2Stream.cpp
@@ -359,11 +359,10 @@ nsresult Http2Stream::MakeOriginURL(const nsACString& scheme,
                                     const nsACString& origin,
                                     nsCOMPtr<nsIURI>& url) {
   return NS_MutateURI(new nsStandardURL::Mutator())
-      .Apply(NS_MutatorMethod(
-          &nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_AUTHORITY,
-          scheme.EqualsLiteral("http") ? NS_HTTP_DEFAULT_PORT
-                                       : NS_HTTPS_DEFAULT_PORT,
-          nsCString(origin), nullptr, nullptr, nullptr))
+      .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_AUTHORITY,
+             scheme.EqualsLiteral("http") ? NS_HTTP_DEFAULT_PORT
+                                          : NS_HTTPS_DEFAULT_PORT,
+             nsCString(origin), nullptr, nullptr, nullptr)
       .Finalize(url);
 }
 

diff --git a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
@@ -351,11 +351,11 @@ nsresult SubstitutingProtocolHandler::NewURI(const nsACString& aSpec,
 
   nsCOMPtr<nsIURI> base(aBaseURI);
   nsCOMPtr<nsIURL> uri;
-  rv = NS_MutateURI(new SubstitutingURL::Mutator())
-           .Apply(NS_MutatorMethod(&nsIStandardURLMutator::Init,
-                                   nsIStandardURL::URLTYPE_STANDARD, -1, spec,
-                                   aCharset, base, nullptr))
-           .Finalize(uri);
+  rv =
+      NS_MutateURI(new SubstitutingURL::Mutator())
+          .Apply(&nsIStandardURLMutator::Init, nsIStandardURL::URLTYPE_STANDARD,
+                 -1, spec, aCharset, base, nullptr)
+          .Finalize(uri);
   if (NS_FAILED(rv)) return rv;
 
   nsAutoCString host;