Closed Bug 758463 Opened 12 years ago Closed 12 years ago

Windows XP does not clear prefetch on updates

Categories

(Toolkit :: Application Update, defect)

12 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla16
Tracking Status
firefox14 --- unaffected
firefox15 --- verified
firefox16 --- verified

People

(Reporter: bbondy, Assigned: bbondy)

References

Details

(Whiteboard: [qa!])

Attachments

(1 file)

In Windows XP you can't clear the prefetch with an admin account at all, you can with the system account though. The 1 time operation that we schedule uses the service (and hence the SYSTEM account) and so clears the prefetch in that case. We also clear prefetch in the maintenanceservice.cpp code that runs on updates. But on XP since we don't use the service by default that runs as the administrative account. So right now on XP we only clear prefetch the one time only and not on updates This is not a blocker for landing the prefetch tasks on m-c for v15. In fact the code for this wouldn't even be hit until the next update, so as long as the fix is in PostUpdate (which runs the updated code from helper.exe) we can land this anytime when v16 is on m-c.
Target Milestone: --- → mozilla16
I think I thought of a good way to do this, I'll still do it in v16. Instead of storing a 1 time boolean pref of if prefetch was cleared, I can store the last version prefetch was cleared in. If the last verison prefetch was cleared in does not match the current version, then I would do the 1 time operation again. This would simplify so we don't have to do prefetch clearing at update time at all. It'll also have immediate effect for the next update after it lands in v16. rstrong - thoughts?
I'm ok with that especially if it reduces the paths where prefetch is cleared and the complexity.
Yup it'll allow to clear some extra code, and make only 1 path for clearing prefetch.
Attached patch Patch v1. (deleted) — Splinter Review
Implementation as per Comment 1. A bit of background: This task is mainly being done because you can't clear prefetch from an administrative account on Windows XP. So doing it in the installer is not sufficient on XP. Instead we issue a command to the service which runs as SYSTEM on each upgrade to do the clear. This also simplifies code because there is only one path to clear the prefetch now.
Attachment #632513 - Flags: review?(jmathies)
Comment on attachment 632513 [details] [diff] [review] Patch v1. code looks fine, and rob approved the approach, so r+.
Attachment #632513 - Flags: review?(jmathies) → review+
Thanks for the review, I pushed this to Oak and started a Nightly build to test updates & clearing.
Whiteboard: [qa+]
(In reply to Brian R. Bondy [:bbondy] from comment #7) > Thanks for the review, I pushed this to Oak and started a Nightly build to > test updates & clearing. I'll be happy to take a look as well for a quick test run across Win XP, Vista, 7 32/64 bit, and 8. I'll take a look when the next Oak build rolls around (which is tomorrow I believe, unless I'm mistaken).
Thanks, yup we have to wait for 2 nightly builds to become available, then we should try an upgrade from a version that didn't have this patch to a build that did have this patch. And also a build that did have this patch to a build that did have this patch. Specifically the install logs should no longer clear prefetch at all. And all clearing should happen after 3 minutes after each upgrade.
jsmith the builds are avail on Oak. I tested this on Win7 and it is working correctly. Just a quick note that before we cleared on each update, now we only clear on each update where the version number changes. So after clearing once, if you're testing with a build that has the same version (like what happens on Nightly), then manually modify app.update.service.lastVersionPrefetchCleared to an older version to retest. So for example on first install app.update.service.lastVersionPrefetchCleared will not exist. After the first clear app.update.service.lastVersionPrefetchCleared will be set to 16.0a1. The next update after that won't clear because it is also 16.0a1. So manually set that back to 15.0a1 before the next update so that it will be cleared again on the next update.
By clear on next update I mean specifically that after the browser starts for 3 minutes after an update.
Also please take note of Bug 765227. So when testing just make sure app.update.stage.enabled is set to false.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
What's the reasoning for only clearing prefetch on version change ? From a quick google it looks like Windows is copying files into its prefetch folder, so they'd be stale as soon as the next update changes files, eg, xul.dll. This would affect Beta too, given the constant versioning there between merges.
The clear operation ever really only needs to happen once, but the clear operation could fail for some reason such as the .pf files being in use by Windows. So we should retry the clear operation periodically. Only .exe files exist in the prefetch folder. When they change I don't believe that they are replaced since the way we disable the prefetch is to set it to 0 size and set it to read only. Maybe Windows tries but gets an access denied error. But that's one of the things that jsmith is testing, to see if the files ever get re-enabled by Windows again.
Ah, I was working from the misconception that prefetch was desirable rather than disabled. Thanks.
Verified on Win 7 64-bit, Win Vista, Win XP, and Win 8. Looks good.
Status: RESOLVED → VERIFIED
Whiteboard: [qa+] → [qa!]
Comment on attachment 632513 [details] [diff] [review] Patch v1. [Approval Request Comment] Bug caused by (feature/regressing bug #): 692255 User impact if declined: I will likely need to land a patch that disables the prefetch clearing functionality soon so it doesn't land on beta. I'd like to make sure the prefetch code is in sync on Nightly and Aurora. Even if we don't end up disabling the functionality, it should be working on Windows XP. Testing completed (on m-c, etc.): This has been working correctly on mozilla-central 06-16 Risk to taking this patch (and alternatives if risky): low String or UUID changes made by this patch: none
Attachment #632513 - Flags: approval-mozilla-aurora?
Brian - Do we still want to uplift this to Aurora now that bug 770883 has been filed to disable the functionality?
Yup, this will need to be uplifted before the one in bug 770883.
Comment on attachment 632513 [details] [diff] [review] Patch v1. [Triage Comment] Ensures we're changing the same bits everywhere, and the big hammer is to disable for FF15 if necessary.
Attachment #632513 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
s/changing/testing/
Whiteboard: [qa!] → [qa+]
QA Contact: jsmith
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: