Allow to exit AppUpdate.jsm update cycle when BITS job is kicked off
Categories
(Toolkit :: Application Update, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox89 | --- | fixed |
People
(Reporter: nalexander, Assigned: bytesized)
References
(Regressed 1 open bug)
Details
Attachments
(3 files)
For the --backgroundtask backgroundupdate
, we'd like to be able to exit the task as soon as a BITS job is started. Right now, the AppUpdate.jsm
state machine transitions to DOWNLOADING
before continuing on; in some way, we want the task to not wait for the BITS job. This might require some small tweaks to the Application Update Service to handle the asynchrony.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 1•3 years ago
|
||
My initial thought was that we could just assume that we are done when we see the DOWNLOADING
state, here. But it looks like we can't do something quite that simple. Here is what the flow of events could look like if we did that:
- Background Update Task runs, kicks off the BITS download, sees the
DOWNLOADING
state and exits. So far so good. - The Background Update Task runs again. But the Update Service may not have had time to transition the the update status from "downloading" to "pending" yet. So the Background Update Task may see that the state is
DOWNLOADING
and immediately exit rather than proceeding with staging.
I think, however, that there is an easy solution to this. When AppUpdater
propagates onProgress
to its listeners (here), it gives the listener the amount of progress that has been made on the download. So instead of just exiting when we see the DOWNLOADING
state, I think that we should exit when we see the DOWNLOADING
state with aProgress != aProgressMax
. That will allow us to distinguish between an in-progress download and a completed download.
There is one other detail that I think should also be added here. If the Background Update Task is downloading during this session, I think that we should lock the Update Service's ability to stage during this session. I'm concerned about the possibility that an extremely fast update download could cause the Update Service to start staging right before the Background Update Task shuts down. This could result in some unpleasant race conditions that I think are best avoided.
Reporter | ||
Comment 2•3 years ago
|
||
(In reply to Kirk Steuber (he/him) [:bytesized] from comment #1)
My initial thought was that we could just assume that we are done when we see the
DOWNLOADING
state, here. But it looks like we can't do something quite that simple. Here is what the flow of events could look like if we did that:
- Background Update Task runs, kicks off the BITS download, sees the
DOWNLOADING
state and exits. So far so good.- The Background Update Task runs again. But the Update Service may not have had time to transition the the update status from "downloading" to "pending" yet. So the Background Update Task may see that the state is
DOWNLOADING
and immediately exit rather than proceeding with staging.I think, however, that there is an easy solution to this. When
AppUpdater
propagatesonProgress
to its listeners (here), it gives the listener the amount of progress that has been made on the download. So instead of just exiting when we see theDOWNLOADING
state, I think that we should exit when we see theDOWNLOADING
state withaProgress != aProgressMax
. That will allow us to distinguish between an in-progress download and a completed download.
This sounds good. I was not aware of progress max -- are we confident that we always know the max, i.e., that we always have Content-Length
or its equivalent?
There is one other detail that I think should also be added here. If the Background Update Task is downloading during this session, I think that we should lock the Update Service's ability to stage during this session. I'm concerned about the possibility that an extremely fast update download could cause the Update Service to start staging right before the Background Update Task shuts down. This could result in some unpleasant race conditions that I think are best avoided.
If we can do this, I would like to. The state machine of notifications feels like a train that's hard to get off the tracks to me, anything to have more station-stops sounds good.
Assignee | ||
Comment 3•3 years ago
|
||
(In reply to Nick Alexander :nalexander [he/him] from comment #2)
I was not aware of progress max -- are we confident that we always know the max, i.e., that we always have
Content-Length
or its equivalent?
We actually require that the file size be reported by the server, and we won't even download the file if the size reported isn't the size we expect. The only exception (which I'll be sure to handle in this implementation) is when BITS shows that the download job exists, but hasn't connected yet. During that time, the total file size is reported as -1
. See here for details on all this.
Also, because the AppUpdater
implementation gives very similar notifications here and here, I'll have to be careful in the implementation here that I check that those values are defined before I use them for anything.
Assignee | ||
Comment 4•3 years ago
|
||
The background update task will exit when downloading begins. We do not want the update process to proceed beyond downloading at this time, both for consistency and to prevent any potential race condiditions around staging while we are shutting down. The new nsIApplicationUpdateService.onlyDownloadUpdatesThisSession interface will allow us to prevent updates from proceeding past the "downloading" stage for the remainder of the session.
Depends on D110647
Assignee | ||
Comment 5•3 years ago
|
||
Depends on D110648
Assignee | ||
Comment 6•3 years ago
|
||
Depends on D110649
Pushed by ksteuber@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f1e9a1bd16a6 Add AUS.onlyDownloadUpdatesThisSession r=nalexander https://hg.mozilla.org/integration/autoland/rev/5273d9bd86d9 Test for AUS.onlyDownloadUpdatesThisSession r=nalexander https://hg.mozilla.org/integration/autoland/rev/b95c7ed878ee Exit the Background Update Task once a new update download has started r=nalexander
Comment 8•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f1e9a1bd16a6
https://hg.mozilla.org/mozilla-central/rev/5273d9bd86d9
https://hg.mozilla.org/mozilla-central/rev/b95c7ed878ee
Description
•