Bug 107582 - IndexedDB: IDBKeyRange::isOnlyKey() does pointer equality comparison
Summary: IndexedDB: IDBKeyRange::isOnlyKey() does pointer equality comparison
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joshua Bell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-22 13:30 PST by Joshua Bell
Modified: 2013-02-01 10:04 PST (History)
5 users (show)

See Also:

Attachments
Patch (5.48 KB, patch)
2013-01-30 14:25 PST, Joshua Bell 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Joshua Bell 2013-01-22 13:30:56 PST 
To simplify the backend API, various methods that can take a key-or-keyrange just accept a keyrange, and we map a script call with just a key to IDBKeyRange::only(key). On the back end, optimized paths can check keyrange->isOnlyKey()

IDBKeyRange::isOnlyKey() does pointer equality comparison, so if there are IPC hijinks between the front- and back-end (e.g. in multiprocess ports) that don't pass along this flag, the equality comparison would fail and the unoptimized path would be used.

It looks like the only caller is in GetOperation::perform() in IDBDatabaseBackendImpl.cpp

A trivial fix would be to replace the equality test with: !m_lower->compare(m_upper)
Comment 1 Joshua Bell 2013-01-30 14:25:40 PST 
Created attachment 185573 [details]
Patch
Comment 2 Joshua Bell 2013-01-30 14:26:02 PST 
alecflett@ - can you take a look?
Comment 3 Alec Flett 2013-01-30 14:28:57 PST 
nice. lgtm
Comment 4 Joshua Bell 2013-01-30 14:33:53 PST 
tony@ - r?
Comment 5 WebKit Review Bot 2013-01-30 15:32:07 PST 
Comment on attachment 185573 [details]
Patch

Rejecting attachment 185573 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-04', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 185573, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13482 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
53>At revision 13482.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/16183937
Comment 6 Joshua Bell 2013-01-30 15:34:33 PST 
Comment on attachment 185573 [details]
Patch

"Merge conflict during commit: Conflict at '/trunk/Source/WebCore/ChangeLog' at /usr/lib/git-core/git-svn line 570"

Just giving it another try.
Comment 7 WebKit Review Bot 2013-01-30 16:27:13 PST 
Comment on attachment 185573 [details]
Patch

Clearing flags on attachment: 185573

Committed r141338: <http://trac.webkit.org/changeset/141338>
Comment 8 WebKit Review Bot 2013-01-30 16:27:16 PST 
All reviewed patches have been landed.  Closing bug.
Comment 9 Joshua Bell 2013-02-01 10:04:35 PST 
And for the record, this graph shows the associated perf boost pretty well:

http://build.chromium.org/f/chromium/perf/chromium-rel-win7-dual/idb_perf/report.html?history=150&rev=180143&graph=testReadCache_50_true