Bug 1325106 - Part 1: Clean-up new Intl.PluralRules code to match latest Intl.cpp code. r=Waldo
authorAndré Bargull <andre.bargull@gmail.com>
Wed, 28 Dec 2016 05:21:02 -0800
changeset 355470 53eca61fc29392bda69e3b023071c8e41785c669
parent 355469 132fd6c7744146d60fa447f8ce026e8cfa8e2a33
child 355471 6fb180c07443407c6f297d0edf4eb7ca47f8f110
push id10621
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 16:02:43 +0000
treeherdermozilla-aurora@dca7b42e6c67 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1325106
milestone53.0a1
Bug 1325106 - Part 1: Clean-up new Intl.PluralRules code to match latest Intl.cpp code. r=Waldo
js/src/builtin/Intl.cpp
js/src/builtin/Intl.h
--- a/js/src/builtin/Intl.cpp
+++ b/js/src/builtin/Intl.cpp
@@ -774,20 +774,18 @@ uplrules_close(UPluralRules *uplrules)
 
 UPluralRules*
 uplrules_openForType(const char *locale, UPluralType type, UErrorCode *status)
 {
     MOZ_CRASH("uplrules_openForType: Intl API disabled");
 }
 
 int32_t
-uplrules_select(const UPluralRules *uplrules,
-               double number,
-               UChar *keyword, int32_t capacity,
-               UErrorCode *status)
+uplrules_select(const UPluralRules *uplrules, double number, UChar *keyword, int32_t capacity,
+                UErrorCode *status)
 {
     MOZ_CRASH("uplrules_select: Intl API disabled");
 }
 
 } // anonymous namespace
 
 #endif
 
@@ -893,22 +891,16 @@ GetInternals(JSContext* cx, HandleObject
 }
 
 static bool
 equal(const char* s1, const char* s2)
 {
     return !strcmp(s1, s2);
 }
 
-static bool
-equal(JSAutoByteString& s1, const char* s2)
-{
-    return !strcmp(s1.ptr(), s2);
-}
-
 static const char*
 icuLocale(const char* locale)
 {
     if (equal(locale, "und"))
         return ""; // ICU root locale
     return locale;
 }
 
@@ -1041,18 +1033,18 @@ Collator(JSContext* cx, const CallArgs& 
 
         obj = NewObjectWithGivenProto(cx, &CollatorClass, proto);
         if (!obj)
             return false;
 
         obj->as<NativeObject>().setReservedSlot(UCOLLATOR_SLOT, PrivateValue(nullptr));
     }
 
-    RootedValue locales(cx, args.length() > 0 ? args[0] : UndefinedValue());
-    RootedValue options(cx, args.length() > 1 ? args[1] : UndefinedValue());
+    RootedValue locales(cx, args.get(0));
+    RootedValue options(cx, args.get(1));
 
     // Step 6.
     if (!IntlInitialize(cx, obj, cx->names().InitializeCollator, locales, options))
         return false;
 
     args.rval().setObject(*obj);
     return true;
 }
@@ -1546,18 +1538,18 @@ NumberFormat(JSContext* cx, const CallAr
 
         obj = NewObjectWithGivenProto(cx, &NumberFormatClass, proto);
         if (!obj)
             return false;
 
         obj->as<NativeObject>().setReservedSlot(UNUMBER_FORMAT_SLOT, PrivateValue(nullptr));
     }
 
-    RootedValue locales(cx, args.length() > 0 ? args[0] : UndefinedValue());
-    RootedValue options(cx, args.length() > 1 ? args[1] : UndefinedValue());
+    RootedValue locales(cx, args.get(0));
+    RootedValue options(cx, args.get(1));
 
     // Step 3.
     if (!IntlInitialize(cx, obj, cx->names().InitializeNumberFormat, locales, options))
         return false;
 
     args.rval().setObject(*obj);
     return true;
 }
