Bug 1696261 - Optimize nsDataHandler and nsSimpleURI::SetSpec to do fewer passes r=necko-reviewers,kershaw

- Rename nsAciiMask.h details:: namespace to asciimask_details so it doesn't clash with ipc/chromium/src/base/task.h
- Add SetSpecAndFilterWhitespace simple URI constructor that filters whitespace instead of just CR/LF.
- Only do one scan of the string in nsSimpleURI::SetPathQueryRefInternal in order to find the end of the path, query & ref.

There are probably more optimizations possible.
In my testing these get me a 1.5x-2x speedup.

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

netwerk/base/moz.build

@@ -98,6 +98,7 @@ XPIDL_SOURCES += [
     "nsISerializationHelper.idl",
     "nsIServerSocket.idl",
     "nsISimpleStreamListener.idl",
+    "nsISimpleURIMutator.idl",
     "nsISocketFilter.idl",
     "nsISocketTransport.idl",
     "nsISocketTransportService.idl",

netwerk/base/nsISimpleURIMutator.idl

@@ -0,0 +1,15 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "nsISupports.idl"
+interface nsIURIMutator;
+
+[scriptable, builtinclass, uuid(e055bddd-f3c2-404b-adec-db9304e93be2)]
+interface nsISimpleURIMutator : nsISupports
+{
+    /**
+     * Same behaviour as nsIURISetSpec.setSpec() but filters whitespace.
+     */
+    nsIURIMutator setSpecAndFilterWhitespace(in AUTF8String aSpec);
+};

netwerk/base/nsSimpleURI.cpp

@@ -61,9 +61,9 @@ NS_INTERFACE_TABLE_HEAD(nsSimpleURI)
   NS_INTERFACE_TABLE(nsSimpleURI, nsIURI, nsISerializable)
   NS_INTERFACE_TABLE_TO_MAP_SEGUE
   NS_IMPL_QUERY_CLASSINFO(nsSimpleURI)
-  if (aIID.Equals(kThisSimpleURIImplementationCID))
+  if (aIID.Equals(kThisSimpleURIImplementationCID)) {
     foundInterface = static_cast<nsIURI*>(this);
-  else
+  } else
     NS_INTERFACE_MAP_ENTRY(nsISizeOf)
 NS_INTERFACE_MAP_END
 
@@ -265,14 +265,18 @@ nsSimpleURI::GetHasRef(bool* result) {
   return NS_OK;
 }
 
-nsresult nsSimpleURI::SetSpecInternal(const nsACString& aSpec) {
+nsresult nsSimpleURI::SetSpecInternal(const nsACString& aSpec,
+                                      bool aStripWhitespace) {
   nsresult rv = net_ExtractURLScheme(aSpec, mScheme);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
   nsAutoCString spec;
-  rv = net_FilterAndEscapeURI(aSpec, esc_OnlyNonASCII, spec);
+  rv = net_FilterAndEscapeURI(
+      aSpec, esc_OnlyNonASCII,
+      aStripWhitespace ? ASCIIMask::MaskWhitespace() : ASCIIMask::MaskCRLFTab(),
+      spec);
   if (NS_FAILED(rv)) {
     return rv;
   }
@@ -280,8 +284,7 @@ nsresult nsSimpleURI::SetSpecInternal(const nsACString& aSpec) {
   int32_t colonPos = spec.FindChar(':');
   MOZ_ASSERT(colonPos != kNotFound, "A colon should be in this string");
   // This sets mPath, mQuery and mRef.
-  return SetPathQueryRefEscaped(Substring(spec, colonPos + 1),
-                                /* aNeedsEscape = */ false);
+  return SetPathQueryRefInternal(Substring(spec, colonPos + 1));
 }
 
 NS_IMETHODIMP
@@ -375,48 +378,22 @@ nsSimpleURI::GetPathQueryRef(nsACString& result) {
 }
 
 nsresult nsSimpleURI::SetPathQueryRef(const nsACString& aPath) {
-  return SetPathQueryRefEscaped(aPath, true);
-}
-nsresult nsSimpleURI::SetPathQueryRefEscaped(const nsACString& aPath,
-                                             bool aNeedsEscape) {
-  nsresult rv;
   nsAutoCString path;
-  if (aNeedsEscape) {
-    rv = NS_EscapeURL(aPath, esc_OnlyNonASCII, path, fallible);
-    if (NS_FAILED(rv)) {
-      return rv;
-    }
-  } else {
-    if (!path.Assign(aPath, fallible)) {
-      return NS_ERROR_OUT_OF_MEMORY;
-    }
+  nsresult rv = NS_EscapeURL(aPath, esc_OnlyNonASCII, path, fallible);
+  if (NS_FAILED(rv)) {
+    return rv;
   }
+  return SetPathQueryRefInternal(path);
+}
 
-  int32_t queryPos = path.FindChar('?');
-  int32_t hashPos = path.FindChar('#');
+nsresult nsSimpleURI::SetPathQueryRefInternal(const nsACString& aPath) {
+  nsresult rv;
+  const auto* start = aPath.BeginReading();
+  const auto* end = aPath.EndReading();
 
-  if (queryPos != kNotFound && hashPos != kNotFound && hashPos < queryPos) {
-    queryPos = kNotFound;
-  }
-
-  nsAutoCString query;
-  if (queryPos != kNotFound) {
-    query.Assign(Substring(path, queryPos));
-    path.Truncate(queryPos);
-  }
-
-  nsAutoCString hash;
-  if (hashPos != kNotFound) {
-    if (query.IsEmpty()) {
-      hash.Assign(Substring(path, hashPos));
-      path.Truncate(hashPos);
-    } else {
-      // We have to search the hash character in the query
-      hashPos = query.FindChar('#');
-      hash.Assign(Substring(query, hashPos));
-      query.Truncate(hashPos);
-    }
-  }
+  // Find the first instance of ? or # that marks the end of the path.
+  auto hashOrQueryFilter = [](char c) { return c == '?' || c == '#'; };
+  const auto* pathEnd = std::find_if(start, end, hashOrQueryFilter);
 
   mIsQueryValid = false;
   mQuery.Truncate();
@@ -425,16 +402,27 @@ nsresult nsSimpleURI::SetPathQueryRefEscaped(const nsACString& aPath,
   mRef.Truncate();
 
   // The path
-  if (!mPath.Assign(path, fallible)) {
+  if (!mPath.Assign(Substring(start, pathEnd), fallible)) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
-  rv = SetQuery(query);
+  if (pathEnd == end) {
+    return NS_OK;
+  }
+
+  const auto* queryEnd =
+      std::find_if(pathEnd, end, [](char c) { return c == '#'; });
+
+  rv = SetQuery(Substring(pathEnd, queryEnd));
   if (NS_FAILED(rv)) {
     return rv;
   }
 
-  return SetRef(hash);
+  if (queryEnd == end) {
+    return NS_OK;
+  }
+
+  return SetRef(Substring(queryEnd, end));
 }
 
 NS_IMETHODIMP
@@ -535,7 +523,7 @@ nsSimpleURI::SchemeIs(const char* i_Scheme, bool* o_Equals) {
 
   // mScheme is guaranteed to be lower case.
   if (*i_Scheme == *this_scheme || *i_Scheme == (*this_scheme - ('a' - 'A'))) {
-    *o_Equals = PL_strcasecmp(this_scheme, i_Scheme) ? false : true;
+    *o_Equals = PL_strcasecmp(this_scheme, i_Scheme) == 0;
   } else {
     *o_Equals = false;
   }
@@ -727,7 +715,8 @@ nsresult nsSimpleURI::SetQueryWithEncoding(const nsACString& aQuery,
 
 // Queries this list of interfaces. If none match, it queries mURI.
 NS_IMPL_NSIURIMUTATOR_ISUPPORTS(nsSimpleURI::Mutator, nsIURISetters,
-                                nsIURIMutator, nsISerializable)
+                                nsIURIMutator, nsISerializable,
+                                nsISimpleURIMutator)
 
 NS_IMETHODIMP
 nsSimpleURI::Mutate(nsIURIMutator** aMutator) {

netwerk/base/nsSimpleURI.h

@@ -13,6 +13,7 @@
 #include "nsIClassInfo.h"
 #include "nsISizeOf.h"
 #include "nsIURIMutator.h"
+#include "nsISimpleURIMutator.h"
 
 namespace mozilla {
 namespace net {
@@ -55,7 +56,8 @@ class nsSimpleURI : public nsIURI, public nsISerializable, public nsISizeOf {
   enum RefHandlingEnum { eIgnoreRef, eHonorRef, eReplaceRef };
 
   virtual nsresult Clone(nsIURI** aURI);
-  virtual nsresult SetSpecInternal(const nsACString& input);
+  virtual nsresult SetSpecInternal(const nsACString& aSpec,
+                                   bool aStripWhitespace = false);
   virtual nsresult SetScheme(const nsACString& input);
   virtual nsresult SetUserPass(const nsACString& input);
   nsresult SetUsername(const nsACString& input);
@@ -96,7 +98,8 @@ class nsSimpleURI : public nsIURI, public nsISerializable, public nsISizeOf {
   virtual nsresult CloneInternal(RefHandlingEnum refHandlingMode,
                                  const nsACString& newRef, nsIURI** clone);
 
-  nsresult SetPathQueryRefEscaped(const nsACString& aPath, bool aNeedsEscape);
+  nsresult EscapeAndSetPathQueryRef(const nsACString& aPath);
+  nsresult SetPathQueryRefInternal(const nsACString& aPath);
 
   bool Deserialize(const mozilla::ipc::URIParams&);
 
@@ -112,6 +115,7 @@ class nsSimpleURI : public nsIURI, public nsISerializable, public nsISizeOf {
  public:
   class Mutator final : public nsIURIMutator,
                         public BaseURIMutator<nsSimpleURI>,
+                        public nsISimpleURIMutator,
                         public nsISerializable {
     NS_DECL_ISUPPORTS
     NS_FORWARD_SAFE_NSIURISETTERS_RET(mURI)
@@ -126,6 +130,22 @@ class nsSimpleURI : public nsIURI, public nsISerializable, public nsISizeOf {
       return InitFromInputStream(aStream);
     }
 
+    [[nodiscard]] NS_IMETHOD SetSpecAndFilterWhitespace(
+        const nsACString& aSpec, nsIURIMutator** aMutator) override {
+      if (aMutator) {
+        *aMutator = do_AddRef(this).take();
+      }
+
+      nsresult rv = NS_OK;
+      RefPtr<nsSimpleURI> uri = new nsSimpleURI();
+      rv = uri->SetSpecInternal(aSpec, /* filterWhitespace */ true);
+      if (NS_FAILED(rv)) {
+        return rv;
+      }
+      mURI = std::move(uri);
+      return NS_OK;
+    }
+
     explicit Mutator() = default;
 
    private:

netwerk/base/nsURLHelper.cpp

@@ -472,6 +472,7 @@ void net_FilterURIString(const nsACString& input, nsACString& result) {
 }
 
 nsresult net_FilterAndEscapeURI(const nsACString& aInput, uint32_t aFlags,
+                                const ASCIIMaskArray& aFilterMask,
                                 nsACString& aResult) {
   aResult.Truncate();
 
@@ -487,9 +488,8 @@ nsresult net_FilterAndEscapeURI(const nsACString& aInput, uint32_t aFlags,
                    charFilter)
           .base();
 
-  const ASCIIMaskArray& mask = ASCIIMask::MaskCRLFTab();
-  return NS_EscapeAndFilterURL(Substring(newStart, newEnd), aFlags, &mask,
-                               aResult, fallible);
+  return NS_EscapeAndFilterURL(Substring(newStart, newEnd), aFlags,
+                               &aFilterMask, aResult, fallible);
 }
 
 #if defined(XP_WIN)

netwerk/base/nsURLHelper.h

@@ -8,6 +8,7 @@
 
 #include "nsString.h"
 #include "nsTArray.h"
+#include "nsASCIIMask.h"
 
 class nsIFile;
 class nsIURLParser;
@@ -99,9 +100,11 @@ void net_FilterURIString(const nsACString& input, nsACString& result);
  *
  * @param aInput the URL spec we want to filter
  * @param aFlags the flags which control which characters we escape
+ * @param aFilterMask a mask of characters that should excluded from the result
  * @param aResult the out param to write to if filtering happens
  */
 nsresult net_FilterAndEscapeURI(const nsACString& aInput, uint32_t aFlags,
+                                const ASCIIMaskArray& aFilterMask,
                                 nsACString& aResult);
 
 #if defined(XP_WIN)

netwerk/protocol/data/nsDataHandler.cpp

@@ -58,7 +58,7 @@ nsDataHandler::GetProtocolFlags(uint32_t* result) {
   nsresult rv;
   nsCOMPtr<nsIURI> uri;
 
-  nsCString spec(aSpec);
+  const nsPromiseFlatCString& spec = PromiseFlatCString(aSpec);
 
 #ifdef ANDROID
   // Due to heap limitations on mobile, limits the size of data URL
@@ -83,14 +83,16 @@ nsDataHandler::GetProtocolFlags(uint32_t* result) {
     if (base64 || (strncmp(contentType.get(), "text/", 5) != 0 &&
                    contentType.Find("xml") == kNotFound)) {
       // it's ascii encoded binary, don't let any spaces in
-      if (!spec.StripWhitespace(mozilla::fallible)) {
-        return NS_ERROR_OUT_OF_MEMORY;
-      }
+      rv = NS_MutateURI(new mozilla::net::nsSimpleURI::Mutator())
+               .Apply(NS_MutatorMethod(
+                   &nsISimpleURIMutator::SetSpecAndFilterWhitespace, spec,
+                   nullptr))
+               .Finalize(uri);
+    } else {
+      rv = NS_MutateURI(new mozilla::net::nsSimpleURI::Mutator())
+               .SetSpec(spec)
+               .Finalize(uri);
     }
-
-    rv = NS_MutateURI(new mozilla::net::nsSimpleURI::Mutator())
-             .SetSpec(spec)
-             .Finalize(uri);
   }
 
   if (NS_FAILED(rv)) return rv;
@@ -239,7 +241,7 @@ nsresult nsDataHandler::ParsePathWithoutRef(
   return NS_OK;
 }
 
-nsresult nsDataHandler::ParseURI(nsCString& spec, nsCString& contentType,
+nsresult nsDataHandler::ParseURI(const nsCString& spec, nsCString& contentType,
                                  nsCString* contentCharset, bool& isBase64,
                                  nsCString* dataBuffer) {
   static constexpr auto kDataScheme = "data:"_ns;

netwerk/protocol/data/nsDataHandler.h

@@ -33,7 +33,7 @@ class nsDataHandler : public nsIProtocolHandler,
   // (the given spec will temporarily be modified but will be returned
   //  to the original before returning)
   // contentCharset and dataBuffer can be nullptr if they are not needed.
-  [[nodiscard]] static nsresult ParseURI(nsCString& spec,
+  [[nodiscard]] static nsresult ParseURI(const nsCString& spec,
                                          nsCString& contentType,
                                          nsCString* contentCharset,
                                          bool& isBase64, nsCString* dataBuffer);

xpcom/string/nsASCIIMask.h

@@ -51,17 +51,18 @@ class ASCIIMask {
 // ...
 // if (someChar < 128 && sABCMask[someChar]) this is A or B or C
 
-namespace details {
+namespace asciimask_details {
 template <typename F, size_t... Indices>
 constexpr std::array<bool, 128> CreateASCIIMask(
     F fun, std::index_sequence<Indices...>) {
   return {{fun(Indices)...}};
 }
-}  // namespace details
+}  // namespace asciimask_details
 
 template <typename F>
 constexpr std::array<bool, 128> CreateASCIIMask(F fun) {
-  return details::CreateASCIIMask(fun, std::make_index_sequence<128>{});
+  return asciimask_details::CreateASCIIMask(fun,
+                                            std::make_index_sequence<128>{});
 }
 
 }  // namespace mozilla