Bug 150939 - Modern IDB: Make indexes actually index
Summary: Modern IDB: Make indexes actually index
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari 9
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on: 150910 151014
Blocks: 149117
  Show dependency tree / graph
 
Reported: 2015-11-05 11:30 PST by Brady Eidson
Modified: 2015-11-19 13:59 PST (History)
5 users (show)

See Also:

Attachments
Patch v1 (91.95 KB, patch)
2015-11-10 16:01 PST, Brady Eidson 		achristensen: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-mavericks-wk2 (942.05 KB, application/zip)
2015-11-10 16:50 PST, Build Bot 		no flags Details
Patch for landing (87.71 KB, patch)
2015-11-10 20:35 PST, Brady Eidson 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Brady Eidson 2015-11-05 11:30:04 PST 
Modern IDB: Make indexes actually index
Comment 1 Brady Eidson 2015-11-10 16:01:31 PST 
Created attachment 265241 [details]
Patch v1
Comment 2 Build Bot 2015-11-10 16:50:11 PST 
Comment on attachment 265241 [details]
Patch v1

Attachment 265241 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/412198

New failing tests:
fast/hidpi/image-set-border-image-simple.html
Comment 3 Build Bot 2015-11-10 16:50:13 PST 
Created attachment 265245 [details]
Archive of layout-test-results from ews107 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5
Comment 4 Alex Christensen 2015-11-10 17:58:09 PST 
Comment on attachment 265241 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=265241&action=review

r=me

> Source/WebCore/Modules/indexeddb/shared/IndexKey.cpp:45
> +    Vector<IDBKeyData> keys;
> +    keys.reserveCapacity(m_keys.size());

reserveInitialCapacity
Comment 5 Brady Eidson 2015-11-10 20:28:37 PST 
(In reply to comment #4)
> Comment on attachment 265241 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265241&action=review
> 
> r=me
> 
> > Source/WebCore/Modules/indexeddb/shared/IndexKey.cpp:45
> > +    Vector<IDBKeyData> keys;
> > +    keys.reserveCapacity(m_keys.size());
> 
> reserveInitialCapacity

Good call. Thx
Comment 6 Brady Eidson 2015-11-10 20:35:05 PST 
Created attachment 265263 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2015-11-10 22:07:04 PST 
Comment on attachment 265263 [details]
Patch for landing

Clearing flags on attachment: 265263

Committed r192294: <http://trac.webkit.org/changeset/192294>
Comment 8 Darin Adler 2015-11-19 13:44:34 PST 
Comment on attachment 265241 [details]
Patch v1

View in context: https://bugs.webkit.org/attachment.cgi?id=265241&action=review

> Source/WebCore/Modules/indexeddb/server/IndexValueEntry.cpp:64
> +    if (m_unique && m_key && *m_key == key) {

This is wrong when m_unique is true but m_key is null or not equal. It will fall into the code below and deterrence the pointer as the wrong type!
Comment 9 Darin Adler 2015-11-19 13:45:51 PST 
Comment on attachment 265263 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=265263&action=review

> Source/WebCore/Modules/indexeddb/server/IndexValueEntry.cpp:64
> +    if (m_unique && m_key && *m_key == key) {

Still wrong here. Need a test case for this and need to fix this by using a nested if rather than just a single one.
Comment 10 Darin Adler 2015-11-19 13:52:32 PST 
deterrence -> dereference
Comment 11 Brady Eidson 2015-11-19 13:59:28 PST 
(In reply to comment #9)
> Comment on attachment 265263 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265263&action=review
> 
> > Source/WebCore/Modules/indexeddb/server/IndexValueEntry.cpp:64
> > +    if (m_unique && m_key && *m_key == key) {
> 
> Still wrong here. Need a test case for this and need to fix this by using a
> nested if rather than just a single one.

This patch landed with this bug as you pointed out.

*BUT* it was caught in a new test and fixed in https://trac.webkit.org/changeset/192610