Bug 151278

Summary: Modern IDB: Make in-memory Index cursors work
Product: WebKit Reporter: Brady Eidson <beidson>
Component: WebCore Misc.Assignee: Brady Eidson <beidson>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, buildbot, commit-queue, rniwa
Priority: P2    
Version: Safari 9   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 149117    
Attachments:
Description Flags
Patch v1
buildbot: commit-queue-
Archive of layout-test-results from ews107 for mac-mavericks-wk2
none
Archive of layout-test-results from ews102 for mac-mavericks
none
Archive of layout-test-results from ews115 for mac-yosemite
none
Patch v2
achristensen: review+, achristensen: commit-queue-
Patch v3
commit-queue: commit-queue-
Patch for landing rebaselined none

Description Brady Eidson 2015-11-13 15:55:14 PST
Modern IDB:Make in-memory Index cursors work
Comment 1 Brady Eidson 2015-11-18 10:33:22 PST
Created attachment 265752 [details]
Patch v1
Comment 2 Build Bot 2015-11-18 11:17:08 PST
Comment on attachment 265752 [details]
Patch v1

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

New failing tests:
imported/w3c/indexeddb/idbcursor_iterating.htm
storage/indexeddb/cursor-reverse-bug.html
imported/w3c/indexeddb/idbcursor-continue.htm
storage/indexeddb/objectstore-cursor.html
storage/indexeddb/mozilla/indexes.html
storage/indexeddb/cursor-prev-no-duplicate.html
imported/w3c/indexeddb/idbcursor-advance.htm
storage/indexeddb/cursor-skip-deleted.html
storage/indexeddb/cursor-delete.html
storage/indexeddb/cursor-inconsistency.html
storage/indexeddb/open-cursor.html
storage/indexeddb/mozilla/cursors.html
Comment 3 Build Bot 2015-11-18 11:17:11 PST
Created attachment 265759 [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 Build Bot 2015-11-18 11:20:17 PST
Comment on attachment 265752 [details]
Patch v1

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

New failing tests:
storage/indexeddb/modern/index-cursor-3.html
Comment 5 Build Bot 2015-11-18 11:20:19 PST
Created attachment 265760 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5
Comment 6 Alex Christensen 2015-11-18 11:21:09 PST
Comment on attachment 265752 [details]
Patch v1

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

I'll finish reviewing once these failing tests are addressed.

> Source/WebCore/Modules/indexeddb/server/IndexValueEntry.cpp:137
> +    return m_entry;

!!m_entry

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:2365
> -		5E2C43741BCF0D750001E2BC /* JSRTCRtpSender.h in Headers */ = {isa = PBXBuildFile; fileRef = 5E2C43701BCF0D690001E2BC /* JSRTCRtpSender.h */; settings = {ASSET_TAGS = (); }; };
> +		5E16A2E41BFA650B0029A21E /* MediaEndpointPeerConnection.h in Headers */ = {isa = PBXBuildFile; fileRef = 5E16A2E31BFA64FB0029A21E /* MediaEndpointPeerConnection.h */; };

What's all this?
Comment 7 Build Bot 2015-11-18 11:32:03 PST
Comment on attachment 265752 [details]
Patch v1

Attachment 265752 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/446328

New failing tests:
storage/indexeddb/modern/index-cursor-3.html
Comment 8 Build Bot 2015-11-18 11:32:05 PST
Created attachment 265764 [details]
Archive of layout-test-results from ews115 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 9 Brady Eidson 2015-11-18 12:12:02 PST
(In reply to comment #6)
> Comment on attachment 265752 [details]
> Patch v1
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265752&action=review
> 
> I'll finish reviewing once these failing tests are addressed.
> 
> > Source/WebCore/Modules/indexeddb/server/IndexValueEntry.cpp:137
> > +    return m_entry;
> 
> !!m_entry

I believe null vs non-null raw pointers convert to bool with no issue. What's the point behind the !!?
Did I miss a coding style update?


> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:2365
> > -		5E2C43741BCF0D750001E2BC /* JSRTCRtpSender.h in Headers */ = {isa = PBXBuildFile; fileRef = 5E2C43701BCF0D690001E2BC /* JSRTCRtpSender.h */; settings = {ASSET_TAGS = (); }; };
> > +		5E16A2E41BFA650B0029A21E /* MediaEndpointPeerConnection.h in Headers */ = {isa = PBXBuildFile; fileRef = 5E16A2E31BFA64FB0029A21E /* MediaEndpointPeerConnection.h */; };
> 
> What's all this?

Xcode nonsense (You've never seen that...?!?!?)
Comment 10 Brady Eidson 2015-11-18 12:13:46 PST
As for the layout test failures, almost certainly due to the changes in IDBKeyData affecting modern and legacy IDB.

Looking into it.
Comment 11 Brady Eidson 2015-11-18 14:38:16 PST
Created attachment 265781 [details]
Patch v2
Comment 12 Alex Christensen 2015-11-18 15:27:26 PST
Comment on attachment 265781 [details]
Patch v2

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

Iterators wrapping iterators wrapping iterators.

> Source/WebCore/Modules/indexeddb/server/MemoryIndex.cpp:157
> +        notifyCursorsOfAllRecordsChanged();

I'm a little concerned that there's no automated mechanism to find any missing calls to this function (which would lead to security bugs), but it won't be hard to find them with test cases.

> Source/WebCore/Modules/indexeddb/server/MemoryIndexCursor.cpp:121
> +            getResult = { };

clear m_currentKey and m_currentPrimaryKey here, too.
Comment 13 Brady Eidson 2015-11-18 15:39:54 PST
Created attachment 265788 [details]
Patch v3
Comment 14 WebKit Commit Bot 2015-11-18 17:37:12 PST
Comment on attachment 265788 [details]
Patch v3

Rejecting attachment 265788 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 265788, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
0-ab3c-d52691b4dbfc ...
Currently at 192599 = 175b90f42ba454ea4480504968bd5cc4e4932986
r192600 = 94db3ba64bdddb8e6db180b5e0863963ecc1d3a3
r192601 = 8eef4aa53ddad333f4e85017a4580ba3f774d6b3
r192602 = de8712090cf7ee354113849565a71d352d771722
r192603 = bfec9b333ac0ed1ec00d36995a55eececd3c34da
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Fast-forwarded master to refs/remotes/origin/master.

Full output: http://webkit-queues.webkit.org/results/447742
Comment 15 Brady Eidson 2015-11-18 19:48:37 PST
Created attachment 265837 [details]
Patch for landing rebaselined
Comment 16 WebKit Commit Bot 2015-11-18 20:36:06 PST
Comment on attachment 265837 [details]
Patch for landing rebaselined

Clearing flags on attachment: 265837

Committed r192610: <http://trac.webkit.org/changeset/192610>
Comment 17 WebKit Commit Bot 2015-11-18 20:36:11 PST
All reviewed patches have been landed.  Closing bug.