Changeset 141338 in webkit

Timestamp:

Jan 30, 2013 4:25:37 PM (11 years ago)

Author:

jsbell@chromium.org

Message:

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.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/indexeddb/IDBBackingStore.cpp (modified) (1 diff)
• Modules/indexeddb/IDBDatabaseBackendImpl.cpp (modified) (1 diff)
• Modules/indexeddb/IDBKeyRange.cpp (modified) (1 diff)
• Modules/indexeddb/IDBKeyRange.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp

@@ -791 +791 @@
         return false;
     }
+    if (!found)
+        return true;
 
     int64_t version;

trunk/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp

@@ -756 +756 @@
         return;
     }
+    if (!primaryKey) {
+        m_callbacks->onSuccess();
+        return;
+    }
     if (m_cursorType == IDBCursorBackendInterface::KeyOnly) {
         // Index Value Retrieval Operation

trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.cpp

@@ -131 +131 @@
 }
 
+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
 

trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.h

@@ -78 +78 @@
     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: