Closed Bug 1695797 Opened 4 years ago Closed 4 years ago

In background task mode, only process updates if we're the only instance running

Categories

(Toolkit :: Background Tasks, enhancement)

enhancement

Tracking

()

RESOLVED FIXED
88 Branch
Tracking Status
firefox88 --- fixed

People

(Reporter: nalexander, Assigned: nalexander)

References

Details

Attachments

(1 file, 2 obsolete files)

Right now, firefox --backgroundtask ... will process any pending update at startup, just like Firefox does. We don't want to do this unless we're the only instance running: if we rename into place a staged update, then we'll update underneath another instance, causing Bug 1492023. This ticket tracks using the multi-instance lock added by Bug 1553982 to avoid pumping the update driver unless we're the lone instance.

Why is this not done without regard to the mode? Per the implementor (:mhowell), the multi-instance lock is designed to allow the user to break/ignore the lock via UI. At application startup, that's not possible. But for background task mode, we can make the decision to always respect the lock.

This is a choice for this specific use-case; it's similar to the choice we intend to pursue when launching the devtools as well: Bug 1120863.

This commit does three main things:

  1. It allows to configure the global singleton nsUpdateSyncManager
    with an nsIFile rather than having it use the ambient XPCOM
    directory service. This allows to initialize the
    nsUpdateSyncManager very early: before processing updates and long
    before XPCOM is initialized. This in turn allows us to determine if
    other instances early enough to skip processing updates when
    appropriate.

When this initialization path is followed, i.e., in Firefox but not
xpcshell, the xpcom-startup notification will be received but no
action taken, since the singleton will already exist.

There is a classic time-of-check, time-of-use race window in this
implementation: an instance may be launched immediately after we check
for other instances. In practice this will result in behaviour that
is alreay possible: two independent instances both processing updates.
It is expected that the updater itself will exclude one of the
instances using its existing mutex.

  1. It updates an existing background task test to use an explicit
    nsIFile rather than the existing directory service method. This
    exercises the newer API. There are other tests that might benefit,
    but there's no harm in remaining with the previous approach, since
    both are required.

  2. It adds a new background task test to verify that update processing
    is skipped if we're not the sole instance running.

Depends on D106993

Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Pushed by nalexander@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8a6f82dc06a9 In background task mode, only process updates if we're the sole instance running. r=mhowell,application-update-reviewers,dthayer,bytesized
Pushed by nalexander@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/de11c0fbd901 In background task mode, only process updates if we're the sole instance running. r=mhowell,application-update-reviewers,dthayer,bytesized
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 88 Branch

Comment on attachment 9207286 [details]
nsAppRunner.cpp

This doesn't seem relevant.

Attachment #9207286 - Attachment is obsolete: true
Attachment #9207286 - Attachment is patch: false
Flags: needinfo?(salinadaynajoshua86533)

Clearing NI since I've relanded this. I needed to tweak the test a little to avoid a Windows issue, ticket is Bug 1696772. The DO NOT LAND patch that came later was for help debugging that ticket.

Flags: needinfo?(nalexander)
Attachment #9207891 - Attachment is obsolete: true
Component: Application Update → Background Tasks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: