Changeset 49840 in webkit


Ignore:
Timestamp:
Oct 19, 2009 10:01:32 PM (15 years ago)
Author:
abarth@webkit.org
Message:

2009-10-19 James Robinson <​jamesr@chromium.org>

Reviewed by Adam Barth.

Better implementation of WebCore::ScriptString for the V8 bindings.

​https://bugs.webkit.org/show_bug.cgi?id=29909

WebCore::ScriptString is used for XMLHttpRequest's responseText attribute which is
shared with JavaScript. Thus, simply using a WebCore::String and copying the value
is pretty inefficient, especially since responseText is built up with a sequence of
operator+= calls. JSC builds use a JSC::UString to share the buffer when possible,
this patch adopts a similar approach for V8.

No new tests, behavior is unchanged and covered by LayoutTests/http/tests/xmlhttprequest

  • WebCore.gypi:
  • bindings/v8/ScriptString.h: (WebCore::ScriptString::ScriptString): (WebCore::ScriptString::operator String): (WebCore::ScriptString::isNull): (WebCore::ScriptString::size): (WebCore::ScriptString::operator=): (WebCore::ScriptString::operator+=): (WebCore::ScriptString::v8StringOrNull):
  • bindings/v8/ScriptStringImpl.cpp: Added. (WebCore::ScriptStringImpl::ScriptStringImpl): (WebCore::ScriptStringImpl::~ScriptStringImpl): (WebCore::ScriptStringImpl::toString): (WebCore::ScriptStringImpl::isNull): (WebCore::ScriptStringImpl::size): (WebCore::ScriptStringImpl::append):
  • bindings/v8/ScriptStringImpl.h: Added. (WebCore::ScriptStringImpl::ScriptStringImpl): (WebCore::ScriptStringImpl::v8StringHandle):
  • bindings/v8/custom/V8XMLHttpRequestCustom.cpp: (WebCore::ACCESSOR_GETTER):
