Bug 178274 – We should wrap the removal of information from the tracker database in a transaction in DatabaseTracker::deleteOrigin()

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

[Maureen Daum]

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.

[Maureen Daum]

This is part of <rdar://problem/34576132>

[Maureen Daum]

Created attachment 323719 [details]
patch

[Brady Eidson]

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.

[Maureen Daum]

Created attachment 323746 [details]
patch using SQLiteTransaction

[Maureen Daum]

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!

[Brian Weinstein]

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?

[Maureen Daum]

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

[Maureen Daum]

Created attachment 323916 [details]
patch

[WebKit Commit Bot]

Comment on attachment 323916 [details]
patch

Clearing flags on attachment: 323916

Committed r223423: <https://trac.webkit.org/changeset/223423>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alex Christensen]

This broke the iOS build.

[Alex Christensen]

Or something related to this:
https://webkit-queues.webkit.org/results/4873526

[Alex Christensen]

JK, https://trac.webkit.org/changeset/223422/webkit broke the build.