Bug 1588260 part 2. Switch nsICollation away from using xpidl [array]. r=peterv,emk
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 11 Nov 2019 18:03:42 +0000
changeset 501489 d9acb8da07684d0511722574cb8a673322604f00
parent 501488 dfa02c90256c40834b0db938ce769506dc2955da
child 501490 bad35a6f6c69e4f3d88a36ff57e2a12b282df455
push id36794
push useraciure@mozilla.com
push dateTue, 12 Nov 2019 09:43:07 +0000
treeherdermozilla-central@5f0b392beadb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspeterv, emk
bugs1588260
milestone72.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 1588260 part 2. Switch nsICollation away from using xpidl [array]. r=peterv,emk Differential Revision: https://phabricator.services.mozilla.com/D49052
dom/xslt/xslt/txXPathResultComparator.cpp
dom/xslt/xslt/txXPathResultComparator.h
intl/locale/nsCollation.cpp
intl/locale/nsICollation.idl
intl/locale/tests/gtest/TestCollation.cpp
--- a/dom/xslt/xslt/txXPathResultComparator.cpp
+++ b/dom/xslt/xslt/txXPathResultComparator.cpp
@@ -6,16 +6,18 @@
 #include "mozilla/FloatingPoint.h"
 
 #include "txXPathResultComparator.h"
 #include "txExpr.h"
 #include "txCore.h"
 #include "nsCollationCID.h"
 #include "nsIServiceManager.h"
 
