Closed Bug 1156046 Opened 5 years ago Closed 3 years ago

nsUpdateProcessor::ProcessUpdate should use the directory service instead of nsXREDirProvider (also removes app update gonk support)

Categories

(Toolkit :: Application Update, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox40 --- affected
firefox55 --- fixed

People

(Reporter: robert.strong.bugs, Assigned: robert.strong.bugs)

References

Details

Attachments

(1 file, 8 obsolete files)

Now that the app update xpcshell tests have a custom directory service the special casing in nsUpdateDriver.cpp for nsUpdateProcessor::ProcessUpdate is no longer necessary.
Assignee: nobody → robert.strong.bugs
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8594558 [details] [diff] [review]
patch rev1

Found a problem with this approach :(
Attachment #8594558 - Attachment is obsolete: true
Attachment #8594558 - Flags: feedback?(netzen)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch patch in progress (obsolete) — Splinter Review
Pushed to oak
Attachment #8599102 - Attachment is obsolete: true
Attached patch patch in progress (obsolete) — Splinter Review
Slightly cleaner
Attachment #8603191 - Attachment is obsolete: true
Attached patch patch rev1 (obsolete) — Splinter Review
This cleans up ProcessUpdates and combines SwitchToUpdatedApp to ApplyUpdate... SwitchToUpdatedApp was only added to allow staging to land.
Attachment #8603199 - Attachment is obsolete: true
Attachment #8603575 - Flags: review?(spohl.mozilla.bugs)
Note: I've run this through try with success but the tests that could verify this code on gonk have been disabled since we added gonk due to other issues. I'm trying to figure out if I can get at least some of these tests to run on gonk and will post a try run at some point later.
Comment on attachment 8603575 [details] [diff] [review]
patch rev1

By fixing bug 821866 I was able to find a bug in the gonk changes so cancelling review until after I fix that.
Attachment #8603575 - Flags: review?(spohl.mozilla.bugs)
Attached patch patch rev1 (obsolete) — Splinter Review
Attachment #8603575 - Attachment is obsolete: true
OS: Windows 8.1 → All
Hardware: x86_64 → All
This also removes gonk
Summary: nsUpdateProcessor::ProcessUpdate should use the directory service instead of nsXREDirProvider → nsUpdateProcessor::ProcessUpdate should use the directory service instead of nsXREDirProvider (also removes app update gonk support)
Though the ApplyUpdate function could be further cleaned up I would prefer just going with combining the existing functionality from ApplyUpdate and SwitchToUpdatedApp into ApplyUpdate and doing any additional cleanup in another bug.
Comment on attachment 8857303 [details] [diff] [review]
patch rev1

Try run is looking good.
Attachment #8857303 - Flags: review?(mhowell)
Comment on attachment 8857303 [details] [diff] [review]
patch rev1

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

::: toolkit/xre/nsUpdateDriver.cpp
@@ +712,2 @@
>    } else {
> +    // Launch updater.exe to either stage or apply an update

nit: the name "updater.exe" got copied into the Mac and Linux sections here.
Attachment #8857303 - Flags: review?(mhowell) → review+
(In reply to Robert Strong [:rstrong] (use needinfo to contact me) from comment #19)
> Before landing I am double checking that the paths are correct for all 3
> cases
> 
> Before the patch
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=7ac8aa602d0609613d095d85fc088909dca677bd
> 
> After the patch
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=abbace5b34ade7510bfdb930515ffe02c320ab10
All of the paths are correct
Accidentally landed with the wrong bug number.
https://hg.mozilla.org/mozilla-central/rev/cc65a6d51e74

I was told that since this made it to m-c it is best to leave it as is.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago3 years ago
Resolution: --- → FIXED
Depends on: 1360745
You need to log in before you can comment on or make changes to this bug.