Ignore:
Timestamp:
Apr 11, 2008, 12:37:33 AM (17 years ago)
Author:
[email protected]
Message:

Reviewed by Geoff.

​https://bugs.webkit.org/show_bug.cgi?id=18402
REGRESSION: visited element handling is incorrect in nested join/toString calls

No change on SunSpider total, possibly a tiny improvement (about 0.1%).

Test: fast/js/array-tostring-and-join.html

  • kjs/JSGlobalObject.h: (KJS::JSGlobalObject::visitedElements): Store visited elements HashSet here, making it common to toString/toLocalizedString/join again.
  • kjs/array_object.cpp: (KJS::arrayProtoFuncToString): (KJS::arrayProtoFuncToLocaleString): (KJS::arrayProtoFuncJoin): Got rid of static variables. Replaced UString with Vector to avoid O(n2) behavior and regain performance.
  • wtf/Vector.h: (WTF::::resize): (WTF::::grow): (WTF::::reserveCapacity): (WTF::::append): (WTF::::insert): Added null checks, so that Vector methods don't crash when out of memory. The caller should check that data pointer is not null before proceeding.
File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/kjs/array_object.cpp

    r31746 r31807  
    9393        return throwError(exec, TypeError);
    9494
    95     static HashSet<JSObject*> visitedElems;
    96     static const UString* empty = new UString("");
    97     static const UString* comma = new UString(",");
    98     bool alreadyVisited = !visitedElems.add(thisObj).second;
     95    bool alreadyVisited = !exec->dynamicGlobalObject()->arrayVisitedElements().add(thisObj).second;
     96    Vector<UChar, 256> strBuffer;
    9997    if (alreadyVisited)
    100         return jsString(*empty);
    101     UString separator = *comma;
    102     UString str = *empty;
     98        return jsString(UString(0, 0)); // return an empty string, avoding infinite recursion.
    10399
    104100    unsigned length = thisObj->get(exec, exec->propertyNames().length)->toUInt32(exec);
    105101    for (unsigned k = 0; k < length; k++) {
    106102        if (k >= 1)
    107             str += separator;
    108         if (str.isNull()) {
     103            strBuffer.append(',');
     104        if (!strBuffer.data()) {
    109105            JSObject* error = Error::create(exec, GeneralError, "Out of memory");
    110106            exec->setException(error);
    … …  
    116112            continue;
    117113
    118         str += element->toString(exec);
    119 
    120         if (str.isNull()) {
     114        UString str = element->toString(exec);
     115        strBuffer.append(str.data(), str.size());
     116
     117        if (!strBuffer.data()) {
    121118            JSObject* error = Error::create(exec, GeneralError, "Out of memory");
    122119            exec->setException(error);
    … …  
    126123            break;
    127124    }
    128     visitedElems.remove(thisObj);
    129     return jsString(str);
     125    exec->dynamicGlobalObject()->arrayVisitedElements().remove(thisObj);
     126    return jsString(UString(strBuffer.data(), strBuffer.data() ? strBuffer.size() : 0));
    130127}
    131128
    … …  
    135132        return throwError(exec, TypeError);
    136133
    137     static HashSet<JSObject*> visitedElems;
    138     static const UString* empty = new UString("");
    139     static const UString* comma = new UString(",");
    140     bool alreadyVisited = !visitedElems.add(thisObj).second;
     134    bool alreadyVisited = !exec->dynamicGlobalObject()->arrayVisitedElements().add(thisObj).second;
     135    Vector<UChar, 256> strBuffer;
    141136    if (alreadyVisited)
    142         return jsString(*empty);
    143     UString separator = *comma;
    144     UString str = *empty;
     137        return jsString(UString(0, 0)); // return an empty string, avoding infinite recursion.
    145138
    146139    unsigned length = thisObj->get(exec, exec->propertyNames().length)->toUInt32(exec);
    147140    for (unsigned k = 0; k < length; k++) {
    148141        if (k >= 1)
    149             str += separator;
    150         if (str.isNull()) {
     142            strBuffer.append(',');
     143        if (!strBuffer.data()) {
    151144            JSObject* error = Error::create(exec, GeneralError, "Out of memory");
    152145            exec->setException(error);
    … …  
    160153        JSObject* o = element->toObject(exec);
    161154        JSValue* conversionFunction = o->get(exec, exec->propertyNames().toLocaleString);
     155        UString str;
    162156        if (conversionFunction->isObject() && static_cast<JSObject*>(conversionFunction)->implementsCall())
    163             str += static_cast<JSObject*>(conversionFunction)->call(exec, o, exec->emptyList())->toString(exec);
     157            str = static_cast<JSObject*>(conversionFunction)->call(exec, o, exec->emptyList())->toString(exec);
    164158        else
    165             str += element->toString(exec);
    166 
    167         if (str.isNull()) {
     159            str = element->toString(exec);
     160        strBuffer.append(str.data(), str.size());
     161
     162        if (!strBuffer.data()) {
    168163            JSObject* error = Error::create(exec, GeneralError, "Out of memory");
    169164            exec->setException(error);
    … …  
    173168            break;
    174169    }
    175     visitedElems.remove(thisObj);
    176     return jsString(str);
     170    exec->dynamicGlobalObject()->arrayVisitedElements().remove(thisObj);
     171    return jsString(UString(strBuffer.data(), strBuffer.data() ? strBuffer.size() : 0));
    177172}
    178173
    179174JSValue* arrayProtoFuncJoin(ExecState* exec, JSObject* thisObj, const List& args)
    180175{
    181     static HashSet<JSObject*> visitedElems;
    182     static const UString* empty = new UString("");
    183     static const UString* comma = new UString(",");
    184     bool alreadyVisited = !visitedElems.add(thisObj).second;
     176    bool alreadyVisited = !exec->dynamicGlobalObject()->arrayVisitedElements().add(thisObj).second;
     177    Vector<UChar, 256> strBuffer;
    185178    if (alreadyVisited)
    186         return jsString(*empty);
    187     UString separator = *comma;
    188     UString str = *empty;
    189 
    190     if (!args[0]->isUndefined())
    191         separator = args[0]->toString(exec);
     179        return jsString(UString(0, 0)); // return an empty string, avoding infinite recursion.
     180
     181    UChar comma = ',';
     182    UString separator = args[0]->isUndefined() ? UString(&comma, 1) : args[0]->toString(exec);
    192183
    193184    unsigned length = thisObj->get(exec, exec->propertyNames().length)->toUInt32(exec);
    194185    for (unsigned k = 0; k < length; k++) {
    195186        if (k >= 1)
    196             str += separator;
    197         if (str.isNull()) {
     187            strBuffer.append(separator.data(), separator.size());
     188        if (!strBuffer.data()) {
    198189            JSObject* error = Error::create(exec, GeneralError, "Out of memory");
    199190            exec->setException(error);
    … …  
    205196            continue;
    206197
    207         str += element->toString(exec);
    208 
    209         if (str.isNull()) {
     198        UString str = element->toString(exec);
     199        strBuffer.append(str.data(), str.size());
     200
     201        if (!strBuffer.data()) {
    210202            JSObject* error = Error::create(exec, GeneralError, "Out of memory");
    211203            exec->setException(error);
    … …  
    215207            break;
    216208    }
    217     visitedElems.remove(thisObj);
    218     return jsString(str);
     209    exec->dynamicGlobalObject()->arrayVisitedElements().remove(thisObj);
     210    return jsString(UString(strBuffer.data(), strBuffer.data() ? strBuffer.size() : 0));
    219211}
    220212
Note: See TracChangeset for help on using the changeset viewer.