Skip to content

Commit

Permalink
Bug 1719550 - Unify collator in mozStorageService and SQLCollations; …
Browse files Browse the repository at this point in the history 
…r=platform-i18n-reviewers,nordzilla

Differential Revision: https://phabricator.services.mozilla.com/D121432
  • Loading branch information
@gregtatum
gregtatum committed Aug 10, 2021
1 parent a7f0aa2 commit ee5176c
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 61 deletions.
39 changes: 21 additions & 18 deletions storage/SQLCollations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "mozilla/ArrayUtils.h"
#include "mozilla/intl/Collator.h"

#include "SQLCollations.h"

using mozilla::intl::Collator;

namespace mozilla {
namespace storage {

Expand All @@ -20,7 +23,7 @@ namespace {
* Helper function for the UTF-8 locale collations.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes in aStr1.
* @param aStr1
Expand All @@ -31,26 +34,26 @@ namespace {
* @param aStr2
* The string to be compared against aStr1 as provided by SQLite. It
* must be a non-null-terminated char* buffer.
* @param aComparisonStrength
* The sorting strength, one of the nsICollation constants.
* @param aSensitivity
* The sorting sensitivity.
* @return aStr1 - aStr2. That is, if aStr1 < aStr2, returns a negative number.
* If aStr1 > aStr2, returns a positive number. If aStr1 == aStr2,
* returns 0.
*/
int localeCollationHelper8(void* aService, int aLen1, const void* aStr1,
int aLen2, const void* aStr2,
int32_t aComparisonStrength) {
Collator::Sensitivity aSensitivity) {
NS_ConvertUTF8toUTF16 str1(static_cast<const char*>(aStr1), aLen1);
NS_ConvertUTF8toUTF16 str2(static_cast<const char*>(aStr2), aLen2);
Service* serv = static_cast<Service*>(aService);
return serv->localeCompareStrings(str1, str2, aComparisonStrength);
return serv->localeCompareStrings(str1, str2, aSensitivity);
}

/**
* Helper function for the UTF-16 locale collations.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes (not characters) in aStr1.
* @param aStr1
Expand All @@ -61,15 +64,15 @@ int localeCollationHelper8(void* aService, int aLen1, const void* aStr1,
* @param aStr2
* The string to be compared against aStr1 as provided by SQLite. It
* must be a non-null-terminated char16_t* buffer.
* @param aComparisonStrength
* The sorting strength, one of the nsICollation constants.
* @param aSensitivity
* The sorting sensitivity.
* @return aStr1 - aStr2. That is, if aStr1 < aStr2, returns a negative number.
* If aStr1 > aStr2, returns a positive number. If aStr1 == aStr2,
* returns 0.
*/
int localeCollationHelper16(void* aService, int aLen1, const void* aStr1,
int aLen2, const void* aStr2,
int32_t aComparisonStrength) {
Collator::Sensitivity aSensitivity) {
const char16_t* buf1 = static_cast<const char16_t*>(aStr1);
const char16_t* buf2 = static_cast<const char16_t*>(aStr2);

Expand All @@ -80,7 +83,7 @@ int localeCollationHelper16(void* aService, int aLen1, const void* aStr1,
nsDependentSubstring str1(buf1, buf1 + (aLen1 / sizeof(char16_t)));
nsDependentSubstring str2(buf2, buf2 + (aLen2 / sizeof(char16_t)));
Service* serv = static_cast<Service*>(aService);
return serv->localeCompareStrings(str1, str2, aComparisonStrength);
return serv->localeCompareStrings(str1, str2, aSensitivity);
}

// This struct is used only by registerCollations below, but ISO C++98 forbids
Expand Down Expand Up @@ -127,53 +130,53 @@ int registerCollations(sqlite3* aDB, Service* aService) {
int localeCollation8(void* aService, int aLen1, const void* aStr1, int aLen2,
const void* aStr2) {
return localeCollationHelper8(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationCaseInSensitive);
Collator::Sensitivity::Base);
}

int localeCollationCaseSensitive8(void* aService, int aLen1, const void* aStr1,
int aLen2, const void* aStr2) {
return localeCollationHelper8(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationAccentInsenstive);
Collator::Sensitivity::Case);
}

int localeCollationAccentSensitive8(void* aService, int aLen1,
const void* aStr1, int aLen2,
const void* aStr2) {
return localeCollationHelper8(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationCaseInsensitiveAscii);
Collator::Sensitivity::Accent);
}

int localeCollationCaseAccentSensitive8(void* aService, int aLen1,
const void* aStr1, int aLen2,
const void* aStr2) {
return localeCollationHelper8(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationCaseSensitive);
Collator::Sensitivity::Variant);
}

int localeCollation16(void* aService, int aLen1, const void* aStr1, int aLen2,
const void* aStr2) {
return localeCollationHelper16(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationCaseInSensitive);
Collator::Sensitivity::Base);
}

int localeCollationCaseSensitive16(void* aService, int aLen1, const void* aStr1,
int aLen2, const void* aStr2) {
return localeCollationHelper16(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationAccentInsenstive);
Collator::Sensitivity::Case);
}

int localeCollationAccentSensitive16(void* aService, int aLen1,
const void* aStr1, int aLen2,
const void* aStr2) {
return localeCollationHelper16(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationCaseInsensitiveAscii);
Collator::Sensitivity::Accent);
}

