Bugzilla

The usual problem with using a trigger is Sqlite does not support per statement triggers, instead the trigger runs per each row, that is expensive on batch removals.
We already do have a trigger on moz_bookmarks deletion and we don't have a batch removal API, so the cost woule be acceptable.

One downside is we must send a page-removed notification, that can be solved by dispatching a main thread runnable and notifying from there. This adds some complexity.

Then we should still remove other orphans caused by the page removal, like icons, annotations, or input history.
We could theorically do those removals in a moz_places DELETE trigger (we have one already). Though that trigger would then be activated by history, that has batch removal APIs "unfortunately".
That trigger would be expensive, and likely we don't want it.

Overall, while we could remove the moz_places entry with a trigger, the benefit would be small because:
* we still have to do all the other page related removals outside of the trigger
* it would complicate notifying about the page removal
So I think it may not be worth it, compared to just running cleanup queries and notifying.

While looking at this I notices we should set an origin's recalc_frecency to 1 when a page is removed. We only do that when a page's frecency changes. This sounds like a bug we must fix regardless, so I filed Bug 1837217 for it.

The usual problem with using a trigger is Sqlite does not support per statement triggers, instead the trigger runs per each row, that is expensive on batch removals.
We already do have a trigger on moz_bookmarks deletion and we don't have a batch removal API, so the cost woule be acceptable.

One downside is we must send a page-removed notification, that can be solved by dispatching a main thread runnable and notifying from there. This adds some complexity.

Then we should still remove other orphans caused by the page removal, like icons, annotations, or input history.
We could theorically do those removals in a moz_places DELETE trigger (we have one already). Though that trigger would then be activated by history, that has batch removal APIs "unfortunately".
That trigger would be expensive, and likely we don't want it.

Overall, while we could remove the moz_places entry with a trigger, the benefit would be small because:
* we still have to do all the other page related removals outside of the trigger
* it would complicate notifying about the page removal
So I think it may not be worth it, compared to just running cleanup queries and notifying.

While looking at this I notices we should set an origin's recalc_frecency to 1 when a page is removed. We only do that when a page's frecency changes. This sounds like a bug we must fix regardless, so I filed Bug 1837217 for it.

Note: I must also note removing from moz_places won't solve this bug, that is about moz_origins

The usual problem with using a trigger is Sqlite does not support per statement triggers, instead the trigger runs per each row, that is expensive on batch removals.
We already do have a trigger on moz_bookmarks deletion and we don't have a batch removal API, so the cost woule be acceptable.

One downside is we must send a page-removed notification, that can be solved by dispatching a main thread runnable and notifying from there. This adds some complexity.

Then we should still remove other orphans caused by the page removal, like icons, annotations, or input history.
We could theorically do those removals in a moz_places DELETE trigger (we have one already). Though that trigger would then be activated by history, that has batch removal APIs "unfortunately".
That trigger would be expensive, and likely we don't want it.

Overall, while we could remove the moz_places entry with a trigger, the benefit would be small because:
* we still have to do all the other page related removals outside of the trigger
* it would complicate notifying about the page removal

So I think it may not be worth it, compared to just running cleanup queries and notifying.

While looking at this I notices we should set an origin's recalc_frecency to 1 when a page is removed. We only do that when a page's frecency changes. This sounds like a bug we must fix regardless, so I filed Bug 1837217 for it.

Note: I must also note removing from moz_places won't solve this bug, that is about moz_origins