Closed Bug 758326 Opened 12 years ago Closed 12 years ago

Port bug 481815 - Provide a Windows service to update applications without asking Administrator password

Categories

(Thunderbird :: Installer, enhancement, P1)

enhancement

Tracking

(thunderbird16 fixed)

RESOLVED FIXED
Thunderbird 17.0
Tracking Status
thunderbird16 --- fixed

People

(Reporter: standard8, Assigned: standard8)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [Main port landed, needs testing then enabling])

Attachments

(10 files, 2 obsolete files)

40.99 KB, patch
standard8
: review+
bwinton
: ui-review+
Details | Diff | Splinter Review
1.14 KB, patch
Bienvenu
: review+
Details | Diff | Splinter Review
691 bytes, patch
rain1
: review+
Details | Diff | Splinter Review
602 bytes, patch
rain1
: review+
Details | Diff | Splinter Review
1.28 KB, patch
rain1
: review+
Details | Diff | Splinter Review
5.94 KB, patch
rain1
: review+
bwinton
: ui-review+
Details | Diff | Splinter Review
760 bytes, patch
rain1
: review+
Details | Diff | Splinter Review
1.79 KB, patch
bwinton
: review+
Details | Diff | Splinter Review
3.77 KB, patch
bwinton
: review+
bwinton
: ui-review+
Details | Diff | Splinter Review
11.67 KB, application/octet-stream
Details
+++ This bug was initially created as a clone of Bug #481815 +++

We want this as it'll mean we get rid of the need for UAC prompts on Windows.

The work here is to port the changes from bug 481815 mainly the browser/ parts of:

http://hg.mozilla.org/mozilla-central/rev/375aaf23cc2f
http://hg.mozilla.org/mozilla-central/rev/314cab6ecbe3
http://hg.mozilla.org/mozilla-central/rev/1bd9f069576e
http://hg.mozilla.org/mozilla-central/rev/ef967515537b
http://hg.mozilla.org/mozilla-central/rev/c7f5af7dad8d
http://hg.mozilla.org/mozilla-central/rev/d74612c58245
http://hg.mozilla.org/mozilla-central/rev/5b9b207ac6ea

There may be a few others, but that's a good block to begin with.
Depends on: 730328
We'll want the configure.in and autoconf.mk.in parts of this changeset as well as the browser/ parts:

http://hg.mozilla.org/mozilla-central/rev/1bd9f069576e
Attached patch WIP (obsolete) — Splinter Review
This patch ports most parts from m-c to c-c. It's mostly Copy&Paste. I can not test this, because I have no windows.
I have no idea what to do with the files from other-licenses/nsis/Contrib in 314cab6ecbe3, because there is no such path in c-c. And I have no idea what to do with the new files and changes in  /build/ from c7f5af7dad8d and d74612c58245 (e.g. there is no /build/makefile.in for c-c).
But I hope this helps anyway.
(In reply to Nomis101 from comment #2)
> Created attachment 627596 [details] [diff] [review]
> WIP
> 
> This patch ports most parts from m-c to c-c. It's mostly Copy&Paste. I can
> not test this, because I have no windows.

Thanks for doing this!

> I have no idea what to do with the files from other-licenses/nsis/Contrib in
> 314cab6ecbe3, because there is no such path in c-c. And I have no idea what
> to do with the new files and changes in  /build/ from c7f5af7dad8d and
> d74612c58245 (e.g. there is no /build/makefile.in for c-c).

That's fine, we shouldn't have to port those changes.

> But I hope this helps anyway.

Yes, thanks. I'm pushing this to try server at the moment, which is the best way of starting to get some testing on this.

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=0c0c67861cf1
The try server build failed because we haven't ported bug 525438. I've pushed an additional change to tweak the code slightly which will hopefully work. Currently waiting on it to build, looks like try is a bit backlogged at the moment:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=cfa9299b4441
Yes, win32 builds on try server are very backlogged due to work on bug 709480: Switch to using 64 bit builders for 32 bit windows builds, to avoid 3GB virtual address space limit.

coop is aware of the problem and it working to get this resolved as soon as possible.
Ok, still didn't work, because we hadn't ported bug 592133. I've done a patch for that and pushed it to try server. Third time lucky maybe...
Depends on: 760988
This is the slightly tweaked patch so that it compiles.
Attachment #629605 - Flags: ui-review?(bwinton)
Attachment #629605 - Flags: review?(mbanner)
We've not had enough chance to do testing on the maintenance service before the merge to be able to enable it, however, I'd like the option that if we get the testing done soon after the merge, then we'd enable it for aurora as well as trunk, hence landing it before we branch (so we've got the strings) just disabled for now.
Attachment #629606 - Flags: review?(dbienvenu)
Comment on attachment 629605 [details] [diff] [review]
[checked in] The fix

