Summary: | We should wrap the removal of information from the tracker database in a transaction in DatabaseTracker::deleteOrigin() | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Maureen Daum <mdaum> | ||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, beidson, bweinstein, commit-queue, mdaum, thorton | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | All | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Maureen Daum
2017-10-13 11:23:00 PDT
This is part of <rdar://problem/34576132> Created attachment 323719 [details]
patch
Comment on attachment 323719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=323719&action=review > Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp:848 > + auto rollbackTransaction = [this]() { > + if (!m_database.executeCommand("ROLLBACK TRANSACTION")) > + RELEASE_LOG_ERROR(DatabaseTracker, "Failed to rollback transaction after trying to remove origin from tracker"); > + }; I think you should be able to use a SQLiteTransaction object here instead of this lambda and manually calling it on exit paths. Created attachment 323746 [details]
patch using SQLiteTransaction
Comment on attachment 323719 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=323719&action=review >> Source/WebCore/Modules/webdatabase/DatabaseTracker.cpp:848 >> + }; > > I think you should be able to use a SQLiteTransaction object here instead of this lambda and manually calling it on exit paths. We don't even have to call rollback() on exit paths because the object destructor does that for us! Comment on attachment 323746 [details] patch using SQLiteTransaction View in context: https://bugs.webkit.org/attachment.cgi?id=323746&action=review > Source/WebCore/ChangeLog:18 > + goes wrong during deletion. Is this part of a different patch? Comment on attachment 323746 [details] patch using SQLiteTransaction View in context: https://bugs.webkit.org/attachment.cgi?id=323746&action=review >> Source/WebCore/ChangeLog:18 >> + goes wrong during deletion. > > Is this part of a different patch? OOPS, this is leftover from v1 … I forgot to pull it out of the changelog Created attachment 323916 [details]
patch
Comment on attachment 323916 [details] patch Clearing flags on attachment: 323916 Committed r223423: <https://trac.webkit.org/changeset/223423> All reviewed patches have been landed. Closing bug. This broke the iOS build. Or something related to this: https://webkit-queues.webkit.org/results/4873526 JK, https://trac.webkit.org/changeset/223422/webkit broke the build. |