Summary: | Implement IDBTransaction.commit() | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||
Component: | WebCore Misc. | Assignee: | Sihui Liu <sihui_liu> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | achristensen, alecflett, beidson, esprehn+autocc, ews-watchlist, ggaren, japhet, jsbell, kondapallykalyan, sihui_liu, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Chris Dumez
2021-07-08 14:19:46 PDT
If someone more knowledgeable with IDB could implement this, that'd be great. If no-one implements it, I'll probably look into it eventually but it will likely take me some time. Created attachment 433661 [details]
Patch
Created attachment 433681 [details]
Patch
Created attachment 433689 [details]
Patch
Comment on attachment 433689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433689&action=review > Source/WebCore/Modules/indexeddb/IDBRequest.cpp:299 > + m_dispatchingEvent = &event; We should rename the data member now that it is no longer a boolean, maybe m_eventBeingDispatched. > Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:487 > + for (auto request : m_openRequests) { auto& to avoid ref counting churn. Could also be written as (if you prefer): auto pendingRequestCount = std::count_if(m_openRequests.begin(), m_openRequests.end(), [](auto& request) { return !request->isDone(); }); > Source/WebCore/Modules/indexeddb/IDBTransaction.cpp:492 > + scheduleOperation(IDBClient::TransactionOperationImpl::create(*this, nullptr, [protectedThis = makeRef(*this), pendingRequestCount] (auto& operation) { makeRef() is deprecated. `protectedThis = Ref { *this }` is shorter with C++17. > LayoutTests/imported/w3c/ChangeLog:11 > + * web-platform-tests/IndexedDB/idb-explicit-commit.any-expected.txt: The change log should probably explain why the test is timing out and why we think that's OK. Created attachment 433810 [details]
Patch
Created attachment 433811 [details]
Patch
Comment on attachment 433811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433811&action=review > LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idb-explicit-commit.any-expected.txt:13 > +TIMEOUT Transactions with same scope should stay in program order, even if one calls commit. Test timed out Is there a good explanation why this is happening, or do we know what it would take to fix it? Comment on attachment 433811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433811&action=review > LayoutTests/imported/w3c/ChangeLog:11 > + * web-platform-tests/IndexedDB/idb-explicit-commit.any-expected.txt: Some test is timed out as our backend does Alex: I asked Sihui to add an explanation about this in the ChangeLog. Comment on attachment 433811 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=433811&action=review >> LayoutTests/imported/w3c/web-platform-tests/IndexedDB/idb-explicit-commit.any-expected.txt:13 >> +TIMEOUT Transactions with same scope should stay in program order, even if one calls commit. Test timed out > > Is there a good explanation why this is happening, or do we know what it would take to fix it? Yes, the test expects a later transaction to finish and fire events to end the first transactions. We don't support running multiple simultaneous transactions so the later transaction will be blocked. We would have passed the next two NOTRUN tests, but the test is timed out here. Comment on attachment 433811 [details]
Patch
Seems fine.
(In reply to Brady Eidson from comment #12) > Comment on attachment 433811 [details] > Patch > > Seems fine. (With other previous feedback) Committed r280053 (239788@main): <https://commits.webkit.org/239788@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 433811 [details]. |