IndexedDB: IDBKeyRange::isOnlyKey() does pointer equality comparison
authorjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Jan 2013 00:25:37 +0000 (00:25 +0000)
committerjsbell@chromium.org <jsbell@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 31 Jan 2013 00:25:37 +0000 (00:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=107582

Reviewed by Tony Chang.

Per the bug title, IDBKeyRange::isOnlyKey() was testing the upper/lower pointers to
determine if this was a "trivial" range, which can be used to fast-path lookups. This
is insufficient in multi-process ports where range values may be thrown across the wire.
This is addressed by using the existing key equality tests.

In addition, the bounds type check incorrectly checked m_lowerType == LowerBoundOpen, which
meant the function could never return true (since upper == lower implies closed bounds).
Therefore, the fast-path case wasn't used even in single-process ports (e.g. DRT). The
slow-path case contructed a backing store cursor over the range, and exited early if the
cursor yielded nothing. The fast-path case doesn't need to create a cursor, so needs to
deal with lookup misses. This revealed two similar (but trivial) lurking bugs in the
fast-path.

No new behavior; covered by tests such as: storage/indexeddb/get-keyrange.html

* Modules/indexeddb/IDBBackingStore.cpp:
(WebCore::IDBBackingStore::getRecord): Handle backing store read miss case.
* Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
(WebCore::GetOperation::perform): Handle backing store read miss case.
* Modules/indexeddb/IDBKeyRange.cpp:
(WebCore::IDBKeyRange::isOnlyKey): Compare keys by value, not pointer, correct
type checks, add assertions.
* Modules/indexeddb/IDBKeyRange.h:
(IDBKeyRange): Move implementation to CPP file.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@141338 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp
Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp
Source/WebCore/Modules/indexeddb/IDBKeyRange.cpp
Source/WebCore/Modules/indexeddb/IDBKeyRange.h

index 151cf9d..3e88a44 100644 (file)
@@ -1,3 +1,35 @@
+2013-01-30  Joshua Bell  <jsbell@chromium.org>
+
+        IndexedDB: IDBKeyRange::isOnlyKey() does pointer equality comparison
+        https://bugs.webkit.org/show_bug.cgi?id=107582
+
+        Reviewed by Tony Chang.
+
+        Per the bug title, IDBKeyRange::isOnlyKey() was testing the upper/lower pointers to
+        determine if this was a "trivial" range, which can be used to fast-path lookups. This
+        is insufficient in multi-process ports where range values may be thrown across the wire.
+        This is addressed by using the existing key equality tests.
+
+        In addition, the bounds type check incorrectly checked m_lowerType == LowerBoundOpen, which
+        meant the function could never return true (since upper == lower implies closed bounds).
+        Therefore, the fast-path case wasn't used even in single-process ports (e.g. DRT). The
+        slow-path case contructed a backing store cursor over the range, and exited early if the
+        cursor yielded nothing. The fast-path case doesn't need to create a cursor, so needs to
+        deal with lookup misses. This revealed two similar (but trivial) lurking bugs in the
+        fast-path.
+
+        No new behavior; covered by tests such as: storage/indexeddb/get-keyrange.html
+
+        * Modules/indexeddb/IDBBackingStore.cpp:
+        (WebCore::IDBBackingStore::getRecord): Handle backing store read miss case.
+        * Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
+        (WebCore::GetOperation::perform): Handle backing store read miss case.
+        * Modules/indexeddb/IDBKeyRange.cpp:
+        (WebCore::IDBKeyRange::isOnlyKey): Compare keys by value, not pointer, correct
+        type checks, add assertions.
+        * Modules/indexeddb/IDBKeyRange.h:
+        (IDBKeyRange): Move implementation to CPP file.
+
 2013-01-30  Joe Mason  <jmason@rim.com>
 
         [BlackBerry] Never store empty credentials in NetworkJob::storeCredentials
index da48499..5a2ce14 100644 (file)
@@ -790,6 +790,8 @@ bool IDBBackingStore::getRecord(IDBBackingStore::Transaction* transaction, int64
         INTERNAL_READ_ERROR(GetRecord);
         return false;
     }
+    if (!found)
+        return true;
 
     int64_t version;
     const char* p = decodeVarInt(data.begin(), data.end(), version);
index b468ecb..fd67640 100644 (file)
@@ -755,6 +755,10 @@ void GetOperation::perform(IDBTransactionBackendImpl* transaction)
         m_callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error in getPrimaryKeyViaIndex."));
         return;
     }
+    if (!primaryKey) {
+        m_callbacks->onSuccess();
+        return;
+    }
     if (m_cursorType == IDBCursorBackendInterface::KeyOnly) {
         // Index Value Retrieval Operation
         m_callbacks->onSuccess(primaryKey.get());
index a635b53..728f953 100644 (file)
@@ -130,6 +130,16 @@ PassRefPtr<IDBKeyRange> IDBKeyRange::bound(ScriptExecutionContext* context, cons
     return IDBKeyRange::create(lower, upper, lowerOpen ? LowerBoundOpen : LowerBoundClosed, upperOpen ? UpperBoundOpen : UpperBoundClosed);
 }
 
+bool IDBKeyRange::isOnlyKey() const
+{
+    if (m_lowerType != LowerBoundClosed || m_upperType != UpperBoundClosed)
+        return false;
+
+    ASSERT(m_lower);
+    ASSERT(m_upper);
+    return m_lower->isEqual(m_upper.get());
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)
index 81865d0..547695a 100644 (file)
@@ -77,10 +77,7 @@ public:
     static PassRefPtr<IDBKeyRange> bound(ScriptExecutionContext* context, const ScriptValue& lower, const ScriptValue& upper, bool lowerOpen, ExceptionCode& ec) { return bound(context, lower, upper, lowerOpen, false, ec); }
     static PassRefPtr<IDBKeyRange> bound(ScriptExecutionContext*, const ScriptValue& lower, const ScriptValue& upper, bool lowerOpen, bool upperOpen, ExceptionCode&);
 
-    bool isOnlyKey()
-    {
-        return m_lower == m_upper && m_lowerType == LowerBoundOpen && m_upperType == UpperBoundClosed;
-    }
+    bool isOnlyKey() const;
 
 private:
     IDBKeyRange(PassRefPtr<IDBKey> lower, PassRefPtr<IDBKey> upper, LowerBoundType lowerType, UpperBoundType upperType);