Bug 230128 – Assertions in IDBTransaction::request*() methods fail on cross-realm methods

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alexey Shvayka]

/Users/admin/Desktop/foo.diff            Assertions in IDBTransaction::request*() methods fail on cross-realm methods

[Alexey Shvayka]

Created attachment 437798 [details]
Patch

[EWS Watchlist]

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

[Radar WebKit Bug Importer]

<rdar://problem/83217153>

[Sihui Liu]

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.

[Alexey Shvayka]

Created attachment 441165 [details]
Patch

Ensure '=' on all changed lines is surrounded by spaces.

[Alexey Shvayka]

(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.

[Sihui Liu]

Comment on attachment 441165 [details]
Patch

r=me

[Alexey Shvayka]

Created attachment 442311 [details]
Patch for landing

[Alexey Shvayka]

(In reply to Sihui Liu from comment #7)
> Comment on attachment 441165 [details]
> Patch
> 
> r=me

Thanks! Landing now after WPT PR was merged.

[EWS]

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].