int localeCollationCaseAccentSensitive16(void* aService, int aLen1,
const void* aStr1, int aLen2,
const void* aStr2) {
return localeCollationHelper16(aService, aLen1, aStr1, aLen2, aStr2,
nsICollation::kCollationCaseSensitive);
Collator::Sensitivity::Variant);
}

} // namespace storage
Expand Down
18 changes: 9 additions & 9 deletions storage/SQLCollations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace storage {
* @param aDB
* The database we'll be registering the collations with.
* @param aService
* The Service that owns the nsICollation used by our collations.
* The Service that owns the collator used by our collations.
* @return the SQLite status code indicating success or failure.
*/
int registerCollations(sqlite3* aDB, Service* aService);
Expand All @@ -36,7 +36,7 @@ int registerCollations(sqlite3* aDB, Service* aService);
* Comparison is case- and accent-insensitive. This is called by SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes in aStr1.
* @param aStr1
Expand All @@ -60,7 +60,7 @@ int localeCollation8(void* aService, int aLen1, const void* aStr1, int aLen2,
* SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes in aStr1.
* @param aStr1
Expand All @@ -84,7 +84,7 @@ int localeCollationCaseSensitive8(void* aService, int aLen1, const void* aStr1,
* SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes in aStr1.
* @param aStr1
Expand All @@ -108,7 +108,7 @@ int localeCollationAccentSensitive8(void* aService, int aLen1,
* Comparison is case- and accent-sensitive. This is called by SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes in aStr1.
* @param aStr1
Expand All @@ -132,7 +132,7 @@ int localeCollationCaseAccentSensitive8(void* aService, int aLen1,
* Comparison is case- and accent-insensitive. This is called by SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes (not characters) in aStr1.
* @param aStr1
Expand All @@ -156,7 +156,7 @@ int localeCollation16(void* aService, int aLen1, const void* aStr1, int aLen2,
* SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes (not characters) in aStr1.
* @param aStr1
Expand All @@ -180,7 +180,7 @@ int localeCollationCaseSensitive16(void* aService, int aLen1, const void* aStr1,
* SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes (not characters) in aStr1.
* @param aStr1
Expand All @@ -204,7 +204,7 @@ int localeCollationAccentSensitive16(void* aService, int aLen1,
* Comparison is case- and accent-sensitive. This is called by SQLite.
*
* @param aService
* The Service that owns the nsICollation used by this collation.
* The Service that owns the collator used by this collation.
* @param aLen1
* The number of bytes (not characters) in aStr1.
* @param aStr1
Expand Down
59 changes: 37 additions & 22 deletions storage/mozStorageService.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include "mozIStorageCompletionCallback.h"
#include "mozIStoragePendingStatement.h"
#include "mozilla/StaticPrefs_storage.h"
#include "mozilla/intl/Collator.h"
#include "mozilla/intl/LocaleService.h"

#include "sqlite3.h"
#include "mozilla/AutoSQLiteLifetime.h"
Expand All @@ -31,6 +33,8 @@
# undef CompareString
#endif

using mozilla::intl::Collator;

namespace mozilla {
namespace storage {

Expand Down Expand Up @@ -354,47 +358,58 @@ nsresult Service::initialize() {

int Service::localeCompareStrings(const nsAString& aStr1,
const nsAString& aStr2,
int32_t aComparisonStrength) {
// The implementation of nsICollation.CompareString() is platform-dependent.
// On Linux it's not thread-safe. It may not be on Windows and OS X either,
// but it's more difficult to tell. We therefore synchronize this method.
Collator::Sensitivity aSensitivity) {
// The mozilla::intl::Collator is not thread safe, since the Collator::Options
// can be changed.
MutexAutoLock mutex(mMutex);

nsICollation* coll = getLocaleCollation();
if (!coll) {
Collator* collator = getCollator();
if (!collator) {
NS_ERROR("Storage service has no collation");
return 0;
}

int32_t res;
nsresult rv = coll->CompareString(aComparisonStrength, aStr1, aStr2, &res);
if (NS_FAILED(rv)) {
NS_ERROR("Collation compare string failed");
return 0;
if (aSensitivity != mLastSensitivity) {
auto result =
mCollator->SetOptions(Collator::Options{.sensitivity = aSensitivity});

if (result.isErr()) {
NS_WARNING("Could not configure the mozilla::intl::Collation.");
return 0;
}
mLastSensitivity = aSensitivity;
}

return res;
return collator->CompareStrings(aStr1, aStr2);
}

nsICollation* Service::getLocaleCollation() {
Collator* Service::getCollator() {
mMutex.AssertCurrentThreadOwns();

if (mLocaleCollation) return mLocaleCollation;
if (mCollator) {
return mCollator.get();
}

nsCOMPtr<nsICollationFactory> collFact =
do_CreateInstance(NS_COLLATIONFACTORY_CONTRACTID);
if (!collFact) {
NS_WARNING("Could not create collation factory");
auto result = mozilla::intl::LocaleService::TryCreateComponent<Collator>();
if (result.isErr()) {
NS_WARNING("Could not create mozilla::intl::Collation.");
return nullptr;
}

nsresult rv = collFact->CreateCollation(getter_AddRefs(mLocaleCollation));
if (NS_FAILED(rv)) {
NS_WARNING("Could not create collation");
mCollator = result.unwrap();

// Sort in a case-insensitive way, where "base" letters are considered
// equal, e.g: a = á, a = A, a ≠ b.
auto optResult = mCollator->SetOptions(
Collator::Options{.sensitivity = Collator::Sensitivity::Base});

if (optResult.isErr()) {
NS_WARNING("Could not configure the mozilla::intl::Collation.");
mCollator = nullptr;
return nullptr;
}

return mLocaleCollation;
return mCollator.get();
}

////////////////////////////////////////////////////////////////////////////////
Expand Down
Loading

0 comments on commit ee5176c

Please sign in to comment.