Closed Bug 709598 Opened 8 years ago Closed 8 years ago

If no registry keys exist for the install dir, don't try to use the maintenance service for updates

Categories

(Toolkit :: Application Update, defect)

x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla12

People

(Reporter: bbondy, Assigned: bbondy)

References

Details

Attachments

(1 file, 2 obsolete files)

In cases that a limited user account installs the maintenance service, we do not try to install the service.  If the service happens to be installed after that though, from a different installation then the limited user account installation, then we may try to use the service.

This is not a big deal anymore since we only use the service if we need to (i.e. only if we can't write to the install dir).  But there are other edge cases we should be catching like build config that disable using maintenance service, or x64 native.  Those should not have pending-service state to begin with, but perhaps we are upgrading from/to a version built with/without the maintenance service, and this check is safest to have.
Attached patch patch v1. (obsolete) — Splinter Review
Attachment #580770 - Flags: review?(robert.bugzilla)
Attached patch Patch v2. (obsolete) — Splinter Review
Rebased.
Attachment #580770 - Attachment is obsolete: true
Attachment #580770 - Flags: review?(robert.bugzilla)
Attachment #581889 - Flags: review?(robert.bugzilla)
Comment on attachment 581889 [details] [diff] [review]
Patch v2.

Looks good!
Attachment #581889 - Flags: review?(robert.bugzilla) → review+
Carrying forward the r+ but I added in a check to allow the fallback key as well.
I checked with Ian and he thinks it is OK to have the fallback key.  We no longer install the fallback key as part of the maintenance service installer (its already removed in bug 481815 from the installer, but the uninstaller removes it still). 

It will be used to bypass the validating if a firefox install actually exists in this location.  But the key must be manually added into HKLM by an elevated process.  

It will be documented in the wiki how to enable tests.  The test slaves already have this reg key so they are already good to go.
Attached patch Patch v3.Splinter Review
Implemented review comments but please see Comment 4.
Security is aware of this by the way and have not given negative feedback about it yet.  I don't anticipate any negative feedback about it.
Attachment #581889 - Attachment is obsolete: true
Attachment #581997 - Flags: review+
(In reply to Brian R. Bondy [:bbondy] from comment #5)
> Created attachment 581997 [details] [diff] [review]
> Patch v3.
> 
> Implemented review comments but please see Comment 4.
> Security is aware of this by the way and have not given negative feedback
> about it yet.  I don't anticipate any negative feedback about it.
Adding the additional check for the fallback key is fine (comment #4).
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: mozilla11 → mozilla12
You need to log in before you can comment on or make changes to this bug.