Review of attachment 629605 [details] [diff] [review]:
-----------------------------------------------------------------

We still need some more testing on this, but it seems that everything is there and it can build and install the maintenance service. As I said with the disabling patch, we'll land this disabled for now, and enable it later once we've done that testing. This does give us the option to enable it on aurora in a week or so if we're ready.
Attachment #629605 - Flags: review?(mbanner) → review+
Attachment #629606 - Flags: review?(dbienvenu) → review+
Comment on attachment 629605 [details] [diff] [review]
[checked in] The fix

ui-r=me with nits mentioned over irc fixed, natch.  ;)
Attachment #629605 - Flags: ui-review?(bwinton) → ui-review+
Thanks for the port Nomis, this is now landed, but disabled whilst we do some testing. Once we're confident enough, I'm hoping that we can enable it on aurora as well as trunk later.

https://hg.mozilla.org/comm-central/rev/981ac3d7e714
https://hg.mozilla.org/comm-central/rev/9db1e7ed1ad9
Whiteboard: [Main port landed, needs testing then enabling]
(In reply to Mark Banner (:standard8) from comment #11)
> Thanks for the port Nomis,
you're welcome. Interestingly I didn't get any of the bugmails, because I was not in the CC (notwithstanding I commented and uploaded a patch), so I was a bit surprised to see the checkin today. :-D
This is the first in a series of steps that I'm working on enabling the maintenance service.

This patch enables channel IDs to be written into the MAR files, this shouldn't break anything as MOZ_VERIFY_MAR_SIGNATURE isn't yet set, so the code in updater.cpp [1] won't be compiled. We additionally know this is safe because Firefox only enables checking on Windows, even though the MAR_CHANNEL_ID is set on all platforms [2].

[1] http://hg.mozilla.org/mozilla-central/annotate/75cdb3f932c6/toolkit/mozapps/update/updater/updater.cpp#l2045
[2] http://hg.mozilla.org/mozilla-central/annotate/75cdb3f932c6/browser/confvars.sh#l11
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #651511 - Flags: review?(irving)
This patch enables the verification of the channel id. This has automated tests for Firefox (which we run as part of xpcshell), and once the enable channel ids patch lands, I'll spin a try build, verify the updates still work and then we can land this patch as well.

Combined, these two patches are part of the work towards the maintenance service.
Comment on attachment 651511 [details] [diff] [review]
[checked in] Enable Channel IDs

Sid said he may take a look at this, so I'll take however gets it first, as I want to get most of this landed this week assuming no issues.
Attachment #651511 - Flags: review?(sagarwal)
Attachment #651511 - Flags: review?(sagarwal) → review+
Attachment #651511 - Flags: review?(irving)
Comment on attachment 651511 [details] [diff] [review]
[checked in] Enable Channel IDs

Checked in: https://hg.mozilla.org/comm-central/rev/1c5fc655f6cf
Attachment #651511 - Attachment description: Enable Channel IDs → [checked in] Enable Channel IDs
Attachment #627596 - Attachment is obsolete: true
Attachment #629605 - Attachment description: The fix → [checked in] The fix
Attachment #629606 - Attachment description: Disable maintenance service → [checked in] Disable maintenance service
Comment on attachment 651514 [details] [diff] [review]
[checked in] Enable verification of channel id

I pushed this to try server earlier and was still able to update from the try builds to the current nightlies, so I think this is good to go.

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=1a3c8a70c828
Attachment #651514 - Flags: review?(sagarwal)
I've just pushed this to try to get some Windows builds for testing. This should enable building of the maintenance service but leave it off by default - hence, this should be a no-op, but let us test it by flipping the pref or choosing it on install.

We can stick a couple of health-warning notes when we push this out for the day or two I'm planning that we test it for, then we can flip the defaults.
Try run for 8507a3a1560e is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=8507a3a1560e
Results (out of 5 total builds):
    success: 1
    warnings: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bugzilla@standard8.plus.com-8507a3a1560e
Try run for b3510b582f5d is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=b3510b582f5d
Results (out of 5 total builds):
    success: 1
    warnings: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bugzilla@standard8.plus.com-b3510b582f5d
Comment on attachment 651514 [details] [diff] [review]
[checked in] Enable verification of channel id

Yes, this looks good. Sorry for the delay.
Attachment #651514 - Flags: review?(sagarwal) → review+
Comment on attachment 651514 [details] [diff] [review]
[checked in] Enable verification of channel id

https://hg.mozilla.org/comm-central/rev/9f89e99ede32
Attachment #651514 - Attachment description: Enable verification of channel id → [checked in] Enable verification of channel id
This patch:

- Enables building and installation of the maintenance service
- Enables use of the maintenance service to avoid the UAC prompt
- Explicitly disables staged/background updates

The staged/background updates are disabled for now, as I need to port another patch to adapt the UI to cope accordingly.

I'm going the whole route of enabling building & use of it, as if we go part way, the unit tests fail for some reason. I couldn't quite work that out, and given the testing Firefox has already had on this, I think it is reasonably safe to go forward with it.

Testing wise, I've checked that:

- Maintenance service is installed when it isn't installed
- Service registry entries are updated when it is already installed
- Fallback to in-app updates works if there's a problem with the maintenance service
- You can disable the maintenance service from preferences
- It seems to work fine with Firefox installed alongside
- Updates for a try server build work, when the root certificates and adjustments are also made

To install root certificates:

- Do bug 704578 comment 27 and add attachment 577619 [details] to the list of root certificates
- Add a similar registry key to the last one in attachment 577617 [details], adjusting the app id as necessary (install Thunderbird with maintenance service, and add the key & values to each item listed under the maintenance service)

Try builds that I used are here:

https://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bugzilla@standard8.plus.com-276e018e2104/
https://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bugzilla@standard8.plus.com-b770ee1c1010/ (these ones won't work until after tomorrow's nightlies, though I'd like to land this soon).
Attachment #651738 - Attachment is obsolete: true
Attachment #653489 - Flags: review?(sagarwal)
This ports the work done in:

http://hg.mozilla.org/mozilla-central/rev/c20d415ef1b5 (bug 307181)
https://hg.mozilla.org/mozilla-central/rev/26d651834b0e (bug 757885)

to our about dialog. It gives us the capability for showing the correct UI when we enable staging of updates in the background.

Try server builds available here (with the preference enabled, coming in the next patch):

https://ftp.mozilla.org/pub/mozilla.org/thunderbird/try-builds/bugzilla@standard8.plus.com-64966c506d40/

If running on Windows you'll need the certs and registry entries installed from the previous comment. Note that the installer removes the extra registry entries.
Attachment #653749 - Flags: ui-review?(bwinton)
Attachment #653749 - Flags: review?(sagarwal)
This does the final enabling of staging for background updates.
Attachment #653751 - Flags: review?(sagarwal)
Comment on attachment 653489 [details] [diff] [review]
[checked in] Enable building of the maintenance service and enable it, disable background (staged) updates

From my testing, there seem to be cases where our install code gets confused about the identity of the maintenance service.

Here's my setup:
A fresh install of Windows 7 x64, neither Tb nor Firefox installed
Firefox Aurora 16.0a2
Thunderbird Daily (try build) 17.0a1, from comment 24

(1) Install Tb. A directory called "C:\Program Files (x86)\Mozilla Maintenance Service" is created with a file called maintenanceservice.exe, file version 17.0a1. The Uninstall.exe in the same directory has version 17.0a1.
(2) Install Aurora. While maintenanceservice.exe isn't overwritten, a new file called maintenanceservice_tmp.exe is created (why?) with file version 16.0a2 (why?). The Uninstall.exe is also overwritten with version 16.0a2 (why?).

AFAICT, maintenanceservice_tmp.exe should not be created and Uninstall.exe should not be overwritten.
Attachment #653489 - Flags: review?(sagarwal) → review-
Comment on attachment 653749 [details] [diff] [review]
[checked in] Port the about dialog changes for staging updates in the background

This looks fine, though I guess it's meant to land after the other patch.
Attachment #653749 - Flags: review?(sagarwal) → review+
Brian or Rob, can you comment on comment 26 please? AFAIK we copied the same code that Firefox has...
> (1) Install Tb. A directory called "C:\Program Files (x86)\Mozilla
> Maintenance Service" is created with a file called maintenanceservice.exe,
> file version 17.0a1. The Uninstall.exe in the same directory has version
> 17.0a1.
> (2) Install Aurora. While maintenanceservice.exe isn't overwritten, a new
> file called maintenanceservice_tmp.exe is created (why?) 
> with file version 16.0a2 (why?). 

This is a normal part of the install process.  When you install a new product of any version that contains the maintenance service, and maintenance service is already installed, then maintenanceservice.exe gets copied alongside the pre-existing one.   The one you are installing gets copied in as maintenanceservice_tmp.exe.  maintenanceservice_tmp.exe gets run to do the actual upgrade by passing it command line args. 

If maintenanceservice_tmp.exe is older, then it just gets scheduled to be removed on the next reboot.  If it is newer than what is currently installed, then it gets copied over top of maintenanceservice.exe.

> The Uninstall.exe is also overwritten with version 16.0a2
> (why?).

That's probably a bug, it's best to keep the newest uninstall.exe. It is not that big of a deal though since that code doesn't change, but it would be best to file for that.
Depends on: 784438
(In reply to Brian R. Bondy [:bbondy] from comment #29)
> This is a normal part of the install process.  When you install a new
> product of any version that contains the maintenance service, and
> maintenance service is already installed, then maintenanceservice.exe gets
> copied alongside the pre-existing one.   The one you are installing gets
> copied in as maintenanceservice_tmp.exe.

Does this happen on updates as well? I noticed the mtimes of maintenanceservice_tmp.exe and Uninstall.exe were updated when I installed an Aurora update.

>  maintenanceservice_tmp.exe gets
> run to do the actual upgrade by passing it command line args.

Run when exactly? I don't think it's run at install, and the Windows service itself always points to maintenanceservice.exe.

> If maintenanceservice_tmp.exe is older, then it just gets scheduled to be
> removed on the next reboot.  If it is newer than what is currently
> installed, then it gets copied over top of maintenanceservice.exe.

How is older/newer determined? mtime, version, build date, something else?

> 
> That's probably a bug, it's best to keep the newest uninstall.exe. It is not
> that big of a deal though since that code doesn't change, but it would be
> best to file for that.

OK, filed bug 784438.
Comment on attachment 653749 [details] [diff] [review]
[checked in] Port the about dialog changes for staging updates in the background

The strings seem good to me.  ui-r+.

Later,
Blake.
Attachment #653749 - Flags: ui-review?(bwinton) → ui-review+
(In reply to Siddharth Agarwal [:sid0] from comment #30)

> Does this happen on updates as well? I noticed the mtimes of
> maintenanceservice_tmp.exe and Uninstall.exe were updated when I installed
> an Aurora update.

Yes on each update or product installation.

> Run when exactly? I don't think it's run at install, 

You can find the logic here:
browser\installer\windows\nsis\maintenanceservice_installer.nsi

This is the line that actually does the calling to the exe to do the upgrade/install:

nsExec::Exec '"$INSTDIR\$TempMaintServiceName" upgrade

> and the Windows service
> itself always points to maintenanceservice.exe.

As it should, maintenanceservice_tmp.exe is only used temporarily. 

> How is older/newer determined? mtime, version, build date, something else?

By file version number. Right click on maintenanceservice.exe, properties, details.  File Version is what you're looking for.

> OK, filed bug 784438.

Thanks!
Comment on attachment 653489 [details] [diff] [review]
[checked in] Enable building of the maintenance service and enable it, disable background (staged) updates

Sid, given the comments, and the dependent bug, do you think we could land this? If necessary, I'll try and get that bug fixed before we release, but I haven't got time to do that before we branch.
Attachment #653489 - Flags: review- → review?(sagarwal)
If you mean bug 784438, I wouldn't let this hold you back.  I probably won't be working on it in the near future.
Comment on attachment 653489 [details] [diff] [review]
[checked in] Enable building of the maintenance service and enable it, disable background (staged) updates

All right.
Attachment #653489 - Flags: review?(sagarwal) → review+
Attachment #653751 - Flags: review?(sagarwal) → review+
Do you know how per file versioning works across products? I just want to make sure that Thunderbird's file version won't show up as a much higher number than Firefox's.  I.e. I hope that the version is mostly determined by the build date of the repo.
Good point. Let me check.
It uses the number of days since January 1, 2000. https://mxr.mozilla.org/mozilla-central/source/config/version_win.pl#17

I verified that January 1, 2000 to August 21, 2012 is 4616 days, and that an August 21 nightly has version 17.0.0.4616.
Sounds good, thanks for checking.
Due to having many branches, for any new changes to the actual service, we already check that having an old service upgrading new code works fine anyway.
Comment on attachment 653749 [details] [diff] [review]
[checked in] Port the about dialog changes for staging updates in the background

Checked in earlier today as this works fine even without the maintenance service enabled:

http://hg.mozilla.org/comm-central/rev/e9d5970233ff
Attachment #653749 - Attachment description: Port the about dialog changes for staging updates in the background → [checked in] Port the about dialog changes for staging updates in the background
Comment on attachment 653489 [details] [diff] [review]
[checked in] Enable building of the maintenance service and enable it, disable background (staged) updates

Checked in:

https://hg.mozilla.org/comm-central/rev/d9388937bc4b
Attachment #653489 - Attachment description: Enable building of the maintenance service and enable it, disable background (staged) updates → [checked in] Enable building of the maintenance service and enable it, disable background (staged) updates
Comment on attachment 653751 [details] [diff] [review]
[checked in] Enable staging of updates in the background

Checked in: https://hg.mozilla.org/comm-central/rev/954a43e8fe42
Attachment #653751 - Attachment description: The fix → [checked in] Enable staging of updates in the background
I've not heard any issues over the weekend, so declaring this as done.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 17.0
This is a roll-up of attachment 651511 [details] [diff] [review], attachment 651514 [details] [diff] [review] and attachment 653489 [details] [diff] [review], that will enable UAC (The first two attachments already landed in or before 16).
Attachment #655967 - Flags: review?(bwinton)
(In reply to Mark Banner (:standard8) from comment #46)
> Created attachment 655967 [details] [diff] [review]
> Port to beta (TB 16)
> 
> This is a roll-up of attachment 651511 [details] [diff] [review], attachment
> 651514 [details] [diff] [review] and attachment 653489 [details] [diff] [review]
> [review], that will enable UAC (The first two attachments already landed in
> or before 16).

and I should have said the only difference here is the ACCEPTED_MAR_CHANNEL_IDS and MAR_CHANNEL_ID are updated for comm-beta and match the same style for those that Firefox has here:

http://hg.mozilla.org/releases/mozilla-beta/file/57ce663f5d6f/browser/confvars.sh
Comment on attachment 655967 [details] [diff] [review]
[checked in] Port to beta (TB 16)

Yep, seems good to me.  r=me.

Later,
Blake.
Attachment #655967 - Flags: review?(bwinton) → review+
This ports the UI changes without using strings. For the button, I've just gone back to using the previous "Restart to Update" version of the string.

For the applying status text, I've actually skipped that, and just left the downloading message status. Once applying has completed, the correct next status will then be displayed (for those who actually use the about dialog to get updates).
Attachment #655992 - Flags: ui-review?(bwinton)
Attachment #655992 - Flags: review?(bwinton)
Comment on attachment 655992 [details] [diff] [review]
[checked in] Port UI changes for background updates to beta

Looks good.  r=me, ui-r=me.
Attachment #655992 - Flags: ui-review?(bwinton)
Attachment #655992 - Flags: ui-review+
Attachment #655992 - Flags: review?(bwinton)
Attachment #655992 - Flags: review+
Comment on attachment 655967 [details] [diff] [review]
[checked in] Port to beta (TB 16)

[Triage Comment]
Thunderbird-drivers have decided to push this forward to 16, so that we have plenty of time to sort out any issues ahead of the ESR that's coming off 17.
Attachment #655967 - Flags: approval-comm-beta+
Attachment #655992 - Flags: approval-comm-beta+
Comment on attachment 655967 [details] [diff] [review]
[checked in] Port to beta (TB 16)

Checked in: https://hg.mozilla.org/releases/comm-beta/rev/0d44eadd4af1
Attachment #655967 - Attachment description: Port to beta (TB 16) → [checked in] Port to beta (TB 16)
Comment on attachment 655992 [details] [diff] [review]
[checked in] Port UI changes for background updates to beta

Checked in: https://hg.mozilla.org/releases/comm-beta/rev/06268baf043e
Attachment #655992 - Attachment description: Port UI changes for background updates to beta → [checked in] Port UI changes for background updates to beta
Depends on: 787988
Comment on attachment 653751 [details] [diff] [review]
[checked in] Enable staging of updates in the background

[Triage Comment]
Somehow I missed enabling the background updates in the patches I ported, so we need this as well.
Attachment #653751 - Flags: approval-comm-beta+
I think that I have spotted a difference between Thunderbird and Firefox background update processes. I am not yet filing a bug, as I am not sure if this is by design:

Aurora 17 applies quietly without UAC both the daily 1 MB update and also the full 22 MB update that gets applied if an earlier daily update has got skipped.

But Earlybird 17 only applies the 1 MB daily update quietly. If a daily update gets skipped, then the next time Earlybird installs the full update (just like Firefox), but it requires UAC confirmation. Is that by design?


Thunderbird Earlybird:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Thunderbird/17.0a2 ID:20120928042011

Firefox Aurora:
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120928042009
(In reply to hannu.nyman from comment #55)
> But Earlybird 17 only applies the 1 MB daily update quietly. If a daily
> update gets skipped, then the next time Earlybird installs the full update
> (just like Firefox), but it requires UAC confirmation. Is that by design?

No, not by design. That shouldn't make a difference.  Could you attach a zip of your:
%programdata%\Mozilla\logs
Attached file logs.zip
Reopening ask I still get a daily dialog asking if I want to let the updater make changes to my system.  A requested in a previous post a zip of the Mozilla logs is included.   I note that Firefox daily does not trigger the prompt, only Thunderbird Nightly.
(In reply to Matt from comment #57)
> Created attachment 8396912 [details]
> logs.zip
> 
> Reopening ask I still get a daily dialog asking if I want to let the updater
> make changes to my system.  A requested in a previous post a zip of the
> Mozilla logs is included.   I note that Firefox daily does not trigger the
> prompt, only Thunderbird Nightly.

Please don't re-open long closed bugs, please file new ones.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: