Bug 150939

Summary: Modern IDB: Make indexes actually index
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, buildbot, commit-queue, darin, rniwa
Priority: P2    
Version: Safari 9   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 150910, 151014    
Bug Blocks: 149117    
Attachments:
Description Flags
Patch v1
achristensen: review+, buildbot: commit-queue-
Archive of layout-test-results from ews107 for mac-mavericks-wk2
none
Patch for landing none

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