@@ -1715,23 +1707,21 @@ js::intl_numberingSystem(JSContext* cx, 
         return false;
 
     args.rval().setString(jsname);
     return true;
 }
 
 
 /**
- *
  * This creates new UNumberFormat with calculated digit formatting
  * properties for PluralRules.
  *
  * This is similar to NewUNumberFormat but doesn't allow for currency or
  * percent types.
- *
  */
 static UNumberFormat*
 NewUNumberFormatForPluralRules(JSContext* cx, HandleObject pluralRules)
 {
     RootedObject internals(cx, GetInternals(cx, pluralRules));
     if (!internals)
        return nullptr;
 
@@ -1744,59 +1734,45 @@ NewUNumberFormatForPluralRules(JSContext
         return nullptr;
 
     uint32_t uMinimumIntegerDigits = 1;
     uint32_t uMinimumFractionDigits = 0;
     uint32_t uMaximumFractionDigits = 3;
     int32_t uMinimumSignificantDigits = -1;
     int32_t uMaximumSignificantDigits = -1;
 
-    RootedId id(cx, NameToId(cx->names().minimumSignificantDigits));
     bool hasP;
-    if (!HasProperty(cx, internals, id, &hasP))
+    if (!HasProperty(cx, internals, cx->names().minimumSignificantDigits, &hasP))
         return nullptr;
+
     if (hasP) {
-        if (!GetProperty(cx, internals, internals, cx->names().minimumSignificantDigits,
-                         &value))
-        {
+        if (!GetProperty(cx, internals, internals, cx->names().minimumSignificantDigits, &value))
             return nullptr;
-        }
         uMinimumSignificantDigits = value.toInt32();
 
-        if (!GetProperty(cx, internals, internals, cx->names().maximumSignificantDigits,
-                         &value))
-        {
+        if (!GetProperty(cx, internals, internals, cx->names().maximumSignificantDigits, &value))
             return nullptr;
-        }
         uMaximumSignificantDigits = value.toInt32();
     } else {
-        if (!GetProperty(cx, internals, internals, cx->names().minimumIntegerDigits,
-                         &value))
-        {
+        if (!GetProperty(cx, internals, internals, cx->names().minimumIntegerDigits, &value))
             return nullptr;
-        }
         uMinimumIntegerDigits = AssertedCast<uint32_t>(value.toInt32());
 
-        if (!GetProperty(cx, internals, internals, cx->names().minimumFractionDigits,
-                         &value))
-        {
+        if (!GetProperty(cx, internals, internals, cx->names().minimumFractionDigits, &value))
             return nullptr;
-        }
         uMinimumFractionDigits = AssertedCast<uint32_t>(value.toInt32());
 
-        if (!GetProperty(cx, internals, internals, cx->names().maximumFractionDigits,
-                         &value))
-        {
+        if (!GetProperty(cx, internals, internals, cx->names().maximumFractionDigits, &value))
             return nullptr;
-        }
         uMaximumFractionDigits = AssertedCast<uint32_t>(value.toInt32());
     }
 
     UErrorCode status = U_ZERO_ERROR;
-    UNumberFormat* nf = unum_open(UNUM_DECIMAL, nullptr, 0, icuLocale(locale.ptr()), nullptr, &status);
+    UNumberFormat* nf =
+        unum_open(UNUM_DECIMAL, nullptr, 0, icuLocale(locale.ptr()), nullptr, &status);
     if (U_FAILURE(status)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
         return nullptr;
     }
     ScopedICUObject<UNumberFormat, unum_close> toClose(nf);
 
     if (uMinimumSignificantDigits != -1) {
         unum_setAttribute(nf, UNUM_SIGNIFICANT_DIGITS_USED, true);
@@ -1880,51 +1856,39 @@ NewUNumberFormat(JSContext* cx, HandleOb
         }
     } else if (StringEqualsAscii(style, "percent")) {
         uStyle = UNUM_PERCENT;
     } else {
         MOZ_ASSERT(StringEqualsAscii(style, "decimal"));
         uStyle = UNUM_DECIMAL;
     }
 
-    RootedId id(cx, NameToId(cx->names().minimumSignificantDigits));
     bool hasP;
-    if (!HasProperty(cx, internals, id, &hasP))
+    if (!HasProperty(cx, internals, cx->names().minimumSignificantDigits, &hasP))
         return nullptr;
+
     if (hasP) {
-        if (!GetProperty(cx, internals, internals, cx->names().minimumSignificantDigits,
-                         &value))
-        {
+        if (!GetProperty(cx, internals, internals, cx->names().minimumSignificantDigits, &value))
             return nullptr;
-        }
         uMinimumSignificantDigits = value.toInt32();
-        if (!GetProperty(cx, internals, internals, cx->names().maximumSignificantDigits,
-                         &value))
-        {
+
+        if (!GetProperty(cx, internals, internals, cx->names().maximumSignificantDigits, &value))
             return nullptr;
-        }
         uMaximumSignificantDigits = value.toInt32();
     } else {
-        if (!GetProperty(cx, internals, internals, cx->names().minimumIntegerDigits,
-                         &value))
-        {
+        if (!GetProperty(cx, internals, internals, cx->names().minimumIntegerDigits, &value))
             return nullptr;
-        }
         uMinimumIntegerDigits = AssertedCast<uint32_t>(value.toInt32());
-        if (!GetProperty(cx, internals, internals, cx->names().minimumFractionDigits,
-                         &value))
-        {
+
+        if (!GetProperty(cx, internals, internals, cx->names().minimumFractionDigits, &value))
             return nullptr;
-        }
         uMinimumFractionDigits = AssertedCast<uint32_t>(value.toInt32());
-        if (!GetProperty(cx, internals, internals, cx->names().maximumFractionDigits,
-                         &value))
-        {
+
+        if (!GetProperty(cx, internals, internals, cx->names().maximumFractionDigits, &value))
             return nullptr;
-        }
         uMaximumFractionDigits = AssertedCast<uint32_t>(value.toInt32());
     }
 
     if (!GetProperty(cx, internals, internals, cx->names().useGrouping, &value))
         return nullptr;
     uUseGrouping = value.toBoolean();
 
     UErrorCode status = U_ZERO_ERROR;
@@ -2593,18 +2557,18 @@ DateTimeFormat(JSContext* cx, const Call
 
         obj = NewObjectWithGivenProto(cx, &DateTimeFormatClass, proto);
         if (!obj)
             return false;
 
         obj->as<NativeObject>().setReservedSlot(UDATE_FORMAT_SLOT, PrivateValue(nullptr));
     }
 
-    RootedValue locales(cx, args.length() > 0 ? args[0] : UndefinedValue());
-    RootedValue options(cx, args.length() > 1 ? args[1] : UndefinedValue());
+    RootedValue locales(cx, args.get(0));
+    RootedValue options(cx, args.get(1));
 
     // Step 3.
     if (!IntlInitialize(cx, obj, cx->names().InitializeDateTimeFormat, locales, options))
         return false;
 
     args.rval().setObject(*obj);
     return true;
 }
@@ -3676,18 +3640,21 @@ static const JSFunctionSpec pluralRules_
     JS_FS_END
 };
 
 /**
  * PluralRules constructor.
  * Spec: ECMAScript 402 API, PluralRules, 1.1
  */
 static bool
