Closed Bug 292408 Opened 19 years ago Closed 19 years ago

Software update driver for nsAppRunner.cpp

Categories

(Toolkit :: Application Update, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.8final

People

(Reporter: darin.moz, Assigned: darin.moz)

References

Details

(Keywords: fixed1.8)

Attachments

(1 file, 1 obsolete file)

The new software update system is going to need some code in nsAppRunner.cpp to
drive the 'updater' executable (see bug 292021).

Basic operation:

 1) Detect presence of update(s) to apply.
 2) Copy updater executable into working directory.
 3) Run updater executable on first update.

The updater will re-launch firefox with a command line flag to inform it that it
has been updated.  Most likely this command line flag will be specified by
firefox when it launches the updater executable.  If there are multiple patches
to apply, this process may be repeated until there are no more patches.

This should all happen very early on in XRE_main.
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → Firefox1.1
Attached patch prototype patch (obsolete) — Splinter Review
This is a prototype patch.  I've only tested this on Windows.
1) I could use a technical overview here: What are we scanning for, anyway?

2) We should unify the code in nsAppRunner.cpp LaunchChild() and
nsUpdateDriver.cpp ApplyUpdate() as much as possible. I know that execv is not
going to work on mac yet... I've got some interesting diagnostics but no
solution yet for that problem.

3) I would really like for EM+update code to be able to restart the app upon
quit (EM code restart directly so that new extensions will be enabled, update
restart to the updater, then relaunch). Perhaps that's a secondary patch, but we
should keep that in mind. Setting an relaunch-pending flag on nsIAppStartup
seems like the best solution to me.

4) The ProcessUpdates documentation should indicate under what conditions the
function may return. In particular, it looks like we're calling it while XPCOM
is initialized, which is bad. We should do all this update checking outside of a
scoped XPCOM startup.
(In reply to comment #2)
> 1) I could use a technical overview here: What are we scanning for, anyway?

The WIKI page outlines the process this guy is trying to implement.  The WIKI
seems to be down right now, but IIRC the link is:
http://wiki.mozilla.org/Software_Update:Startup


> 2) We should unify the code in nsAppRunner.cpp LaunchChild() and
> nsUpdateDriver.cpp ApplyUpdate() as much as possible. I know that execv is not
> going to work on mac yet... I've got some interesting diagnostics but no
> solution yet for that problem.

Yeah.  Does "system" work on the Mac?  I'm currently just using that from
updater.cpp to re-launch firefox w/ command line flags.


> 3) I would really like for EM+update code to be able to restart the app upon
> quit (EM code restart directly so that new extensions will be enabled, update
> restart to the updater, then relaunch). Perhaps that's a secondary patch, but we
> should keep that in mind. Setting an relaunch-pending flag on nsIAppStartup
> seems like the best solution to me.

Yeah.. I like this plan too.  The more I think about it, the more I actually
want our updating to happen after the browser shuts down, not before it starts
up.  I think that users would prefer that.  We could make it an option, but by
default I think we should always try to startup without delay.  So, I'm thinking
that I'd just move this ProcessUpdates check down where we are about to exit
from XRE_main under normal circumstances.


> 4) The ProcessUpdates documentation should indicate under what conditions the
> function may return. In particular, it looks like we're calling it while XPCOM
> is initialized, which is bad. We should do all this update checking outside of a
> scoped XPCOM startup.

Hrm... no, I am pretty sure that I'm calling ProcessUpdates outside of XPCOM
startup.  At any rate, that was the intention.


Thanks for the feedback.
Attached patch v1 patchSplinter Review
I decided to stick with spawning the updater process from app startup.	If we
want it to appear as though it is running at shutdown, then we can just restart
Firefox.  Anyways, this is a detail that can easily be changed later if
desired.  For now, I want to move forward with what I've got.  It works, etc.
Attachment #182549 - Attachment is obsolete: true
Attachment #185038 - Flags: review?(benjamin)
Comment on attachment 185038 [details] [diff] [review]
v1 patch

In general I prefer while (PR_TRUE) loops to for (;;), but it's not a big deal.
Attachment #185038 - Flags: review?(benjamin) → review+
Attachment #185038 - Flags: approval-aviary1.1a1?
Comment on attachment 185038 [details] [diff] [review]
v1 patch

Changing approval request from 1.1a1 -> 1.1a2
Attachment #185038 - Flags: approval-aviary1.1a1? → approval-aviary1.1a2?
Comment on attachment 185038 [details] [diff] [review]
v1 patch

a=chofmann
Attachment #185038 - Flags: approval-aviary1.1a2? → approval-aviary1.1a2+
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Keywords: fixed1.8
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: