Bug 1644163 - Avoid traversing a nsTSubstring tuple twice to determine its length and dependency on a buffer in Assign. r=froydnj

This also extracts new private member functions AssignOwned and AssignNonDependent.

Furthermore, it fixes an inconsistency for the dependent case: Formerly,
this caused the substring tuple to be materialized into a linear string
infallibly, although the function was a fallible one. It only assigned the
resulting string fallibly, but this would never have failed, since the
allocation already happened before.

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

xpcom/string/nsTSubstring.cpp

@@ -507,6 +507,24 @@ void nsTSubstring<T>::Assign(self_type&& aStr) {
   }
 }
 
+template <typename T>
+void nsTSubstring<T>::AssignOwned(self_type&& aStr) {
+  NS_ASSERTION(aStr.mDataFlags & (DataFlags::REFCOUNTED | DataFlags::OWNED),
+               "neither shared nor owned");
+
+  // If they have a REFCOUNTED or OWNED buffer, we can avoid a copy - so steal
+  // their buffer and reset them to the empty string.
+
+  // |aStr| should be null-terminated
+  NS_ASSERTION(aStr.mDataFlags & DataFlags::TERMINATED,
+               "shared or owned, but not terminated");
+
+  ::ReleaseData(this->mData, this->mDataFlags);
+
+  SetData(aStr.mData, aStr.mLength, aStr.mDataFlags);
+  aStr.SetToEmptyBuffer();
+}
+
 template <typename T>
 bool nsTSubstring<T>::Assign(self_type&& aStr, const fallible_t& aFallible) {
   // We're moving |aStr| in this method, so we need to try to steal the data,
@@ -519,17 +537,7 @@ bool nsTSubstring<T>::Assign(self_type&& aStr, const fallible_t& aFallible) {
   }
 
   if (aStr.mDataFlags & (DataFlags::REFCOUNTED | DataFlags::OWNED)) {
-    // If they have a REFCOUNTED or OWNED buffer, we can avoid a copy - so steal
-    // their buffer and reset them to the empty string.
-
-    // |aStr| should be null-terminated
-    NS_ASSERTION(aStr.mDataFlags & DataFlags::TERMINATED,
-                 "shared or owned, but not terminated");
-
-    ::ReleaseData(this->mData, this->mDataFlags);
-
-    SetData(aStr.mData, aStr.mLength, aStr.mDataFlags);
-    aStr.SetToEmptyBuffer();
+    AssignOwned(std::move(aStr));
     return true;
   }
 
@@ -550,26 +558,40 @@ void nsTSubstring<T>::Assign(const substring_tuple_type& aTuple) {
 }
 
 template <typename T>
-bool nsTSubstring<T>::Assign(const substring_tuple_type& aTuple,
-                             const fallible_t& aFallible) {
-  if (aTuple.IsDependentOn(this->mData, this->mData + this->mLength)) {
-    // take advantage of sharing here...
-    return Assign(string_type(aTuple), aFallible);
-  }
+bool nsTSubstring<T>::AssignNonDependent(const substring_tuple_type& aTuple,
+                                         size_type aTupleLength,
+                                         const mozilla::fallible_t& aFallible) {
+  NS_ASSERTION(aTuple.Length() == aTupleLength, "wrong length passed");
 
-  size_type length = aTuple.Length();
-
-  mozilla::Result<uint32_t, nsresult> r = StartBulkWriteImpl(length);
+  mozilla::Result<uint32_t, nsresult> r = StartBulkWriteImpl(aTupleLength);
   if (r.isErr()) {
     return false;
   }
 
-  aTuple.WriteTo(this->mData, length);
+  aTuple.WriteTo(this->mData, aTupleLength);
 
-  FinishBulkWriteImpl(length);
+  FinishBulkWriteImpl(aTupleLength);
   return true;
 }
 
+template <typename T>
+bool nsTSubstring<T>::Assign(const substring_tuple_type& aTuple,
+                             const fallible_t& aFallible) {
+  const auto [isDependentOnThis, tupleLength] =
+      aTuple.IsDependentOnWithLength(this->mData, this->mData + this->mLength);
+  if (isDependentOnThis) {
+    string_type temp;
+    self_type& tempSubstring = temp;
+    if (!tempSubstring.AssignNonDependent(aTuple, tupleLength, aFallible)) {
+      return false;
+    }
+    AssignOwned(std::move(temp));
+    return true;
+  }
+
+  return AssignNonDependent(aTuple, tupleLength, aFallible);
+}
+
 template <typename T>
 void nsTSubstring<T>::Adopt(char_type* aData, size_type aLength) {
   if (aData) {

xpcom/string/nsTSubstring.h

@@ -1271,6 +1271,11 @@ class nsTSubstring : public mozilla::detail::nsTStringRepr<T> {
       size_type aOldSuffixStart = 0, size_type aNewSuffixStart = 0);
 
  private:
+  void AssignOwned(self_type&& aStr);
+  bool AssignNonDependent(const substring_tuple_type& aTuple,
+                          size_type aTupleLength,
+                          const mozilla::fallible_t& aFallible);
+
   /**
    * Do not call this except from within FinishBulkWriteImpl() and
    * SetCapacity().