We should wrap the removal of information from the tracker database in a transaction in DatabaseTracker::deleteOrigin(). Otherwise we could end up removing information from just one of the tables if something goes wrong.
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.