+using namespace mozilla;
+
 #define kAscending (1 << 0)
 #define kUpperFirst (1 << 1)
 
 txResultStringComparator::txResultStringComparator(bool aAscending,
                                                    bool aUpperFirst,
                                                    const nsString& aLanguage) {
   mSorting = 0;
   if (aAscending) mSorting |= kAscending;
@@ -48,111 +50,98 @@ nsresult txResultStringComparator::creat
                                                        txObject*& aResult) {
   nsAutoPtr<StringValue> val(new StringValue);
   if (!val) {
     return NS_ERROR_OUT_OF_MEMORY;
   }
 
   if (!mCollation) return NS_ERROR_FAILURE;
 
-  val->mCaseKeyString = new nsString;
+  val->mCaseKeyString = MakeUnique<nsString>();
   nsString& nsCaseKey = *val->mCaseKeyString;
   nsresult rv = aExpr->evaluateToString(aContext, nsCaseKey);
   NS_ENSURE_SUCCESS(rv, rv);
 
   if (nsCaseKey.IsEmpty()) {
     aResult = val.forget();
 
     return NS_OK;
   }
 
   rv = mCollation->AllocateRawSortKey(nsICollation::kCollationCaseInSensitive,
-                                      nsCaseKey, &val->mKey, &val->mLength);
+                                      nsCaseKey, val->mKey);
   NS_ENSURE_SUCCESS(rv, rv);
 
   aResult = val.forget();
 
   return NS_OK;
 }
 
 int txResultStringComparator::compareValues(txObject* aVal1, txObject* aVal2) {
   StringValue* strval1 = (StringValue*)aVal1;
   StringValue* strval2 = (StringValue*)aVal2;
 
   if (!mCollation) return -1;
 
-  if (strval1->mLength == 0) {
-    if (strval2->mLength == 0) return 0;
+  if (strval1->mKey.Length() == 0) {
+    if (strval2->mKey.Length() == 0) return 0;
     return ((mSorting & kAscending) ? -1 : 1);
   }
 
-  if (strval2->mLength == 0) return ((mSorting & kAscending) ? 1 : -1);
+  if (strval2->mKey.Length() == 0) return ((mSorting & kAscending) ? 1 : -1);
 
   nsresult rv;
   int32_t result = -1;
-  rv = mCollation->CompareRawSortKey(strval1->mKey, strval1->mLength,
-                                     strval2->mKey, strval2->mLength, &result);
+  rv = mCollation->CompareRawSortKey(strval1->mKey, strval2->mKey, &result);
   if (NS_FAILED(rv)) {
     // XXX ErrorReport
     return -1;
   }
 
   if (result != 0) return ((mSorting & kAscending) ? 1 : -1) * result;
 
-  if ((strval1->mCaseKeyLength == 0) && (strval1->mLength != 0)) {
-    rv = strval1->initCaseKeyBuffer(mCollation);
+  if (strval1->mCaseKeyString && strval1->mKey.Length() != 0) {
+    rv = strval1->initCaseKey(mCollation);
     if (NS_FAILED(rv)) {
       // XXX ErrorReport
       return -1;
     }
   }
-  if ((strval2->mCaseKeyLength == 0) && (strval2->mLength != 0)) {
-    rv = strval2->initCaseKeyBuffer(mCollation);
+  if (strval2->mCaseKeyString && strval2->mKey.Length() != 0) {
+    rv = strval2->initCaseKey(mCollation);
     if (NS_FAILED(rv)) {
       // XXX ErrorReport
       return -1;
     }
   }
-  rv = mCollation->CompareRawSortKey(
-      strval1->mCaseKeyBuffer, strval1->mCaseKeyLength, strval2->mCaseKeyBuffer,
-      strval2->mCaseKeyLength, &result);
+  rv = mCollation->CompareRawSortKey(strval1->mCaseKey, strval2->mCaseKey,
+                                     &result);
   if (NS_FAILED(rv)) {
     // XXX ErrorReport
     return -1;
   }
 
   return ((mSorting & kAscending) ? 1 : -1) *
          ((mSorting & kUpperFirst) ? -1 : 1) * result;
 }
 
-txResultStringComparator::StringValue::StringValue()
-    : mKey(0), mCaseKeyString(nullptr), mLength(0), mCaseKeyLength(0) {}
+txResultStringComparator::StringValue::StringValue() = default;
 
-txResultStringComparator::StringValue::~StringValue() {
-  free(mKey);
-  if (mCaseKeyLength > 0)
-    free(mCaseKeyBuffer);
-  else
-    delete mCaseKeyString;
-}
+txResultStringComparator::StringValue::~StringValue() = default;
 
-nsresult txResultStringComparator::StringValue::initCaseKeyBuffer(
+nsresult txResultStringComparator::StringValue::initCaseKey(
     nsICollation* aCollation) {
-  nsString* caseString = mCaseKeyString;
-  mCaseKeyString = nullptr;
   nsresult rv = aCollation->AllocateRawSortKey(
-      nsICollation::kCollationCaseSensitive, *caseString, &mCaseKeyBuffer,
-      &mCaseKeyLength);
+      nsICollation::kCollationCaseSensitive, *mCaseKeyString, mCaseKey);
   if (NS_FAILED(rv)) {
-    mCaseKeyString = caseString;
-    mCaseKeyLength = 0;
+    mCaseKey.SetLength(0);
     return rv;
   }
 
-  delete caseString;
+  mCaseKeyString = nullptr;
   return NS_OK;
 }
 
 txResultNumberComparator::txResultNumberComparator(bool aAscending) {
   mAscending = aAscending ? 1 : -1;
 }
 
 nsresult txResultNumberComparator::createSortableValue(Expr* aExpr,
--- a/dom/xslt/xslt/txXPathResultComparator.h
+++ b/dom/xslt/xslt/txXPathResultComparator.h
@@ -2,16 +2,17 @@
 /* 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/. */
 
 #ifndef TRANSFRMX_XPATHRESULTCOMPARATOR_H
 #define TRANSFRMX_XPATHRESULTCOMPARATOR_H
 
 #include "mozilla/Attributes.h"
+#include "mozilla/UniquePtr.h"
 #include "txCore.h"
 #include "nsCOMPtr.h"
 #include "nsICollation.h"
 #include "nsString.h"
 
 class Expr;
 class txIEvalContext;
 
@@ -54,27 +55,23 @@ class txResultStringComparator : public 
                             uint8_t** aKey, uint32_t* aLength);
   int mSorting;
 
   class StringValue : public txObject {
    public:
     StringValue();
     ~StringValue();
 
-    nsresult initCaseKeyBuffer(nsICollation* aCollation);
+    nsresult initCaseKey(nsICollation* aCollation);
 
-    uint8_t* mKey;
-    union {
-      nsString* mCaseKeyString;
-      uint8_t* mCaseKeyBuffer;
-    };
-    // When mCaseKeyLength is nonzero, we have an mCaseKeyBuffer, which needs to
-    // be freed with free().  Otherwise we have an mCaseKeyString, which needs
-    // to have operator delete called on it.
-    uint32_t mLength, mCaseKeyLength;
+    nsTArray<uint8_t> mKey;
+    // Either mCaseKeyString is non-null, or we have a usable key in mCaseKey
+    // already.
+    mozilla::UniquePtr<nsString> mCaseKeyString;
+    nsTArray<uint8_t> mCaseKey;
   };
 };
 
 /*
  * Compare results as numbers (data-type="number")
  */
 class txResultNumberComparator : public txXPathResultComparator {
  public:
--- a/intl/locale/nsCollation.cpp
+++ b/intl/locale/nsCollation.cpp
@@ -125,45 +125,37 @@ nsCollation::Initialize(const nsACString
   ucol_close(collator);
 
   mInit = true;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsCollation::AllocateRawSortKey(int32_t strength, const nsAString& stringIn,
-                                uint8_t** key, uint32_t* outLen) {
+                                nsTArray<uint8_t>& key) {
   NS_ENSURE_TRUE(mInit, NS_ERROR_NOT_INITIALIZED);
-  NS_ENSURE_ARG_POINTER(key);
-  NS_ENSURE_ARG_POINTER(outLen);
 
   nsresult res = EnsureCollator(strength);
   NS_ENSURE_SUCCESS(res, res);
 
   uint32_t stringInLen = stringIn.Length();
 
   const UChar* str = (const UChar*)stringIn.BeginReading();
 
   int32_t keyLength =
       ucol_getSortKey(mCollatorICU, str, stringInLen, nullptr, 0);
   NS_ENSURE_TRUE((stringInLen == 0 || keyLength > 0), NS_ERROR_FAILURE);
 
-  // Since key is freed elsewhere with free, allocate with malloc.
-  uint8_t* newKey = (uint8_t*)malloc(keyLength + 1);
-  if (!newKey) {
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
+  key.SetLength(keyLength + 1);
 
-  keyLength =
-      ucol_getSortKey(mCollatorICU, str, stringInLen, newKey, keyLength + 1);
+  keyLength = ucol_getSortKey(mCollatorICU, str, stringInLen, key.Elements(),
+                              keyLength + 1);
   NS_ENSURE_TRUE((stringInLen == 0 || keyLength > 0), NS_ERROR_FAILURE);
 
-  *key = newKey;
-  *outLen = keyLength;
-
+  key.SetLength(keyLength);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsCollation::CompareString(int32_t strength, const nsAString& string1,
                            const nsAString& string2, int32_t* result) {
   NS_ENSURE_TRUE(mInit, NS_ERROR_NOT_INITIALIZED);
   NS_ENSURE_ARG_POINTER(result);
@@ -190,29 +182,35 @@ nsCollation::CompareString(int32_t stren
     default:
       MOZ_CRASH("ucol_strcoll returned bad UCollationResult");
   }
   *result = res;
   return NS_OK;
 }
 
 NS_IMETHODIMP
-nsCollation::CompareRawSortKey(const uint8_t* key1, uint32_t len1,
-                               const uint8_t* key2, uint32_t len2,
-                               int32_t* result) {
+nsCollation::CompareRawSortKey(const nsTArray<uint8_t>& key1,
+                               const nsTArray<uint8_t>& key2, int32_t* result) {
   NS_ENSURE_TRUE(mInit, NS_ERROR_NOT_INITIALIZED);
-  NS_ENSURE_ARG_POINTER(key1);
-  NS_ENSURE_ARG_POINTER(key2);
   NS_ENSURE_ARG_POINTER(result);
   *result = 0;
 
-  int32_t tmpResult = strcmp((const char*)key1, (const char*)key2);
+  size_t minLength = std::min(key1.Length(), key2.Length());
+  int32_t tmpResult = strncmp((const char*)key1.Elements(),
+                              (const char*)key2.Elements(), minLength);
   int32_t res;
   if (tmpResult < 0) {
     res = -1;
   } else if (tmpResult > 0) {
     res = 1;
+  } else if (key1.Length() > minLength) {
+    // First string contains second one, so comes later, hence return > 0.
+    res = 1;
+  } else if (key2.Length() > minLength) {
+    // First string is a substring of second one, so comes earlier,
+    // hence return < 0.
+    res = -1;
   } else {
     res = 0;
   }
   *result = res;
   return NS_OK;
 }
--- a/intl/locale/nsICollation.idl
+++ b/intl/locale/nsICollation.idl
@@ -46,20 +46,18 @@ interface nsICollation : nsISupports {
   // init this interface to a specified locale (should only be called by collation factory)
   void initialize(in ACString locale);
 
   // compare two strings
   // result is same as strcmp
   long compareString(in long strength, in AString string1, in AString string2);
 
   // allocate sort key from input string
-  // returns newly allocated key, and its band its byte length
-  [noscript] void allocateRawSortKey(in long strength,
-                                     in AString stringIn,
-                                     [array,size_is(outLen)] out octet key,
-                                     out unsigned long outLen);
+  // returns newly allocated key
+  [noscript] Array<octet> allocateRawSortKey(in long strength,
+                                             in AString stringIn);
 
   // compare two sort keys
-  // length is a byte length, result is same as strcmp
-  [noscript] long compareRawSortKey([const,array,size_is(len1)] in octet key1, in unsigned long len1,
-                                    [const,array,size_is(len2)] in octet key2, in unsigned long len2);
+  // result is same as strcmp
+  [noscript] long compareRawSortKey(in Array<octet> key1,
+                                    in Array<octet> key2);
 
 };
--- a/intl/locale/tests/gtest/TestCollation.cpp
+++ b/intl/locale/tests/gtest/TestCollation.cpp
@@ -4,47 +4,40 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "gtest/gtest.h"
 #include "nsCollationCID.h"
 #include "nsComponentManagerUtils.h"
 #include "nsCOMPtr.h"
 #include "nsICollation.h"
 #include "nsString.h"
+#include "nsTArray.h"
 
 TEST(Collation, AllocateRowSortKey)
 {
   nsCOMPtr<nsICollationFactory> colFactory =
       do_CreateInstance(NS_COLLATIONFACTORY_CONTRACTID);
   ASSERT_TRUE(colFactory);
 
   // Don't throw error even if locale name is invalid
   nsCOMPtr<nsICollation> collator;
   nsresult rv = colFactory->CreateCollationForLocale(
       NS_LITERAL_CSTRING("$languageName"), getter_AddRefs(collator));
   ASSERT_TRUE(NS_SUCCEEDED(rv));
 
-  uint8_t* sortKey1 = nullptr;
-  uint32_t sortKeyLen1 = 0;
+  nsTArray<uint8_t> sortKey1;
   // Don't throw error even if locale name is invalid
   rv = collator->AllocateRawSortKey(nsICollation::kCollationStrengthDefault,
-                                    NS_LITERAL_STRING("ABC"), &sortKey1,
-                                    &sortKeyLen1);
+                                    NS_LITERAL_STRING("ABC"), sortKey1);
   ASSERT_TRUE(NS_SUCCEEDED(rv));
 
-  uint8_t* sortKey2 = nullptr;
-  uint32_t sortKeyLen2 = 0;
+  nsTArray<uint8_t> sortKey2;
   // Don't throw error even if locale name is invalid
   rv = collator->AllocateRawSortKey(nsICollation::kCollationStrengthDefault,
-                                    NS_LITERAL_STRING("DEF"), &sortKey2,
-                                    &sortKeyLen2);
+                                    NS_LITERAL_STRING("DEF"), sortKey2);
   ASSERT_TRUE(NS_SUCCEEDED(rv));
 
   int32_t result;
-  rv = collator->CompareRawSortKey(sortKey1, sortKeyLen1, sortKey2, sortKeyLen2,
-                                   &result);
+  rv = collator->CompareRawSortKey(sortKey1, sortKey2, &result);
   ASSERT_TRUE(NS_SUCCEEDED(rv));
 
   ASSERT_TRUE(result < 0);
-
-  free(sortKey1);
-  free(sortKey2);
 }