Attachment 190597 Details for Bug 111002 – Patch

[patch] Patch

bug-111002-20130227135003.patch (text/plain), 11.95 KB, created by Alec Flett on 2013-02-27 13:53:50 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Alec Flett

Created: 2013-02-27 13:53:50 PST

Size: 11.95 KB

patch

obsolete

>Subversion Revision: 144217
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c4f7dc8ae524ad631efc12154ad2def3da08883c..60b3c3eae6be539f524fe08797ee84927baa7890 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2013-02-27  Alec Flett  <alecflett@chromium.org>
>+
>+        IndexedDB: Avoid ScriptValue copies in IDBAny
>+        https://bugs.webkit.org/show_bug.cgi?id=111002
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This avoids some v8 handle thrashing in the long term,
>+        and protects us against some crashes in the short term.
>+
>+        The crashes will be fixed in 
>+        https://bugs.webkit.org/show_bug.cgi?id=110206.
>+
>+        * Modules/indexeddb/IDBAny.cpp:
>+        (WebCore::IDBAny::createNull):
>+        (WebCore::IDBAny::createString):
>+        (WebCore::IDBAny::IDBAny):
>+        (WebCore::IDBAny::scriptValue):
>+        * Modules/indexeddb/IDBAny.h:
>+        (WebCore::IDBAny::create):
>+        (IDBAny):
>+        (WebCore::IDBAny::keyPath):
>+        * bindings/v8/custom/V8IDBAnyCustom.cpp:
>+        (WebCore::toV8):
>+        (WebCore):
>+
> 2013-02-26  Alexey Proskuryakov  <ap@apple.com>
> 
>         Don't add a body to platform request until necessary
>diff --git a/Source/WebCore/Modules/indexeddb/IDBAny.cpp b/Source/WebCore/Modules/indexeddb/IDBAny.cpp
>index 81906068c1eecb2f9153a25d5605041beeeb611d..cb1e63ed0d2099b473f4535acc534124d508f5d7 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBAny.cpp
>+++ b/Source/WebCore/Modules/indexeddb/IDBAny.cpp
>@@ -40,26 +40,24 @@ namespace WebCore {
> 
> PassRefPtr<IDBAny> IDBAny::createInvalid()
> {
>-    return adoptRef(new IDBAny());
>+    return adoptRef(new IDBAny(UndefinedType));
> }
> 
> PassRefPtr<IDBAny> IDBAny::createNull()
> {
>-    RefPtr<IDBAny> idbAny = adoptRef(new IDBAny());
>-    idbAny->setNull();
>-    return idbAny.release();
>+    return adoptRef(new IDBAny(NullType));
> }
> 
> PassRefPtr<IDBAny> IDBAny::createString(const String& value)
> {
>-    RefPtr<IDBAny> idbAny = adoptRef(new IDBAny());
>-    idbAny->set(value);
>-    return idbAny.release();
>+    return adoptRef(new IDBAny(value));
> }
> 
>-IDBAny::IDBAny()
>-    : m_type(UndefinedType)
>+IDBAny::IDBAny(Type type)
>+    : m_type(type)
>+    , m_integer(0)
> {
>+    ASSERT(type == UndefinedType || type == NullType);
> }
> 
> IDBAny::~IDBAny()
>@@ -114,7 +112,7 @@ PassRefPtr<IDBTransaction> IDBAny::idbTransaction()
>     return m_idbTransaction;
> }
> 
>-ScriptValue IDBAny::scriptValue()
>+const ScriptValue& IDBAny::scriptValue()
> {
>     ASSERT(m_type == ScriptValueType);
>     return m_scriptValue;
>@@ -132,108 +130,87 @@ int64_t IDBAny::integer()
>     return m_integer;
> }
> 
>-void IDBAny::setNull()
>+IDBAny::IDBAny(PassRefPtr<DOMStringList> value)
>+    : m_type(DOMStringListType)
>+    , m_domStringList(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = NullType;
> }
> 
>-void IDBAny::set(PassRefPtr<DOMStringList> value)
>+IDBAny::IDBAny(PassRefPtr<IDBCursorWithValue> value)
>+    : m_type(IDBCursorWithValueType)
>+    , m_idbCursorWithValue(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = DOMStringListType;
>-    m_domStringList = value;
> }
> 
>-void IDBAny::set(PassRefPtr<IDBCursorWithValue> value)
>+IDBAny::IDBAny(PassRefPtr<IDBCursor> value)
>+    : m_type(IDBCursorType)
>+    , m_idbCursor(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IDBCursorWithValueType;
>-    m_idbCursorWithValue = value;
> }
> 
>-void IDBAny::set(PassRefPtr<IDBCursor> value)
>+IDBAny::IDBAny(PassRefPtr<IDBDatabase> value)
>+    : m_type(IDBDatabaseType)
>+    , m_idbDatabase(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IDBCursorType;
>-    m_idbCursor = value;
> }
> 
>-void IDBAny::set(PassRefPtr<IDBDatabase> value)
>+IDBAny::IDBAny(PassRefPtr<IDBFactory> value)
>+    : m_type(IDBFactoryType)
>+    , m_idbFactory(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IDBDatabaseType;
>-    m_idbDatabase = value;
> }
> 
>-void IDBAny::set(PassRefPtr<IDBFactory> value)
>+IDBAny::IDBAny(PassRefPtr<IDBIndex> value)
>+    : m_type(IDBIndexType)
>+    , m_idbIndex(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IDBFactoryType;
>-    m_idbFactory = value;
> }
> 
>-void IDBAny::set(PassRefPtr<IDBIndex> value)
>+IDBAny::IDBAny(PassRefPtr<IDBTransaction> value)
>+    : m_type(IDBTransactionType)
>+    , m_idbTransaction(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IDBIndexType;
>-    m_idbIndex = value;
> }
> 
>-void IDBAny::set(PassRefPtr<IDBTransaction> value)
>+IDBAny::IDBAny(PassRefPtr<IDBObjectStore> value)
>+    : m_type(IDBObjectStoreType)
>+    , m_idbObjectStore(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IDBTransactionType;
>-    m_idbTransaction = value;
> }
> 
>-void IDBAny::set(PassRefPtr<IDBObjectStore> value)
>+IDBAny::IDBAny(const ScriptValue& value)
>+    : m_type(ScriptValueType)
>+    , m_scriptValue(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IDBObjectStoreType;
>-    m_idbObjectStore = value;
> }
> 
>-void IDBAny::set(const ScriptValue& value)
>+IDBAny::IDBAny(const IDBKeyPath& value)
>+    : m_type(KeyPathType)
>+    , m_idbKeyPath(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = ScriptValueType;
>-    m_scriptValue = value;
> }
> 
>-void IDBAny::set(const IDBKeyPath& value)
>+IDBAny::IDBAny(const String& value)
>+    : m_type(StringType)
>+    , m_string(value)
>+    , m_integer(0)
> {
>-    ASSERT(m_type == UndefinedType);
>-    switch (value.type()) {
>-    case IDBKeyPath::NullType:
>-        m_type = NullType;
>-        break;
>-    case IDBKeyPath::StringType:
>-        m_type = StringType;
>-        m_string = value.string();
>-        break;
>-    case IDBKeyPath::ArrayType:
>-        RefPtr<DOMStringList> keyPaths = DOMStringList::create();
>-        for (Vector<String>::const_iterator it = value.array().begin(); it != value.array().end(); ++it)
>-            keyPaths->append(*it);
>-        m_type = DOMStringListType;
>-        m_domStringList = keyPaths.release();
>-        break;
>-    }
> }
> 
>-void IDBAny::set(const String& value)
>+IDBAny::IDBAny(int64_t value)
>+    : m_type(IntegerType)
>+    , m_integer(value)
> {
>-    ASSERT(m_type == UndefinedType);
>-    m_type = StringType;
>-    m_string = value;
>-}
>-
>-void IDBAny::set(int64_t value)
>-{
>-    ASSERT(m_type == UndefinedType);
>-    m_type = IntegerType;
>-    m_integer = value;
> }
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/Modules/indexeddb/IDBAny.h b/Source/WebCore/Modules/indexeddb/IDBAny.h
>index 16c2bb967d5a1083795048a31824f026e76a0180..082d5d0f1f7763e3af48f01521b363248fd02b42 100644
>--- a/Source/WebCore/Modules/indexeddb/IDBAny.h
>+++ b/Source/WebCore/Modules/indexeddb/IDBAny.h
>@@ -28,6 +28,7 @@
> 
> #if ENABLE(INDEXED_DATABASE)
> 
>+#include "IDBKeyPath.h"
> #include "ScriptValue.h"
> #include "ScriptWrappable.h"
> #include <wtf/PassRefPtr.h>
>@@ -55,29 +56,21 @@ public:
>     template<typename T>
>     static PassRefPtr<IDBAny> create(T* idbObject)
>     {
>-        RefPtr<IDBAny> any = IDBAny::createInvalid();
>-        any->set(idbObject);
>-        return any.release();
>+        return adoptRef(new IDBAny(idbObject));
>     }
>     template<typename T>
>     static PassRefPtr<IDBAny> create(const T& idbObject)
>     {
>-        RefPtr<IDBAny> any = IDBAny::createInvalid();
>-        any->set(idbObject);
>-        return any.release();
>+        return adoptRef(new IDBAny(idbObject));
>     }
>     template<typename T>
>     static PassRefPtr<IDBAny> create(PassRefPtr<T> idbObject)
>     {
>-        RefPtr<IDBAny> any = IDBAny::createInvalid();
>-        any->set(idbObject);
>-        return any.release();
>+        return adoptRef(new IDBAny(idbObject));
>     }
>     static PassRefPtr<IDBAny> create(int64_t value)
>     {
>-        RefPtr<IDBAny> any = IDBAny::createInvalid();
>-        any->set(value);
>-        return any.release();
>+        return adoptRef(new IDBAny(value));
>     }
>     ~IDBAny();
> 
>@@ -95,6 +88,7 @@ public:
>         ScriptValueType,
>         IntegerType,
>         StringType,
>+        KeyPathType,
>     };
> 
>     Type type() const { return m_type; }
>@@ -107,42 +101,41 @@ public:
>     PassRefPtr<IDBIndex> idbIndex();
>     PassRefPtr<IDBObjectStore> idbObjectStore();
>     PassRefPtr<IDBTransaction> idbTransaction();
>-    ScriptValue scriptValue();
>+    const ScriptValue& scriptValue();
>     int64_t integer();
>     const String& string();
>-
>-    // Set can only be called once.
>-    void setNull();
>-    void set(PassRefPtr<DOMStringList>);
>-    void set(PassRefPtr<IDBCursor>);
>-    void set(PassRefPtr<IDBCursorWithValue>);
>-    void set(PassRefPtr<IDBDatabase>);
>-    void set(PassRefPtr<IDBFactory>);
>-    void set(PassRefPtr<IDBIndex>);
>-    void set(PassRefPtr<IDBObjectStore>);
>-    void set(PassRefPtr<IDBTransaction>);
>-    void set(const IDBKeyPath&);
>-    void set(const String&);
>-    void set(const ScriptValue&);
>-    void set(int64_t);
>+    const IDBKeyPath& keyPath() { return m_idbKeyPath; };
> 
> private:
>-    IDBAny();
>+    IDBAny(Type);
>+    IDBAny(PassRefPtr<DOMStringList>);
>+    IDBAny(PassRefPtr<IDBCursor>);
>+    IDBAny(PassRefPtr<IDBCursorWithValue>);
>+    IDBAny(PassRefPtr<IDBDatabase>);
>+    IDBAny(PassRefPtr<IDBFactory>);
>+    IDBAny(PassRefPtr<IDBIndex>);
>+    IDBAny(PassRefPtr<IDBObjectStore>);
>+    IDBAny(PassRefPtr<IDBTransaction>);
>+    IDBAny(const IDBKeyPath&);
>+    IDBAny(const String&);
>+    IDBAny(const ScriptValue&);
>+    IDBAny(int64_t);
> 
>-    Type m_type;
>+    const Type m_type;
> 
>     // Only one of the following should ever be in use at any given time.
>-    RefPtr<DOMStringList> m_domStringList;
>-    RefPtr<IDBCursor> m_idbCursor;
>-    RefPtr<IDBCursorWithValue> m_idbCursorWithValue;
>-    RefPtr<IDBDatabase> m_idbDatabase;
>-    RefPtr<IDBFactory> m_idbFactory;
>-    RefPtr<IDBIndex> m_idbIndex;
>-    RefPtr<IDBObjectStore> m_idbObjectStore;
>-    RefPtr<IDBTransaction> m_idbTransaction;
>-    ScriptValue m_scriptValue;
>-    String m_string;
>-    int64_t m_integer;
>+    const RefPtr<DOMStringList> m_domStringList;
>+    const RefPtr<IDBCursor> m_idbCursor;
>+    const RefPtr<IDBCursorWithValue> m_idbCursorWithValue;
>+    const RefPtr<IDBDatabase> m_idbDatabase;
>+    const RefPtr<IDBFactory> m_idbFactory;
>+    const RefPtr<IDBIndex> m_idbIndex;
>+    const RefPtr<IDBObjectStore> m_idbObjectStore;
>+    const RefPtr<IDBTransaction> m_idbTransaction;
>+    const IDBKeyPath m_idbKeyPath;
>+    const ScriptValue m_scriptValue;
>+    const String m_string;
>+    const int64_t m_integer;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/bindings/v8/custom/V8IDBAnyCustom.cpp b/Source/WebCore/bindings/v8/custom/V8IDBAnyCustom.cpp
>index cf4064fdd5283072728e9989ada6376fe2c661cd..e119ee5bf017cbb545fa2438e1152bcc5cf66b2e 100644
>--- a/Source/WebCore/bindings/v8/custom/V8IDBAnyCustom.cpp
>+++ b/Source/WebCore/bindings/v8/custom/V8IDBAnyCustom.cpp
>@@ -44,6 +44,23 @@
> 
> namespace WebCore {
> 
>+static v8::Handle<v8::Value> toV8(const IDBKeyPath& value, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
>+{
>+    switch (value.type()) {
>+    case IDBKeyPath::NullType:
>+        return v8NullWithCheck(isolate);
>+    case IDBKeyPath::StringType:
>+        return v8String(value.string(), isolate);
>+    case IDBKeyPath::ArrayType:
>+        RefPtr<DOMStringList> keyPaths = DOMStringList::create();
>+        for (Vector<String>::const_iterator it = value.array().begin(); it != value.array().end(); ++it)
>+            keyPaths->append(*it);
>+        return toV8(keyPaths.release(), creationContext, isolate);
>+    }
>+    ASSERT_NOT_REACHED();
>+    return v8::Undefined();
>+}
>+
> v8::Handle<v8::Value> toV8(IDBAny* impl, v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)
> {
>     if (!impl)
>@@ -76,6 +93,8 @@ v8::Handle<v8::Value> toV8(IDBAny* impl, v8::Handle<v8::Object> creationContext,
>         return v8String(impl->string(), isolate);
>     case IDBAny::IntegerType:
>         return v8::Number::New(impl->integer());
>+    case IDBAny::KeyPathType:
>+        return toV8(impl->keyPath(), creationContext, isolate);
>     }
> 
>     ASSERT_NOT_REACHED();

Actions: View | Formatted Diff | Diff

Attachments on bug 111002: 190569 | 190589 | 190590 | 190597 | 191044