757711 - Service does not get replaced for newer builds since bug 748764

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect)

Description

[Brian R. Bondy [:bbondy]]

I noticed when doing an update from a build yesterday to today's build that the service was not getting updated.

> Could not overwrite old service binary file. This should never happen, but if it does the next upgrade 
> will fix it, the service is not a critical component that needs to be installed for upgrades to 
> work. (123)
> ERROR: Could not move old service file out of the way from: ""C:\Program Files (x86)\Mozilla 
> Maintenance Service\maintenanceservice.exe"" to ""C:\Program Files (x86)\Mozilla Maintenance 
> Service\maintenanceservice.eold". Service will not be upgraded. (123)

What's happening is that bug 748764 added quotes in the path of the install binary.
We get that path on the next update, but Windows returns it with quotes.  
The MoveFile call fails because of an invalid path error because it has quotes around it.

Comment 1

[Brian R. Bondy [:bbondy]]

This would only effect installs that were done after bug 748764 landed.
Which was for m-c: 
2012-05-02 12:42:07 PDT 

And for beta and aurora on:
2012-05-06 20:13:36 PDT

Comment 2

[Brian R. Bondy [:bbondy]]

The fix will need to:
1) make the postupdate remove the quotes on the service path.
2) Fix the serviceinstall.cpp code

Comment 3

[(no longer active)]

This is a problem for background updates.  I filed bug 757716 to disable Nightly updates on Windows, and will now back out bug 307181. :(

Comment 4

[Brian R. Bondy [:bbondy]]

> The fix will need to:
> 1) make the postupdate remove the quotes on the service path.
> 2) Fix the serviceinstall.cpp code

Actually we can skip #1 because we will execute the new maintenanceservice.exe when updating the service.

Comment 5

[Brian R. Bondy [:bbondy]]

Requesting tracking since this needs to go out with the fix in bug 748764 before release.

Comment 6

[Brian R. Bondy [:bbondy]]

Attachment: Patch v1.

Comment 7

[Brian R. Bondy [:bbondy]]

By the way I made a test program and you can call PathUnquoteSpacesW on a path that is not quoted and it will have no effect.

Comment 8

[(no longer active)]

Comment on attachment 626380 [details] [diff] [review]
Patch v1.

r=me.

Comment 9

[Brian R. Bondy [:bbondy]]

Comment on attachment 626380 [details] [diff] [review]
Patch v1.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 748764
User impact if declined: maintenanceservice.exe won't be replaced.  There are some security tasks that landed after this one that users may not get updated to.
Testing completed (on m-c, etc.): Hasn't landed on m-c yet but I tested updates on elm and it fixes the problem.
Risk to taking this patch (and alternatives if risky): Low
String or UUID changes made by this patch: none

Comment 10

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/ca2b81301149

Comment 11

[Brian R. Bondy [:bbondy]]

Thanks for landing

Comment 12

[Alex Keybl [:akeybl]]

Comment on attachment 626380 [details] [diff] [review]
Patch v1.

[Triage Comment]
It's critical that we keep our update service and FF version in lock step. Approved for Aurora 14 and Beta 13.

Comment 13

[Brian R. Bondy [:bbondy]]

http://hg.mozilla.org/releases/mozilla-aurora/rev/2056622a1ac0
http://hg.mozilla.org/releases/mozilla-beta/rev/7bff206f5c75

Comment 14

[Brian R. Bondy [:bbondy]]

Simona can you confirm if we check the maintenanceservice-install.log for errors in litmus tests?

Bug 748764 introduced this regression on beta on 2012-05-06.
I'm not sure how often we run the litmus tests, but could you please add a test for that if not already present?

Comment 15

[u279076]

Simona, can you please verify this bug is now fixed. Since this is critical it needs verification in Firefox 13, 14, and 15. Thanks.

Comment 16

[Simona Badau, Desktop QA]

(In reply to Brian R. Bondy [:bbondy] from comment #14)
> Simona can you confirm if we check the maintenanceservice-install.log for
> errors in litmus tests?
> 
> Bug 748764 introduced this regression on beta on 2012-05-06.
> I'm not sure how often we run the litmus tests, but could you please add a
> test for that if not already present?

Added a test case in the Beta Smoketests that will be ran now regularly. Also added all the existing test cases on the Aurora branch included in the Basic Functional tests and Full Functional tests.

Comment 17

[Brian R. Bondy [:bbondy]]

Awesome, thanks so much Simona!

Comment 18

[Simona Badau, Desktop QA]

On the Nightly branch the service still does not get updated in the Mozilla Maintenance service install folder. If I check in the Firefox install folder then the service looks updated. The maintenanceservice-install log does not updated either. I reproduced this on Windows 7 with the disabled UAC and on Windows XP.

On Aurora both the service and the maintenanceservice-install log get properly updated.

Comment 19

[Brian R. Bondy [:bbondy]]

Simona what you are seeing is:
bug 758998 - Background updates breaks upgrading the service on XP

It says XP in the title but it also happens for any build where the service is not used.  You can test this bug in particular by setting app.update.stage.enabled to false first. 

Please also test bug 758998 though with the steps you were testing this bug with.

Comment 20

[Simona Badau, Desktop QA]

Verified that when updating from Firefox 13 beta 5 to Firefox 13 beta 6 the service is updated to a newest one. Verified on Windows 7, Windows XP and on Windows Vista x64:

Mozilla/5.0 (Windows NT 6.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/20100101 Firefox/13.0
Mozilla/5.0 (Windows NT 6.0; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0

Verified also on Windows 7 and on Win XP that when updating the second newest Aurora and Nightly builds the service get updated. On the Nightly branch verified that the service gets updated to the latest one by changing the setting app.update.stage.enabled to false due to bug 758998.

Mozilla/5.0 (Windows NT 5.1; rv:14.0) Gecko/20120529 Firefox/14.0a2
Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20120530 Firefox/14.0a2

Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/15.0 Firefox/15.0a1
Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/15.0 Firefox/15.0a1

Thanks Brian for the guidance.

Comment 21

[u279076]

Based on the last comment I think this can be marked verified for Firefox 14 and 15.