Closed Bug 1572615 Opened 7 months ago Closed 7 months ago

Make bookmark merges fully abortable

Categories

(Firefox :: Sync, task, P1)

task

Tracking

()

RESOLVED FIXED
Firefox 70
Tracking Status
firefox70 --- fixed

People

(Reporter: Lina, Assigned: Lina)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

We have an AbortController on the Rust side that we can use to abort a merge, but it's only triggered at shutdown. Additionally, it's not currently possible to interrupt fetching records or notifying observers, both of which can be pretty expensive (see bug 1561467, in particular, that notifying observers took 18 seconds in one case. I think our use of Async.jankYielder means that we might wait for longer periods).

A while ago, Mark suggested having a watchdog timer automatically abort merges that take longer than a few minutes. 5 feels like a good starting point.

This commit changes mozISyncedBookmarksMerger.merge to return a
cancelable operation, and adds an abort signal option to
SyncedBookmarksMirror.apply. Aborting the signal's
AbortController, or finalizing the mirror, interrupts the merge.

This commit introduces a new Watchdog class that signals an abort,
either after a delay or at shutdown, and wires up the buffered engine
to use it.

Depends on D41310

Attachment #9084196 - Attachment description: Bug 1572615 - Make bookmark merges abortable. r?markh!,tcsc! → Bug 1572615 - Make bookmark merges abortable. r?markh!
Attachment #9084197 - Attachment description: Bug 1572615 - Abort bookmark merges that take longer than 5 minutes. r?markh!,tcsc! → Bug 1572615 - Abort bookmark merges that take longer than 5 minutes. r?markh!
Pushed by kcambridge@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9f104a3a1304
Make bookmark merges abortable. r=markh
https://hg.mozilla.org/integration/autoland/rev/595541e822af
Abort bookmark merges that take longer than 5 minutes. r=markh
Regressions: 1573709
Status: ASSIGNED → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70
You need to log in before you can comment on or make changes to this bug.