Closed Bug 1130488 Opened 7 years ago Closed 7 years ago

Perma orange on comm-central: TEST-UNEXPECTED-TIMEOUT | toolkit/components/downloads/test/unit/test_cancel_download_files_removed.js | Test timed out

Categories

(Toolkit :: Downloads API, defect)

Product:
Toolkit
Component:
Downloads API
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla38
Tracking Flags:
Tracking Status
firefox36 --- unaffected
firefox37 --- unaffected
firefox38 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: jcranmer, Assigned: mkmelin)

References

Details

(Keywords: intermittent-failure, regression)

Attachments

(2 files, 1 obsolete file)

bug1130488_register_legacy_dlmanager.patch
2.69 KB, patch
philip.chee
: review+
Details | Diff | Splinter Review
mozilla-bug1130488_disable_dl_tests.patch
2.24 KB, patch
Paolo
: review+
Details | Diff | Splinter Review 
Along with a few other tests. <https://hg.mozilla.org/mozilla-central/rev/afbba6c11b19> is the regressing patch
The required fix is outlined in bug 1114624.

(I'm excluding this bug from bugmail, please use bug 1114624 or a new one if you need more information.)
OS: Linux → All
Hardware: x86_64 → All
I think this is it
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
(In reply to Magnus Melin from comment #39)
> Created attachment 8560996 [details] [diff] [review]
> bug1130488_register_legacy_dlmanager.patch
> 
> I think this is it
May I suggest you put this in mailGlue.js in a profile-after-change section?
Yeah, the above patch wasn't working
This is probably the correct place yes. 

I think this makes things work in practice, but tests still fail. This code never gets called there.
Attachment #8560996 - Attachment is obsolete: true
(someone feel free to take over)
Assignee: mkmelin+mozilla → nobody
(In reply to Magnus Melin from comment #42)
> Created attachment 8561092 [details] [diff] [review]
> bug1130488_register_legacy_dlmanager.patch
> 
> This is probably the correct place yes. 
> 
> I think this makes things work in practice, but tests still fail. This code
> never gets called there.

The following works in SeaMonkey:
https://bug1130515.bugzilla.mozilla.org/attachment.cgi?id=8560878
(Please ignore the nsIDownloadManagerUI bits, they are not necessary)
xpcshell tests don't start the profile, hence the profile-after-change never gets called.
Let's just disable these tests for thunderbird until we drop the legacy download manager. 

I don't think we can override the Downloads.manifest change (short of backing out 1114624), which is what's causing the problems.
Assignee: nobody → mkmelin+mozilla
Attachment #8563577 - Flags: review?(paolo.mozmail)
Comment on attachment 8563577 [details] [diff] [review]
mozilla-bug1130488_disable_dl_tests.patch

I think it's fine if you want to just disable the xpcshell tests after you fix the component registration. In the tests for the JavaScript API we used to have the registration of the alternative components using the CID in the head.js file, but this might be more work than needed here, and we also plan to remove the old tests anyways.
Attachment #8563577 - Flags: review?(paolo.mozmail) → review+
Duplicate of this bug: 1042271
Comment on attachment 8561092 [details] [diff] [review]
bug1130488_register_legacy_dlmanager.patch

This, minus the dump.
I not exactly sure what was broken without this... basic attachment handling seems ok with and without.
Attachment #8561092 - Flags: review?(philip.chee)
Comment on attachment 8561092 [details] [diff] [review]
bug1130488_register_legacy_dlmanager.patch

> +        Components.manager.QueryInterface(Components.interfaces.nsIComponentRegistrar)
> +          .registerFactory(Components.ID("{1b4c85df-cbdd-4bb6-b04e-613caece083c}"),
..............................................^
{1b4c85df-cbdd-4bb6-b04e-613caece083c} is the CID of the jsdownloads nsITransfer.

From Bug 1114624 Comment 0:

(In reply to :Paolo Amadini from comment #0)
> The override required in applications that still use nsIDownloadManager
> would be implemented at startup using a technique similar to the one removed
> by the patch.

> The legacy CID and the contract ID are defined as NS_DOWNLOAD_CID and
> NS_TRANSFER_CONTRACTID, and they are
> "{b02be33b-d47c-4bd3-afd9-402a942426b0}" and "@mozilla.org/transfer;1"
> respectively.

s/1b4c85df-cbdd-4bb6-b04e-613caece083c/b02be33b-d47c-4bd3-afd9-402a942426b0/
r=me with this fixed.


(In reply to Magnus Melin from comment #119)
> Comment on attachment 8561092 [details] [diff] [review]
> bug1130488_register_legacy_dlmanager.patch
> 
> This, minus the dump.
> I not exactly sure what was broken without this... basic attachment handling
> seems ok with and without.
1. Go Tools->Saved Files
2. Notice that saved (i.e. downloaded) attachments don't show up in the legacy downloads window.
Attachment #8561092 - Flags: review?(philip.chee) → review+
Keywords: regression
Target Milestone: --- → mozilla38
Didn't work....
What didn't work? The disabling hasn't been merged to mozilla-central yet.
https://hg.mozilla.org/mozilla-central/rev/69f1b692f6fe
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Depends on: 1087233
You need to log in before you can comment on or make changes to this bug.