-PluralRules(JSContext* cx, const CallArgs& args, bool construct)
+PluralRules(JSContext* cx, unsigned argc, Value* vp)
 {
+    CallArgs args = CallArgsFromVp(argc, vp);
+    bool construct = args.isConstructing();
+
     RootedObject obj(cx);
 
     if (!construct) {
         JSObject* intl = cx->global()->getOrCreateIntlObject(cx);
         if (!intl)
             return false;
         RootedValue self(cx, args.thisv());
         if (!self.isUndefined() && (!self.isObject() || self.toObject() != *intl)) {
@@ -3720,31 +3687,16 @@ PluralRules(JSContext* cx, const CallArg
 
     if (!IntlInitialize(cx, obj, cx->names().InitializePluralRules, locales, options))
         return false;
 
     args.rval().setObject(*obj);
     return true;
 }
 
-static bool
-PluralRules(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    return PluralRules(cx, args, args.isConstructing());
-}
-
-bool
-js::intl_PluralRules(JSContext* cx, unsigned argc, Value* vp)
-{
-    CallArgs args = CallArgsFromVp(argc, vp);
-    MOZ_ASSERT(args.length() == 2);
-    return PluralRules(cx, args, true);
-}
-
 static void
 pluralRules_finalize(FreeOp* fop, JSObject* obj)
 {
     MOZ_ASSERT(fop->onMainThread());
 
     // This is-undefined check shouldn't be necessary, but for internal
     // brokenness in object allocation code.  For the moment, hack around it by
     // explicitly guarding against the possibility of the reserved slot not
@@ -3809,16 +3761,17 @@ js::intl_PluralRules_availableLocales(JS
     args.rval().set(result);
     return true;
 }
 
 bool
 js::intl_SelectPluralRule(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
+    MOZ_ASSERT(args.length() == 2);
 
     RootedObject pluralRules(cx, &args[0].toObject());
 
     UNumberFormat* nf = NewUNumberFormatForPluralRules(cx, pluralRules);
     if (!nf)
         return false;
 
     ScopedICUObject<UNumberFormat, unum_close> closeNumberFormat(nf);
@@ -3832,89 +3785,91 @@ js::intl_SelectPluralRule(JSContext* cx,
     if (!GetProperty(cx, internals, internals, cx->names().locale, &value))
         return false;
     JSAutoByteString locale(cx, value.toString());
     if (!locale)
         return false;
 
     if (!GetProperty(cx, internals, internals, cx->names().type, &value))
         return false;
-    JSAutoByteString type(cx, value.toString());
+    RootedLinearString type(cx, value.toString()->ensureLinear(cx));
     if (!type)
         return false;
 
     double x = args[1].toNumber();
 
     // We need a NumberFormat in order to format the number
     // using the number formatting options (minimum/maximum*Digits)
-    // before we push the result to PluralRules
+    // before we push the result to PluralRules.
     //
     // This should be fixed in ICU 59 and we'll be able to switch to that
     // API: http://bugs.icu-project.org/trac/ticket/12763
     //
     RootedValue fmtNumValue(cx);
     if (!intl_FormatNumber(cx, nf, x, &fmtNumValue))
         return false;
-    RootedString fmtNumValueString(cx, fmtNumValue.toString());
     AutoStableStringChars stableChars(cx);
-    if (!stableChars.initTwoByte(cx, fmtNumValueString))
+    if (!stableChars.initTwoByte(cx, fmtNumValue.toString()))
         return false;
 
     const UChar* uFmtNumValue = Char16ToUChar(stableChars.twoByteRange().begin().get());
 
     UErrorCode status = U_ZERO_ERROR;
 
     UFormattable* fmt = unum_parseToUFormattable(nf, nullptr, uFmtNumValue,
-                                                 stableChars.twoByteRange().length(), 0, &status);
+                                                 stableChars.twoByteRange().length(), nullptr,
+                                                 &status);
     if (U_FAILURE(status)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
         return false;
     }
 
     ScopedICUObject<UFormattable, ufmt_close> closeUFormattable(fmt);
 
     double y = ufmt_getDouble(fmt, &status);
     if (U_FAILURE(status)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
         return false;
     }
 
     UPluralType category;
-
-    if (equal(type, "cardinal")) {
+    if (StringEqualsAscii(type, "cardinal")) {
         category = UPLURAL_TYPE_CARDINAL;
     } else {
-        MOZ_ASSERT(equal(type, "ordinal"));
+        MOZ_ASSERT(StringEqualsAscii(type, "ordinal"));
         category = UPLURAL_TYPE_ORDINAL;
     }
 
     UPluralRules* pr = uplrules_openForType(icuLocale(locale.ptr()), category, &status);
     if (U_FAILURE(status)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
         return false;
     }
 
     ScopedICUObject<UPluralRules, uplrules_close> closePluralRules(pr);
 
     Vector<char16_t, INITIAL_CHAR_BUFFER_SIZE> chars(cx);
     if (!chars.resize(INITIAL_CHAR_BUFFER_SIZE))
         return false;
 
-    int size = uplrules_select(pr, y, Char16ToUChar(chars.begin()), INITIAL_CHAR_BUFFER_SIZE, &status);
+    int32_t size = uplrules_select(pr, y, Char16ToUChar(chars.begin()), INITIAL_CHAR_BUFFER_SIZE,
+                                   &status);
     if (status == U_BUFFER_OVERFLOW_ERROR) {
+        MOZ_ASSERT(size >= 0);
         if (!chars.resize(size))
             return false;
         status = U_ZERO_ERROR;
         uplrules_select(pr, y, Char16ToUChar(chars.begin()), size, &status);
     }
     if (U_FAILURE(status)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
         return false;
     }
 
+    MOZ_ASSERT(size >= 0);
     JSString* str = NewStringCopyN<CanGC>(cx, chars.begin(), size);
     if (!str)
         return false;
 
     args.rval().setString(str);
     return true;
 }
 
@@ -3923,84 +3878,80 @@ js::intl_GetPluralCategories(JSContext* 
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 2);
 
     JSAutoByteString locale(cx, args[0].toString());
     if (!locale)
         return false;
 
-    JSAutoByteString type(cx, args[1].toString());
+    JSLinearString* type = args[1].toString()->ensureLinear(cx);
     if (!type)
         return false;
 
-    UErrorCode status = U_ZERO_ERROR;
-
     UPluralType category;
-
-    if (equal(type, "cardinal")) {
+    if (StringEqualsAscii(type, "cardinal")) {
         category = UPLURAL_TYPE_CARDINAL;
     } else {
-        MOZ_ASSERT(equal(type, "ordinal"));
+        MOZ_ASSERT(StringEqualsAscii(type, "ordinal"));
         category = UPLURAL_TYPE_ORDINAL;
     }
 
-    UPluralRules* pr = uplrules_openForType(
-        icuLocale(locale.ptr()),
-        category,
-        &status
-    );
+    UErrorCode status = U_ZERO_ERROR;
+    UPluralRules* pr = uplrules_openForType(icuLocale(locale.ptr()), category, &status);
     if (U_FAILURE(status)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
         return false;
     }
 
     ScopedICUObject<UPluralRules, uplrules_close> closePluralRules(pr);
 
-    // We should get a C API for that in ICU 59 and switch to it
+    // We should get a C API for that in ICU 59 and switch to it.
     // https://ssl.icu-project.org/trac/ticket/12772
-    //
     icu::StringEnumeration* kwenum =
         reinterpret_cast<icu::PluralRules*>(pr)->getKeywords(status);
+    if (U_FAILURE(status)) {
+        JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
+        return false;
+    }
+
     UEnumeration* ue = uenum_openFromStringEnumeration(kwenum, &status);
-
     if (U_FAILURE(status)) {
         JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
         return false;
     }
 
     ScopedICUObject<UEnumeration, uenum_close> closeEnum(ue);
 
     RootedObject res(cx, NewDenseEmptyArray(cx));
     if (!res)
         return false;
 
     RootedValue element(cx);
     uint32_t i = 0;
-    int32_t catSize;
-    const char* cat;
 
     do {
-        cat = uenum_next(ue, &catSize, &status);
+        int32_t catSize;
+        const char* cat = uenum_next(ue, &catSize, &status);
         if (U_FAILURE(status)) {
             JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
             return false;
         }
 
         if (!cat)
             break;
 
+        MOZ_ASSERT(catSize >= 0);
         JSString* str = NewStringCopyN<CanGC>(cx, cat, catSize);
         if (!str)
             return false;
 
         element.setString(str);
-        if (!DefineElement(cx, res, i, element))
+        if (!DefineElement(cx, res, i++, element))
             return false;
-        i++;
     } while (true);
 
     args.rval().setObject(*res);
     return true;
 }
 
 bool
 js::intl_GetCalendarInfo(JSContext* cx, unsigned argc, Value* vp)
@@ -4312,27 +4263,29 @@ ComputeSingleDisplayName(JSContext* cx, 
             return nullptr;
         }
 
         UErrorCode status = U_ZERO_ERROR;
         int32_t resultSize =
             udat_getSymbols(fmt, symbolType, index, Char16ToUChar(chars.begin()),
                             INITIAL_CHAR_BUFFER_SIZE, &status);
         if (status == U_BUFFER_OVERFLOW_ERROR) {
+            MOZ_ASSERT(resultSize >= 0);
             if (!chars.resize(resultSize))
                 return nullptr;
             status = U_ZERO_ERROR;
             udat_getSymbols(fmt, symbolType, index, Char16ToUChar(chars.begin()),
                             resultSize, &status);
         }
         if (U_FAILURE(status)) {
             JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_INTERNAL_INTL_ERROR);
             return nullptr;
         }
 
+        MOZ_ASSERT(resultSize >= 0);
         return NewStringCopyN<CanGC>(cx, chars.begin(), resultSize);
     }
 
     ReportBadKey(cx, pattern);
     return nullptr;
 }
 
 bool
@@ -4345,28 +4298,27 @@ js::intl_ComputeDisplayNames(JSContext* 
 
     // 1. Assert: locale is a string.
     str = args[0].toString();
     JSAutoByteString locale;
     if (!locale.encodeUtf8(cx, str))
         return false;
 
     // 2. Assert: style is a string.
-    str = args[1].toString();
-    JSAutoByteString style;
-    if (!style.encodeUtf8(cx, str))
+    JSLinearString* style = args[1].toString()->ensureLinear(cx);
+    if (!style)
         return false;
 
     DisplayNameStyle dnStyle;
-    if (equal(style, "narrow")) {
+    if (StringEqualsAscii(style, "narrow")) {
         dnStyle = DisplayNameStyle::Narrow;
-    } else if (equal(style, "short")) {
+    } else if (StringEqualsAscii(style, "short")) {
         dnStyle = DisplayNameStyle::Short;
     } else {
-        MOZ_ASSERT(equal(style, "long"));
+        MOZ_ASSERT(StringEqualsAscii(style, "long"));
         dnStyle = DisplayNameStyle::Long;
     }
 
     // 3. Assert: keys is an Array.
     RootedArrayObject keys(cx, &args[2].toObject().as<ArrayObject>());
     if (!keys)
         return false;
 
--- a/js/src/builtin/Intl.h
+++ b/js/src/builtin/Intl.h
@@ -361,26 +361,16 @@ intl_patternForSkeleton(JSContext* cx, u
  * Usage: formatted = intl_FormatDateTime(dateTimeFormat, x)
  */
 extern MOZ_MUST_USE bool
 intl_FormatDateTime(JSContext* cx, unsigned argc, Value* vp);
 
 /******************** PluralRules ********************/
 
 /**
- * Returns a new PluralRules instance.
- * Self-hosted code cannot cache this constructor (as it does for others in
- * Utilities.js) because it is initialized after self-hosted code is compiled.
- *
- * Usage: pluralRules = intl_PluralRules(locales, options)
- */
-extern MOZ_MUST_USE bool
-intl_PluralRules(JSContext* cx, unsigned argc, Value* vp);
-
-/**
  * Returns an object indicating the supported locales for plural rules
  * by having a true-valued property for each such locale with the
  * canonicalized language tag as the property name. The object has no
  * prototype.
  *
  * Usage: availableLocales = intl_PluralRules_availableLocales()
  */
 extern MOZ_MUST_USE bool