Closed
Bug 611076
Opened 14 years ago
Closed 14 years ago
Cancelling an AddonInstall that has been redirected to a new URL isn't possible
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
VERIFIED
FIXED
mozilla2.0b8
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: mossop, Assigned: mossop)
References
Details
Attachments
(2 files)
4.39 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
2.14 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
If the AddonInstall gets a http redirect then cancelling it won't work as we fail to grab the new channel.
Assignee | ||
Updated•14 years ago
|
blocking2.0: --- → betaN+
Assignee | ||
Comment 1•14 years ago
|
||
Simple fix that turned out difficult to test. The http activity observer allows us to wait until the end of the redirected request and verify that it was cancelled. Also slipped in a couple of logging changes that really should have been in bug 593535
Attachment #489845 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•14 years ago
|
Status: NEW → ASSIGNED
Whiteboard: [has patch][needs review rs]
Updated•14 years ago
|
Attachment #489845 -
Flags: review?(robert.bugzilla) → review+
Updated•14 years ago
|
Whiteboard: [has patch][needs review rs] → [has patch]
Assignee | ||
Comment 2•14 years ago
|
||
Landed: http://hg.mozilla.org/mozilla-central/rev/b0e1d3528ad3
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [has patch]
Target Milestone: --- → mozilla2.0b8
Assignee | ||
Comment 3•14 years ago
|
||
Backed out due to windows debug test failures
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 4•14 years ago
|
||
The failure is actually a problem that I guess had already existed and was never tested. In the cancel method we attempt to delete the temporary file but since the download channel closes asynchronously the file is still in use and windows fails that attempt. Adding a second attempt in onStopRequest works.
Attachment #489998 -
Flags: review?(robert.bugzilla)
Assignee | ||
Updated•14 years ago
|
Attachment #489998 -
Attachment is obsolete: true
Attachment #489998 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 5•14 years ago
|
||
Comment on attachment 489998 [details] [diff] [review] properly cleanup temp files Found another issue (bug 611755) but decided it wasn't really worth fixing here for now.
Attachment #489998 -
Attachment is obsolete: false
Attachment #489998 -
Flags: review?(robert.bugzilla)
Comment 6•14 years ago
|
||
Comment on attachment 489998 [details] [diff] [review] properly cleanup temp files What are the chances of there being other cases where temp files need to be cleaned up? Is there a way to test this case as well?
Attachment #489998 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6) > Comment on attachment 489998 [details] [diff] [review] > properly cleanup temp files > > What are the chances of there being other cases where temp files need to be > cleaned up? Is there a way to test this case as well? The xpcshell tests caught this because they verify that there are no temp files left over at the end of every run, which should mean that every case that is tested isn't leaving behind temp files. At this point it might be god to see if we can still do code coverage analysis of test runs to see whether any of hte code still isn't being tested.
Assignee | ||
Comment 8•14 years ago
|
||
Landed: http://hg.mozilla.org/mozilla-central/rev/c85961497ae9
Status: REOPENED → RESOLVED
Closed: 14 years ago → 14 years ago
Resolution: --- → FIXED
Comment 9•13 years ago
|
||
Dave, anything I can test manually? I have created a PHP script which redirects to an XPI file, but I don't see a difference. Probably I don't completely understand this issue.
Assignee | ||
Comment 10•13 years ago
|
||
(In reply to comment #9) > Dave, anything I can test manually? I have created a PHP script which redirects > to an XPI file, but I don't see a difference. Probably I don't completely > understand this issue. What would happen is that cancelling would appear to work in that the add-on wouldn't then get installed or anything, but the add-on's file would continue to be downloaded and get saved in the OS temp directory.
Comment 11•13 years ago
|
||
Ok, so this is kinda hard to test manually without having the download progress bar and a way to cancel a download. Looks like we have to trust the automated test you have created. Marking as verified fixed based on non-failing tests.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•