Title: [141338] trunk/Source/WebCore
Revision
141338
Author
jsb...@chromium.org
Date
2013-01-30 16:25:37 -0800 (Wed, 30 Jan 2013)

Log 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.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (141337 => 141338)


--- trunk/Source/WebCore/ChangeLog	2013-01-31 00:05:31 UTC (rev 141337)
+++ trunk/Source/WebCore/ChangeLog	2013-01-31 00:25:37 UTC (rev 141338)
@@ -1,3 +1,35 @@
+2013-01-30  Joshua Bell  <jsb...@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  <jma...@rim.com>
 
         [BlackBerry] Never store empty credentials in NetworkJob::storeCredentials

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp (141337 => 141338)


--- trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp	2013-01-31 00:05:31 UTC (rev 141337)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp	2013-01-31 00:25:37 UTC (rev 141338)
@@ -790,6 +790,8 @@
         INTERNAL_READ_ERROR(GetRecord);
         return false;
     }
+    if (!found)
+        return true;
 
     int64_t version;
     const char* p = decodeVarInt(data.begin(), data.end(), version);

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp (141337 => 141338)


--- trunk/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp	2013-01-31 00:05:31 UTC (rev 141337)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp	2013-01-31 00:25:37 UTC (rev 141338)
@@ -755,6 +755,10 @@
         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());

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.cpp (141337 => 141338)


--- trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.cpp	2013-01-31 00:05:31 UTC (rev 141337)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.cpp	2013-01-31 00:25:37 UTC (rev 141338)
@@ -130,6 +130,16 @@
     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)

Modified: trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.h (141337 => 141338)


--- trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.h	2013-01-31 00:05:31 UTC (rev 141337)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBKeyRange.h	2013-01-31 00:25:37 UTC (rev 141338)
@@ -77,10 +77,7 @@
     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);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to