In background task mode, only process updates if we're the only instance running
Categories
(Toolkit :: Background Tasks, enhancement)
Tracking
()
Tracking | Status | |
---|---|---|
firefox88 | --- | fixed |
People
(Reporter: nalexander, Assigned: nalexander)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
text/x-phabricator-request
|
Details |
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.
Assignee | ||
Comment 1•4 years ago
|
||
This commit does three main things:
- It allows to configure the global singleton
nsUpdateSyncManager
with annsIFile
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.
-
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. -
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
Updated•4 years ago
|
Assignee | ||
Comment 2•4 years ago
|
||
Try build is looking happy over at https://treeherder.mozilla.org/jobs?repo=try&test_paths=browser-toolbox&revision=08366517b94fbf6a76e0d0c3e0534bb7104720ee.
Comment 4•4 years ago
|
||
Backed out changeset 8a6f82dc06a9 (bug 1695797) for xpcshell failures in test_backgroundtask_update_sync_manager.js.
https://hg.mozilla.org/integration/autoland/rev/1646ae19a2625033ef7d7606cf806d9ed053568a
Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=8a6f82dc06a98796026e47976a76bdd1b1e689df&selectedTaskRun=BzxwAcchTTuFodzpIbTizQ.0
Failure log:
https://treeherder.mozilla.org/logviewer?job_id=332116836&repo=autoland&lineNumber=6747
Comment 6•4 years ago
|
||
bugherder |
Comment 8•4 years ago
|
||
Comment on attachment 9207286 [details]
nsAppRunner.cpp
This doesn't seem relevant.
Assignee | ||
Comment 9•4 years ago
|
||
Assignee | ||
Comment 10•4 years ago
|
||
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.
Updated•4 years ago
|
Updated•3 years ago
|
Description
•