Bug 230128 - Assertions in IDBTransaction::request*() methods fail on cross-realm methods
Summary: Assertions in IDBTransaction::request*() methods fail on cross-realm methods
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Enhancement
Assignee: Alexey Shvayka
URL:
Keywords: InRadar
Depends on:
Blocks: 231702
  Show dependency tree / graph
 
Reported: 2021-09-09 15:55 PDT by Alexey Shvayka
Modified: 2021-10-24 10:37 PDT (History)
14 users (show)

See Also:

Attachments
Patch (79.10 KB, patch)
2021-09-09 16:14 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch (79.07 KB, patch)
2021-10-13 17:36 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Patch for landing (79.25 KB, patch)
2021-10-24 09:53 PDT, Alexey Shvayka 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) 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 Alexey Shvayka 2021-09-09 15:55:23 PDT Comment hidden (obsolete)
Comment 1 Alexey Shvayka 2021-09-09 16:14:00 PDT 
Created attachment 437798 [details]
Patch
Comment 2 EWS Watchlist 2021-09-09 16:14:45 PDT 
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 Radar WebKit Bug Importer 2021-09-16 15:56:23 PDT 
<rdar://problem/83217153>
Comment 4 Sihui Liu 2021-10-13 16:14:20 PDT 
Comment on attachment 437798 [details]
Patch

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

Patch looks good! Nits: there are many places that space is missing around = (search for "state="). 

And I think we usually don't modify imported tests directly (as they should be synced with upstream). 
So maybe either upload the test to wpt first and import it; or place the test in LayoutTests/storage/indexeddb (may need some modification).

> Source/WebCore/Modules/indexeddb/IDBIndex.cpp:181
> +    return doOpenCursor(direction, [state=&execState, key]() {

Nit: space around =

> Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:634
> +    return doGetAll(count, [state=&execState, key]() {

Ditto.

> Source/WebCore/Modules/indexeddb/IDBObjectStore.cpp:671
> +    return doGetAllKeys(count, [state=&execState, key]() {

Ditto.
Comment 5 Alexey Shvayka 2021-10-13 17:36:53 PDT 
Created attachment 441165 [details]
Patch

Ensure '=' on all changed lines is surrounded by spaces.
Comment 6 Alexey Shvayka 2021-10-13 17:37:50 PDT 
(In reply to Sihui Liu from comment #4)
> And I think we usually don't modify imported tests directly (as they should
> be synced with upstream). 
> So maybe either upload the test to wpt first and import it; or place the
> test in LayoutTests/storage/indexeddb (may need some modification).

Thank you for feedback, Sihui!

Regarding the tests: I am following the https://trac.webkit.org/wiki/WPTExportProcess here; as soon as the patch is reviewed, the WPT PR (https://github.com/web-platform-tests/wpt/pull/31221) gets approved by the bot and landed by me to ensure we are in sync.
Comment 7 Sihui Liu 2021-10-13 20:50:52 PDT 
Comment on attachment 441165 [details]
Patch

r=me
Comment 8 Alexey Shvayka 2021-10-24 09:53:45 PDT 
Created attachment 442311 [details]
Patch for landing
Comment 9 Alexey Shvayka 2021-10-24 09:58:16 PDT 
(In reply to Sihui Liu from comment #7)
> Comment on attachment 441165 [details]
> Patch
> 
> r=me

Thanks! Landing now after WPT PR was merged.
Comment 10 EWS 2021-10-24 10:37:48 PDT 
Committed r284761 (243470@main): <https://commits.webkit.org/243470@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442311 [details].