Location:
trunk/WebCore
Files:
4 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r49839 r49840  
     12009-10-19  James Robinson  <jamesr@chromium.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        Better implementation of WebCore::ScriptString for the V8 bindings.
     6
     7        https://bugs.webkit.org/show_bug.cgi?id=29909
     8
     9        WebCore::ScriptString is used for XMLHttpRequest's responseText attribute which is
     10        shared with JavaScript.  Thus, simply using a WebCore::String and copying the value
     11        is pretty inefficient, especially since responseText is built up with a sequence of
     12        operator+= calls.  JSC builds use a JSC::UString to share the buffer when possible,
     13        this patch adopts a similar approach for V8.
     14
     15        No new tests, behavior is unchanged and covered by LayoutTests/http/tests/xmlhttprequest
     16
     17        * WebCore.gypi:
     18        * bindings/v8/ScriptString.h:
     19        (WebCore::ScriptString::ScriptString):
     20        (WebCore::ScriptString::operator String):
     21        (WebCore::ScriptString::isNull):
     22        (WebCore::ScriptString::size):
     23        (WebCore::ScriptString::operator=):
     24        (WebCore::ScriptString::operator+=):
     25        (WebCore::ScriptString::v8StringOrNull):
     26        * bindings/v8/ScriptStringImpl.cpp: Added.
     27        (WebCore::ScriptStringImpl::ScriptStringImpl):
     28        (WebCore::ScriptStringImpl::~ScriptStringImpl):
     29        (WebCore::ScriptStringImpl::toString):
     30        (WebCore::ScriptStringImpl::isNull):
     31        (WebCore::ScriptStringImpl::size):
     32        (WebCore::ScriptStringImpl::append):
     33        * bindings/v8/ScriptStringImpl.h: Added.
     34        (WebCore::ScriptStringImpl::ScriptStringImpl):
     35        (WebCore::ScriptStringImpl::v8StringHandle):
     36        * bindings/v8/custom/V8XMLHttpRequestCustom.cpp:
     37        (WebCore::ACCESSOR_GETTER):
     38
    1392009-10-19  Adam Barth  <abarth@webkit.org>
    240

  • trunk/WebCore/WebCore.gypi

    r49778 r49840  
    772772            'bindings/v8/ScriptState.cpp',
    773773            'bindings/v8/ScriptString.h',
     774            'bindings/v8/ScriptStringImpl.cpp',
     775            'bindings/v8/ScriptStringImpl.h',
    774776            'bindings/v8/ScriptValue.cpp',
    775777            'bindings/v8/ScriptValue.h',

  • trunk/WebCore/bindings/v8/ScriptString.h

    r42568 r49840  
    3333
    3434#include "PlatformString.h"
     35#include "ScriptStringImpl.h"
     36#include "V8Binding.h"
    3537
    3638namespace WebCore {
    … …  
    3840class ScriptString {
    3941public:
    40     ScriptString() {}
    41     ScriptString(const String& s) : m_str(s) {}
    42     ScriptString(const char* s) : m_str(s) {}
     42    ScriptString() : m_impl(0) {}
     43    ScriptString(const String& s) : m_impl(new ScriptStringImpl(s)) {}
     44    ScriptString(const char* s) : m_impl(new ScriptStringImpl(s)) {}
    4345
    44     operator String() const { return m_str; }
     46    operator String() const { return m_impl->toString(); }
    4547
    46     bool isNull() const { return m_str.isNull(); }
    47     size_t size() const { return m_str.length(); }
     48    bool isNull() const { return !m_impl.get() || m_impl->isNull(); }
     49    size_t size() const { return m_impl->size(); }
    4850
    4951    ScriptString& operator=(const char* s)
    5052    {
    51         m_str = s;
     53        m_impl = new ScriptStringImpl(s);
    5254        return *this;
    5355    }
    … …  
    5557    ScriptString& operator+=(const String& s)
    5658    {
    57         m_str += s;
     59        m_impl->append(s);
    5860        return *this;
    5961    }
    6062
     63    v8::Handle<v8::Value> v8StringOrNull() const
     64    {
     65        return isNull() ? v8::Handle<v8::Value>(v8::Null()) : v8::Handle<v8::Value>(m_impl->v8StringHandle());
     66    }
     67
    6168private:
    62     String m_str;
     69    RefPtr<ScriptStringImpl> m_impl;
    6370};
    6471

  • trunk/WebCore/bindings/v8/ScriptStringImpl.cpp

    r49839 r49840  
    2929 */
    3030
    31 #ifndef ScriptString_h
    32 #define ScriptString_h
     31#include "config.h"
     32#include "ScriptStringImpl.h"
    3333
    34 #include "PlatformString.h"
     34#include "V8Binding.h"
    3535
    3636namespace WebCore {
    3737
    38 class ScriptString {
    39 public:
    40     ScriptString() {}
    41     ScriptString(const String& s) : m_str(s) {}
    42     ScriptString(const char* s) : m_str(s) {}
     38ScriptStringImpl::ScriptStringImpl(const String& s)
     39{
     40    v8::HandleScope scope;
     41    m_handle.set(v8String(s));
     42}
    4343
    44     operator String() const { return m_str; }
     44ScriptStringImpl::ScriptStringImpl(const char* s)
     45{
     46    v8::HandleScope scope;
     47    m_handle.set(v8::String::New(s));
     48}
    4549
    46     bool isNull() const { return m_str.isNull(); }
    47     size_t size() const { return m_str.length(); }
     50String ScriptStringImpl::toString() const
     51{
     52    return v8StringToWebCoreString(m_handle.get());
     53}
    4854
    49     ScriptString& operator=(const char* s)
    50     {
    51         m_str = s;
    52         return *this;
    53     }
     55bool ScriptStringImpl::isNull() const
     56{
     57    return m_handle.get().IsEmpty();
     58}
    5459
    55     ScriptString& operator+=(const String& s)
    56     {
    57         m_str += s;
    58         return *this;
    59     }
     60size_t ScriptStringImpl::size() const
     61{
     62    return m_handle.get()->Length();
     63}
    6064
    61 private:
    62     String m_str;
    63 };
     65void ScriptStringImpl::append(const String& s)
     66{
     67    v8::HandleScope scope;
     68    if (m_handle.get().IsEmpty())
     69        m_handle.set(v8String(s));
     70    else
     71        m_handle.set(v8::String::Concat(m_handle.get(), v8String(s)));
     72}
    6473
    6574} // namespace WebCore
    66 
    67 #endif // ScriptString_h

  • trunk/WebCore/bindings/v8/ScriptStringImpl.h

    r49839 r49840  
    2929 */
    3030
    31 #ifndef ScriptString_h
    32 #define ScriptString_h
     31#ifndef ScriptStringImpl_h
     32#define ScriptStringImpl_h
    3333
     34#include "OwnHandle.h"
    3435#include "PlatformString.h"
     36
     37#include <v8.h>
    3538
    3639namespace WebCore {
    3740
    38 class ScriptString {
     41// This class is used for strings that tend to be shared with JavaScript frequently.  The JSC implementation uses wtf::UString - see bindings/js/ScriptString.h
     42// Currently XMLHttpRequest uses a ScriptString to build up the responseText attribute.  As data arrives from the network, it is appended to the ScriptString
     43// via operator+= and a JavaScript readystatechange event is fired.  JavaScript can access the responseText attribute of the XMLHttpRequest object.  JavaScript
     44// may also query the responseXML attribute of the XMLHttpRequest object which results in the responseText attribute being coerced into a WebCore::String and
     45// then parsed as an XML document.
     46// This implementation optimizes for the common case where the responseText is built up with many calls to operator+= before the actual text is queried.
     47class ScriptStringImpl : public RefCounted<ScriptStringImpl> {
    3948public:
    40     ScriptString() {}
    41     ScriptString(const String& s) : m_str(s) {}
    42     ScriptString(const char* s) : m_str(s) {}
     49    ScriptStringImpl() {}
     50    ScriptStringImpl(const String& s);
     51    ScriptStringImpl(const char* s);
    4352
    44     operator String() const { return m_str; }
     53    String toString() const;
    4554
    46     bool isNull() const { return m_str.isNull(); }
    47     size_t size() const { return m_str.length(); }
     55    bool isNull() const;
     56    size_t size() const;
    4857
    49     ScriptString& operator=(const char* s)
    50     {
    51         m_str = s;
    52         return *this;
    53     }
     58    void append(const String& s);
    5459
    55     ScriptString& operator+=(const String& s)
    56     {
    57         m_str += s;
    58         return *this;
    59     }
     60    v8::Handle<v8::String> v8StringHandle() { return m_handle.get(); }
    6061
    6162private:
    62     String m_str;
     63    OwnHandle<v8::String> m_handle;
    6364};
    6465
    6566} // namespace WebCore
    6667
    67 #endif // ScriptString_h
     68#endif // ScriptStringImpl_h

  • trunk/WebCore/bindings/v8/custom/V8XMLHttpRequestCustom.cpp

    r48978 r49840  
    4747ACCESSOR_GETTER(XMLHttpRequestResponseText)
    4848{
    49     // FIXME: This is only needed because webkit set this getter as custom.
    50     // So we need a custom method to avoid forking the IDL file.
    5149    INC_STATS("DOM.XMLHttpRequest.responsetext._get");
    5250    XMLHttpRequest* xmlHttpRequest = V8DOMWrapper::convertToNativeObject<XMLHttpRequest>(V8ClassIndex::XMLHTTPREQUEST, info.Holder());
    53     return v8StringOrNull(xmlHttpRequest->responseText());
     51    return xmlHttpRequest->responseText().v8StringOrNull();
    5452}
    5553
Note: See TracChangeset for help on using the changeset viewer.