IndexedDB: Replace IDL operation overloading with ScriptValue handling

Source/modules/indexeddb/IDBObjectStore.cpp

Index: Source/modules/indexeddb/IDBObjectStore.cpp
diff --git a/Source/modules/indexeddb/IDBObjectStore.cpp b/Source/modules/indexeddb/IDBObjectStore.cpp
index 203f3d73bf5f21895b9f8b282a6de6b314febe35..8f6b4ae18ea3e23b55196b426ef67cfdd821d4a1 100644
--- a/Source/modules/indexeddb/IDBObjectStore.cpp
+++ b/Source/modules/indexeddb/IDBObjectStore.cpp
@@ -70,17 +70,13 @@ PassRefPtr<DOMStringList> IDBObjectStore::indexNames() const
     return indexNames.release();
 }
 
-PassRefPtr<IDBRequest> IDBObjectStore::get(ScriptExecutionContext* context, PassRefPtr<IDBKeyRange> keyRange, ExceptionState& es)
+PassRefPtr<IDBRequest> IDBObjectStore::get(ScriptExecutionContext* context, const ScriptValue& key, ExceptionState& es)
 {
     IDB_TRACE("IDBObjectStore::get");
     if (isDeleted()) {
         es.throwDOMException(InvalidStateError, IDBDatabase::objectStoreDeletedErrorMessage);
         return 0;
     }
-    if (!keyRange) {
-        es.throwDOMException(DataError, IDBDatabase::noKeyOrKeyRangeErrorMessage);
-        return 0;
-    }
     if (m_transaction->isFinished()) {
         es.throwDOMException(TransactionInactiveError, IDBDatabase::transactionFinishedErrorMessage);
         return 0;
@@ -89,19 +85,19 @@ PassRefPtr<IDBRequest> IDBObjectStore::get(ScriptExecutionContext* context, Pass
         es.throwDOMException(TransactionInactiveError, IDBDatabase::transactionInactiveErrorMessage);
         return 0;
     }
+    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::fromScriptValue(context, key, es);
+    if (es.hadException())
+        return 0;
+    if (!keyRange) {
+        es.throwDOMException(DataError, IDBDatabase::noKeyOrKeyRangeErrorMessage);
+        return 0;
+    }
+
     RefPtr<IDBRequest> request = IDBRequest::create(context, IDBAny::create(this), m_transaction.get());
     backendDB()->get(m_transaction->id(), id(), IDBIndexMetadata::InvalidId, keyRange, false, request);
     return request.release();
 }
 
-PassRefPtr<IDBRequest> IDBObjectStore::get(ScriptExecutionContext* context, const ScriptValue& key, ExceptionState& es)
-{
-    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::only(context, key, es);
-    if (es.hadException())
-        return 0;
-    return get(context, keyRange.release(), es);
-}
-
 static void generateIndexKeysForValue(DOMRequestState* requestState, const IDBIndexMetadata& indexMetadata, const ScriptValue& objectValue, IDBObjectStore::IndexKeys* indexKeys)
 {
     ASSERT(indexKeys);
@@ -233,7 +229,7 @@ PassRefPtr<IDBRequest> IDBObjectStore::put(IDBDatabaseBackendInterface::PutMode
     return request.release();
 }
 
-PassRefPtr<IDBRequest> IDBObjectStore::deleteFunction(ScriptExecutionContext* context, PassRefPtr<IDBKeyRange> keyRange, ExceptionState& es)
+PassRefPtr<IDBRequest> IDBObjectStore::deleteFunction(ScriptExecutionContext* context, const ScriptValue& key, ExceptionState& es)
 {
     IDB_TRACE("IDBObjectStore::delete");
     if (isDeleted()) {
@@ -252,6 +248,10 @@ PassRefPtr<IDBRequest> IDBObjectStore::deleteFunction(ScriptExecutionContext* co
         es.throwDOMException(ReadOnlyError);
         return 0;
     }
+
+    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::fromScriptValue(context, key, es);
+    if (es.hadException())
+        return 0;
     if (!keyRange) {
         es.throwDOMException(DataError, IDBDatabase::noKeyOrKeyRangeErrorMessage);
         return 0;
@@ -262,14 +262,6 @@ PassRefPtr<IDBRequest> IDBObjectStore::deleteFunction(ScriptExecutionContext* co
     return request.release();
 }
 
-PassRefPtr<IDBRequest> IDBObjectStore::deleteFunction(ScriptExecutionContext* context, const ScriptValue& key, ExceptionState& es)
-{
-    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::only(context, key, es);
-    if (es.hadException())
-        return 0;
-    return deleteFunction(context, keyRange.release(), es);
-}
-
 PassRefPtr<IDBRequest> IDBObjectStore::clear(ScriptExecutionContext* context, ExceptionState& es)
 {
     IDB_TRACE("IDBObjectStore::clear");
@@ -428,10 +420,7 @@ PassRefPtr<IDBIndex> IDBObjectStore::createIndex(ScriptExecutionContext* context
     if (es.hadException())
         return 0;
 
-    RefPtr<IDBRequest> indexRequest = openCursor(context, static_cast<IDBKeyRange*>(0), IDBCursor::directionNext(), IDBDatabaseBackendInterface::PreemptiveTask, es);
-    ASSERT(!es.hadException());
-    if (es.hadException())
-        return 0;

[haraken, 2013/08/25 23:51:06]

Why can you remove these checks?

[jsbell, 2013/08/26 16:06:18]

This change switches from calling a script-exposed method to an "internal"
method. The new "internal" openCursor() method is used from all call sites after
the checks, and doesn't take an ExceptionState.

The script-exposed openCursor() checks that the index was in a valid state for
operations (duplicating the checks at the top of this method) and that the
direction string is valid (but now the enum value is passed).

+    RefPtr<IDBRequest> indexRequest = openCursor(context, static_cast<IDBKeyRange*>(0), IndexedDB::CursorNext, IDBDatabaseBackendInterface::PreemptiveTask);
     indexRequest->preventPropagation();
 
     // This is kept alive by being the success handler of the request, which is in turn kept alive by the owning transaction.
@@ -514,7 +503,7 @@ void IDBObjectStore::deleteIndex(const String& name, ExceptionState& es)
     }
 }
 
-PassRefPtr<IDBRequest> IDBObjectStore::openCursor(ScriptExecutionContext* context, PassRefPtr<IDBKeyRange> range, const String& directionString, IDBDatabaseBackendInterface::TaskType taskType, ExceptionState& es)
+PassRefPtr<IDBRequest> IDBObjectStore::openCursor(ScriptExecutionContext* context, const ScriptValue& range, const String& directionString, ExceptionState& es)
 {
     IDB_TRACE("IDBObjectStore::openCursor");
     if (isDeleted()) {
@@ -529,26 +518,28 @@ PassRefPtr<IDBRequest> IDBObjectStore::openCursor(ScriptExecutionContext* contex
         es.throwDOMException(TransactionInactiveError, IDBDatabase::transactionInactiveErrorMessage);
         return 0;
     }
+
     IndexedDB::CursorDirection direction = IDBCursor::stringToDirection(directionString, es);
     if (es.hadException())
         return 0;
 
-    RefPtr<IDBRequest> request = IDBRequest::create(context, IDBAny::create(this), m_transaction.get());
-    request->setCursorDetails(IndexedDB::CursorKeyAndValue, direction);
+    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::fromScriptValue(context, range, es);
+    if (es.hadException())
+        return 0;
 

[haraken, 2013/08/25 23:51:06]

Don't you need to check if(!keyRange) ?

[jsbell, 2013/08/26 16:06:18]

openCursor() can take a null range.

-    backendDB()->openCursor(m_transaction->id(), id(), IDBIndexMetadata::InvalidId, range, direction, false, static_cast<IDBDatabaseBackendInterface::TaskType>(taskType), request);
-    return request.release();
+    return openCursor(context, keyRange, direction, IDBDatabaseBackendInterface::NormalTask);
 }
 
-PassRefPtr<IDBRequest> IDBObjectStore::openCursor(ScriptExecutionContext* context, const ScriptValue& key, const String& direction, ExceptionState& es)
+PassRefPtr<IDBRequest> IDBObjectStore::openCursor(ScriptExecutionContext* context, PassRefPtr<IDBKeyRange> range, IndexedDB::CursorDirection direction, IDBDatabaseBackendInterface::TaskType taskType)
 {
-    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::only(context, key, es);
-    if (es.hadException())
-        return 0;
-    return openCursor(context, keyRange.release(), direction, es);
+    RefPtr<IDBRequest> request = IDBRequest::create(context, IDBAny::create(this), m_transaction.get());
+    request->setCursorDetails(IndexedDB::CursorKeyAndValue, direction);
+
+    backendDB()->openCursor(m_transaction->id(), id(), IDBIndexMetadata::InvalidId, range, direction, false, taskType, request);
+    return request.release();
 }
 
-PassRefPtr<IDBRequest> IDBObjectStore::count(ScriptExecutionContext* context, PassRefPtr<IDBKeyRange> range, ExceptionState& es)
+PassRefPtr<IDBRequest> IDBObjectStore::count(ScriptExecutionContext* context, const ScriptValue& range, ExceptionState& es)
 {
     IDB_TRACE("IDBObjectStore::count");
     if (isDeleted()) {
@@ -563,17 +554,14 @@ PassRefPtr<IDBRequest> IDBObjectStore::count(ScriptExecutionContext* context, Pa
         es.throwDOMException(TransactionInactiveError, IDBDatabase::transactionInactiveErrorMessage);
         return 0;
     }
-    RefPtr<IDBRequest> request = IDBRequest::create(context, IDBAny::create(this), m_transaction.get());
-    backendDB()->count(m_transaction->id(), id(), IDBIndexMetadata::InvalidId, range, request);
-    return request.release();
-}
 
-PassRefPtr<IDBRequest> IDBObjectStore::count(ScriptExecutionContext* context, const ScriptValue& key, ExceptionState& es)
-{
-    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::only(context, key, es);
+    RefPtr<IDBKeyRange> keyRange = IDBKeyRange::fromScriptValue(context, range, es);
     if (es.hadException())
         return 0;
-    return count(context, keyRange.release(), es);
+

[haraken, 2013/08/25 23:51:06]

Shall we add ASSERT(keyRange) ?

[jsbell, 2013/08/26 16:06:18]

count() can take a null range

+    RefPtr<IDBRequest> request = IDBRequest::create(context, IDBAny::create(this), m_transaction.get());
+    backendDB()->count(m_transaction->id(), id(), IDBIndexMetadata::InvalidId, keyRange, request);
+    return request.release();
 }
 
 void IDBObjectStore::transactionFinished()